add meshconfig reconciliation to gateway controllers (#57893) (#57915)

* add meshconfig reconciliation to gateway controllers



* refactor test helper for lint



---------

Signed-off-by: Lucas Copi <[email protected]>

Author: lcopi

pilot/pkg/config/kube/gateway/deploymentcontroller.go

@@ -331,6 +331,14 @@ func NewDeploymentController(
 		}
 	}))
 
+	dc.env.Watcher.AddMeshHandler(func() {
+		// if there is a change to the meshconfig we need to reprocess all gateways
+		// to reconcile things like the deployment image
+		for _, gw := range dc.gateways.List(corev1.NamespaceAll, klabels.Everything()) {
+			dc.queue.AddObject(gw)
+		}
+	})
+
 	// we check if the generation has changed on a gateway, or if the annotations or labels have been updated
 	// reconciliation is expensive, so status updates for attachedRoutes can be skipped.
 
@@ -403,6 +411,7 @@ func (d *DeploymentController) Run(stop <-chan struct{}) {
 		d.gateways.HasSynced,
 		d.gatewayClasses.HasSynced,
 		d.tagWatcher.HasSynced,
+		d.env.Watcher.AsCollection().HasSynced,
 	)
 	d.queue.Run(stop)
 	controllers.ShutdownAll(

pilot/pkg/config/kube/gateway/deploymentcontroller_test.go

@@ -46,6 +46,7 @@ import (
 	"istio.io/istio/pkg/config"
 	"istio.io/istio/pkg/config/constants"
 	"istio.io/istio/pkg/config/mesh"
+	"istio.io/istio/pkg/config/mesh/meshwatcher"
 	"istio.io/istio/pkg/config/schema/gvk"
 	"istio.io/istio/pkg/config/schema/gvr"
 	"istio.io/istio/pkg/kube"
@@ -574,7 +575,7 @@ metadata:
 			kclient.NewWriteClient[*k8sbeta.Gateway](client).Create(tt.gw.DeepCopy())
 			kclient.NewWriteClient[*appsv1.Deployment](client).Create(upgradeDeployment)
 			stop := test.NewStop(t)
-			env := model.NewEnvironment()
+			env := newTestEnv()
 			env.PushContext().ProxyConfigs = tt.pcs
 			tw := revisions.NewTagWatcher(client, "")
 			go tw.Run(stop)
@@ -612,6 +613,79 @@ metadata:
 	}
 }
 
