fix: check sidecar container for rolling update (envoyproxy#1323)

**Description**
Fix previous PR's missing check for sidecar containers when handling
envoy gateway deployment/daemonset rolling update.
envoyproxy#1263

Signed-off-by: Dan Sun <[email protected]>
Signed-off-by: Hrushikesh Patil <[email protected]>

Author: yuzisun

cmd/aigw/translate.go

@@ -236,7 +236,7 @@ func translateCustomResourceObjects(
 	gwC := controller.NewGatewayController(fakeClient, fakeClientSet, logr.FromSlogHandler(logger.Handler()),
 		"docker.io/envoyproxy/ai-gateway-extproc:latest", true, func() string {
 			return "aigw-translate"
-		},
+		}, false,
 	)
 	// Create and reconcile the custom resources to store the translated objects.
 	// Note that the order of creation is important as some objects depend on others.

internal/controller/controller.go

@@ -97,10 +97,16 @@ func StartControllers(ctx context.Context, mgr manager.Manager, config *rest.Con
 	if err = ApplyIndexing(ctx, indexer.IndexField); err != nil {
 		return fmt.Errorf("failed to apply indexing: %w", err)
 	}
+	var versionInfo *version.Info
+	kube := kubernetes.NewForConfigOrDie(config)
+	versionInfo, err = kube.Discovery().ServerVersion()
+	if err != nil {
+		return fmt.Errorf("failed to get server version: %w", err)
+	}
 
 	gatewayEventChan := make(chan event.GenericEvent, 100)
 	gatewayC := NewGatewayController(c, kubernetes.NewForConfigOrDie(config),
-		logger.WithName("gateway"), options.ExtProcImage, false, uuid.NewString)
+		logger.WithName("gateway"), options.ExtProcImage, false, uuid.NewString, isKubernetes133OrLater(versionInfo, logger))
 	if err = TypedControllerBuilderForCRD(mgr, &gwapiv1.Gateway{}).
 		WatchesRawSource(source.Channel(
 			gatewayEventChan,
@@ -196,13 +202,6 @@ func StartControllers(ctx context.Context, mgr manager.Manager, config *rest.Con
 	}
 
 	if !options.DisableMutatingWebhook {
-		kube := kubernetes.NewForConfigOrDie(config)
-		var versionInfo *version.Info
-		versionInfo, err = kube.Discovery().ServerVersion()
-		if err != nil {
-			return fmt.Errorf("failed to get server version: %w", err)
-		}
-
 		h := admission.WithCustomDefaulter(Scheme, &corev1.Pod{}, newGatewayMutator(c, kube,
 			logger.WithName("gateway-mutator"),
 			options.ExtProcImage,

internal/controller/gateway.go

@@ -46,19 +46,20 @@ const (
 // to check if the pods of the gateway deployment need to be rolled out.
 func NewGatewayController(
 	client client.Client, kube kubernetes.Interface, logger logr.Logger,
-	extProcImage string, standAlone bool, uuidFn func() string,
+	extProcImage string, standAlone bool, uuidFn func() string, extProcAsSideCar bool,
 ) *GatewayController {
 	uf := uuidFn
 	if uf == nil {
 		uf = uuid.NewString
 	}
 	return &GatewayController{
-		client:       client,
-		kube:         kube,
-		logger:       logger,
-		extProcImage: extProcImage,
-		standAlone:   standAlone,
-		uuidFn:       uf,
+		client:           client,
+		kube:             kube,
+		logger:           logger,
+		extProcImage:     extProcImage,
+		standAlone:       standAlone,
+		uuidFn:           uf,
+		extProcAsSideCar: extProcAsSideCar,
 	}
 }
 
@@ -71,6 +72,9 @@ type GatewayController struct {
 	// standAlone indicates whether the controller is running in standalone mode.
 	standAlone bool
 	uuidFn     func() string // Function to generate a new UUID for the filter config.
+	// Whether to run the extProc container as a sidecar (true) as a normal container (false).
+	// This is essentially a workaround for old k8s versions, and we can remove this in the future.
+	extProcAsSideCar bool
 }
 
 // Reconcile implements the reconcile.Reconciler for gwapiv1.Gateway.
@@ -469,11 +473,21 @@ func (c *GatewayController) annotateGatewayPods(ctx context.Context,
 	for _, pod := range pods {
 		// Get the pod spec and check if it has the extproc container.
 		podSpec := pod.Spec
-		for i := range podSpec.Containers {
-			// If there's an extproc container with the current target image, we don't need to roll out the deployment.
-			if podSpec.Containers[i].Name == extProcContainerName && podSpec.Containers[i].Image == c.extProcImage {
-				rollout = false
-				break
+		if c.extProcAsSideCar {
+			for i := range podSpec.InitContainers {
+				// If there's an extproc sidecar container with the current target image, we don't need to roll out the deployment.
+				if podSpec.InitContainers[i].Name == extProcContainerName && podSpec.InitContainers[i].Image == c.extProcImage {
+					rollout = false
+					break
+				}
+			}
+		} else {
+			for i := range podSpec.Containers {
+				// If there's an extproc container with the current target image, we don't need to roll out the deployment.
+				if podSpec.Containers[i].Name == extProcContainerName && podSpec.Containers[i].Image == c.extProcImage {
+					rollout = false
+					break
+				}
 			}
 		}
 

internal/controller/gateway_test.go

@@ -36,7 +36,7 @@ func TestGatewayController_Reconcile(t *testing.T) {
 	fakeKube := fake2.NewClientset()
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true, Level: zapcore.DebugLevel})))
 	c := NewGatewayController(fakeClient, fakeKube, ctrl.Log,
-		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil)
+		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil, true)
 
 	const namespace = "ns"
 	t.Run("not found must be non error", func(t *testing.T) {
@@ -164,7 +164,7 @@ func TestGatewayController_reconcileFilterConfigSecret(t *testing.T) {
 	kube := fake2.NewClientset()
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true, Level: zapcore.DebugLevel})))
 	c := NewGatewayController(fakeClient, kube, ctrl.Log,
-		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil)
+		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil, true)
 
 	const gwNamespace = "ns"
 	routes := []aigv1a1.AIGatewayRoute{
@@ -289,7 +289,7 @@ func TestGatewayController_reconcileFilterConfigSecret_SkipsDeletedRoutes(t *tes
 	kube := fake2.NewClientset()
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true, Level: zapcore.DebugLevel})))
 	c := NewGatewayController(fakeClient, kube, ctrl.Log,
-		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil)
+		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil, true)
 
 	const gwNamespace = "ns"
 	now := metav1.Now()
@@ -400,7 +400,7 @@ func TestGatewayController_bspToFilterAPIBackendAuth(t *testing.T) {
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true, Level: zapcore.DebugLevel})))
 	c := NewGatewayController(fakeClient, kube, ctrl.Log,
 
-		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil)
+		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil, true)
 
 	const namespace = "ns"
 	for _, bsp := range []*aigv1a1.BackendSecurityPolicy{
@@ -547,7 +547,7 @@ func TestGatewayController_bspToFilterAPIBackendAuth(t *testing.T) {
 func TestGatewayController_bspToFilterAPIBackendAuth_ErrorCases(t *testing.T) {
 	fakeClient := requireNewFakeClientWithIndexes(t)
 	c := NewGatewayController(fakeClient, fake2.NewClientset(), ctrl.Log,
-		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil)
+		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil, true)
 
 	ctx := context.Background()
 	namespace := "test-namespace"
@@ -608,7 +608,7 @@ func TestGatewayController_bspToFilterAPIBackendAuth_ErrorCases(t *testing.T) {
 func TestGatewayController_GetSecretData_ErrorCases(t *testing.T) {
 	fakeClient := requireNewFakeClientWithIndexes(t)
 	c := NewGatewayController(fakeClient, fake2.NewClientset(), ctrl.Log,
-		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil)
+		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil, true)
 
 	ctx := context.Background()
 	namespace := "test-namespace"
@@ -633,7 +633,7 @@ func TestGatewayController_annotateGatewayPods(t *testing.T) {
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true, Level: zapcore.DebugLevel})))
 	const v2Container = "ai-gateway-extproc:v2"
 	c := NewGatewayController(fakeClient, kube, ctrl.Log,
-		v2Container, false, nil)
+		v2Container, false, nil, true)
 	t.Run("pod with extproc", func(t *testing.T) {
 		pod, err := kube.CoreV1().Pods(egNamespace).Create(t.Context(), &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
@@ -748,7 +748,7 @@ func TestGatewayController_annotateDaemonSetGatewayPods(t *testing.T) {
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true, Level: zapcore.DebugLevel})))
 	const v2Container = "ai-gateway-extproc:v2"
 	c := NewGatewayController(fakeClient, kube, ctrl.Log,
-		v2Container, false, nil)
+		v2Container, false, nil, true)
 
 	t.Run("pod without extproc", func(t *testing.T) {
 		pod, err := kube.CoreV1().Pods(egNamespace).Create(t.Context(), &corev1.Pod{
@@ -863,7 +863,7 @@ func TestGatewayController_backendWithMaybeBSP(t *testing.T) {
 	kube := fake2.NewClientset()
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true, Level: zapcore.DebugLevel})))
 	const v2Container = "ai-gateway-extproc:v2"
-	c := NewGatewayController(fakeClient, kube, ctrl.Log, v2Container, false, nil)
+	c := NewGatewayController(fakeClient, kube, ctrl.Log, v2Container, false, nil, true)
 
 	_, _, err := c.backendWithMaybeBSP(t.Context(), "foo", "bar")
 	require.ErrorContains(t, err, `aiservicebackends.aigateway.envoyproxy.io "bar" not found`)
@@ -923,7 +923,7 @@ func TestGatewayController_reconcileFilterMCPConfigSecret(t *testing.T) {
 	kube := fake2.NewClientset()
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true, Level: zapcore.DebugLevel})))
 	c := NewGatewayController(fakeClient, kube, ctrl.Log,
-		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil)
+		"docker.io/envoyproxy/ai-gateway-extproc:latest", false, nil, true)
 
 	const gwNamespace = "ns"
 	// Two routes with different CreationTimestamp for deterministic order.