+func TestMeshGatewayReconciliation(t *testing.T) {
+	c := kube.NewFakeClient(&corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "default",
+		},
+	})
+	tw := revisions.NewTagWatcher(c, "default")
+
+	// not using the new helper becase
+	// we need to explicitly modify this
+	// later in the thread
+	env := model.NewEnvironment()
+	m := mesh.DefaultMeshConfig()
+	watch := meshwatcher.NewTestWatcher(m)
+	env.Watcher = watch
+	d := NewDeploymentController(c, "", env, testInjectionConfig(t, ""), func(fn func()) {}, tw, "", "")
+
+	reconciles := atomic.NewInt32(0)
+	wantReconcile := int32(0)
+	expectReconciled := func() {
+		t.Helper()
+		wantReconcile++
+		assert.EventuallyEqual(t, reconciles.Load, wantReconcile, retry.Timeout(time.Second*5), retry.Message("no reconciliation"))
+	}
+
+	writes := make(chan string, 2)
+	d.patcher = func(g schema.GroupVersionResource, name string, namespace string, data []byte, subresources ...string) error {
+		if g == gvr.Service {
+			reconciles.Inc()
+		}
+		if g == gvr.KubernetesGateway {
+			b, err := yaml.JSONToYAML(data)
+			if err != nil {
+				return err
+			}
+			writes <- string(b)
+		}
+		return nil
+	}
+
+	stop := test.NewStop(t)
+	gws := clienttest.Wrap(t, d.gateways)
+	go tw.Run(stop)
+	go d.Run(stop)
+	c.RunAndWait(stop)
+	kube.WaitForCacheSync("test", stop, d.queue.HasSynced)
+
+	// create the initial gateway for the deployment controller
+	// to reconcile when the meshconfig changes
+	defaultGateway := &k8sbeta.Gateway{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "gw",
+			Namespace: "default",
+		},
+		Spec: k8s.GatewaySpec{
+			GatewayClassName: k8s.ObjectName(features.GatewayAPIDefaultGatewayClass),
+		},
+	}
+	gws.Create(defaultGateway)
+	assert.Equal(t, assert.ChannelHasItem(t, writes), buildPatch())
+	expectReconciled()
+	assert.ChannelIsEmpty(t, writes)
+
+	// modify the meshconfig to trigger a reprocess of the gateways
+	m.DefaultConfig.Image = &istioio_networking_v1beta1.ProxyImage{ImageType: "distroless"}
+	watch.Set(m)
+
+	// confirm the gateway is reconciled
+	assert.ChannelHasItem(t, writes)
+	expectReconciled()
+	assert.ChannelIsEmpty(t, writes)
+}
+
 func buildFilter(allowedNamespace string) kubetypes.DynamicObjectFilter {
 	return kubetypes.NewStaticObjectFilter(func(obj any) bool {
 		if ns, ok := obj.(string); ok {
@@ -638,7 +712,7 @@ func TestVersionManagement(t *testing.T) {
 		},
 	})
 	tw := revisions.NewTagWatcher(c, "default")
-	env := &model.Environment{}
+	env := newTestEnv()
 	d := NewDeploymentController(c, "", env, testInjectionConfig(t, ""), func(fn func()) {}, tw, "", "")
 	reconciles := atomic.NewInt32(0)
 	wantReconcile := int32(0)
@@ -678,7 +752,7 @@ func TestVersionManagement(t *testing.T) {
 		},
 	}
 	gws.Create(defaultGateway)
-	assert.Equal(t, assert.ChannelHasItem(t, writes), buildPatch(ControllerVersion))
+	assert.Equal(t, assert.ChannelHasItem(t, writes), buildPatch())
 	expectReconciled()
 	assert.ChannelIsEmpty(t, writes)
 	// Test fake doesn't actual do Apply, so manually do this
@@ -699,7 +773,7 @@ func TestVersionManagement(t *testing.T) {
 	defaultGateway.Annotations = map[string]string{}
 	gws.Update(defaultGateway)
 	expectReconciled()
-	assert.Equal(t, assert.ChannelHasItem(t, writes), buildPatch(ControllerVersion))
+	assert.Equal(t, assert.ChannelHasItem(t, writes), buildPatch())
 	assert.ChannelIsEmpty(t, writes)
 	// Test fake doesn't actual do Apply, so manually do this
 	defaultGateway.Annotations = map[string]string{ControllerVersionAnnotation: fmt.Sprint(ControllerVersion)}
@@ -712,7 +786,7 @@ func TestVersionManagement(t *testing.T) {
 	defaultGateway.Annotations = map[string]string{ControllerVersionAnnotation: fmt.Sprint(1)}
 	gws.Update(defaultGateway)
 	expectReconciled()
-	assert.Equal(t, assert.ChannelHasItem(t, writes), buildPatch(ControllerVersion))
+	assert.Equal(t, assert.ChannelHasItem(t, writes), buildPatch())
 	assert.ChannelIsEmpty(t, writes)
 	// Test fake doesn't actual do Apply, so manually do this
 	defaultGateway.Annotations = map[string]string{ControllerVersionAnnotation: fmt.Sprint(ControllerVersion)}
@@ -995,7 +1069,7 @@ func TestHandlerEnqueueFunction(t *testing.T) {
 			client.Kube().Discovery().(*fakediscovery.FakeDiscovery).FakedServerVersion = &kubeVersion.Info{Major: "1", Minor: "28"}
 
 			tw := revisions.NewTagWatcher(client, "")
-			env := model.NewEnvironment()
+			env := newTestEnv()
 			go tw.Run(stop)
 
 			d := NewDeploymentController(client, cluster.ID(features.ClusterName), env, dummyWebHookInjectFn, func(fn func()) {
@@ -1075,11 +1149,22 @@ global:
 	return injConfig
 }
 
-func buildPatch(version int) string {
+// buildPatch used to accept a version
+// but lint flagged that ControllerVersion
+// was always used so the function has been
+// adjusted to hardcode ControllerVersion
+func buildPatch() string {
 	return fmt.Sprintf(`apiVersion: gateway.networking.k8s.io/v1beta1
 kind: Gateway
 metadata:
   annotations:
     gateway.istio.io/controller-version: "%d"
-`, version)
+`, ControllerVersion)
+}
+
+func newTestEnv() *model.Environment {
+	env := model.NewEnvironment()
+	env.Watcher = meshwatcher.NewTestWatcher(mesh.DefaultMeshConfig())
+
+	return env
 }

releasenotes/notes/57890.yaml

@@ -0,0 +1,10 @@
+apiVersion: release-notes/v2
+kind: bug-fix
+area: installation
+
+issue:
+ - https://github.com/istio/istio/issues/57890
+
+releaseNotes:
+- |
+  **Fixed** missing gateway reconciliation for meshconfig changes