Update workload network when ns network label has changed (istio#47574)

* update workload network when ns network label has changed * fix based on comments * fix not using discoverySelector * fix race * use discoveryselector members * add releasenotes * fix tests * update releasenotes * Apply suggestions from code review Co-authored-by: Lin Sun <lin.sun@solo.io> * Update releasenotes/notes/47574.yaml --------- Co-authored-by: Lin Sun <lin.sun@solo.io>
WillardHu · Nov 22, 2023 · 97fc0a1 · 97fc0a1
1 parent 3fb1bd2
commit 97fc0a1
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 0 deletions.
diff --git a/pilot/pkg/serviceregistry/kube/controller/ambientindex.go b/pilot/pkg/serviceregistry/kube/controller/ambientindex.go
@@ -955,6 +955,20 @@ func (c *Controller) AdditionalPodSubscriptions(
 	return shouldSubscribe
 }
 
+// syncAllWorkloadsForAmbient refreshes all ambient workloads.
+func (c *Controller) syncAllWorkloadsForAmbient() {
+	if c.ambientIndex != nil {
+		var namespaces []string
+		if c.opts.DiscoveryNamespacesFilter != nil {
+			namespaces = c.opts.DiscoveryNamespacesFilter.GetMembers().UnsortedList()
+		}
+		for _, ns := range namespaces {
+			pods := c.podsClient.List(ns, klabels.Everything())
+			c.ambientIndex.HandleSelectedNamespace(ns, pods, c)
+		}
+	}
+}
+
 func workloadNameAndType(pod *v1.Pod) (string, workloadapi.WorkloadType) {
 	objMeta, typeMeta := kubeutil.GetDeployMetaFromPod(pod)
 	switch typeMeta.Kind {

diff --git a/pilot/pkg/serviceregistry/kube/controller/ambientindex_test.go b/pilot/pkg/serviceregistry/kube/controller/ambientindex_test.go
@@ -986,8 +986,10 @@ func newAmbientTestServer(t *testing.T, clusterID cluster.ID, networkID network.
 		CRDs:             []schema.GroupVersionResource{gvr.KubernetesGateway},
 		ConfigController: cfg,
 		MeshWatcher:      mesh.NewFixedWatcher(&meshconfig.MeshConfig{RootNamespace: systemNS}),
+		NetworksWatcher:  mesh.NewFixedNetworksWatcher(nil),
 		ClusterID:        clusterID,
 		ConfigCluster:    true,
+		SystemNamespace:  systemNS,
 	})
 	controller.network = networkID
 	pc := clienttest.Wrap(t, controller.podsClient)

diff --git a/pilot/pkg/serviceregistry/kube/controller/fake.go b/pilot/pkg/serviceregistry/kube/controller/fake.go
@@ -52,6 +52,7 @@ type FakeControllerOptions struct {
 	SkipRun                   bool
 	ConfigController          model.ConfigStoreController
 	ConfigCluster             bool
+	SystemNamespace           string
 }
 
 type FakeController struct {
@@ -92,6 +93,7 @@ func NewFakeControllerWithOptions(t test.Failer, opts FakeControllerOptions) (*F
 		MeshServiceController:     meshServiceController,
 		ConfigCluster:             opts.ConfigCluster,
 		ConfigController:          opts.ConfigController,
+		SystemNamespace:           opts.SystemNamespace,
 	}
 	c := NewController(opts.Client, options)
 	meshServiceController.AddRegistry(c)

diff --git a/pilot/pkg/serviceregistry/kube/controller/network.go b/pilot/pkg/serviceregistry/kube/controller/network.go
@@ -157,6 +157,11 @@ func (c *Controller) onNetworkChange() {
 		log.Errorf("one or more errors force-syncing endpoints: %v", err)
 	}
 	c.reloadNetworkGateways()
+	// This is to ensure the ambient workloads are updated dynamically, aligning them with the current network settings.
+	// With this, the pod do not need to restart when the network configuration changes.
+	if features.EnableAmbientControllers {
+		c.syncAllWorkloadsForAmbient()
+	}
 }
 
 // reloadMeshNetworks will read the mesh networks configuration to setup

diff --git a/pilot/pkg/serviceregistry/kube/controller/network_test.go b/pilot/pkg/serviceregistry/kube/controller/network_test.go
@@ -37,7 +37,9 @@ import (
 	"istio.io/istio/pkg/kube/kclient"
 	"istio.io/istio/pkg/kube/kclient/clienttest"
 	"istio.io/istio/pkg/test"
+	"istio.io/istio/pkg/test/util/assert"
 	"istio.io/istio/pkg/test/util/retry"
+	"istio.io/istio/pkg/util/sets"
 )
 
 func TestNetworkUpdateTriggers(t *testing.T) {
@@ -229,3 +231,80 @@ func addMeshNetworksFromRegistryGateway(t *testing.T, c *FakeController, watcher
 		},
 	}})
 }
+
+func TestSyncAllWorkloadsFromAmbient(t *testing.T) {
+	test.SetForTest(t, &features.EnableAmbientControllers, true)
+
+	s := newAmbientTestServer(t, testC, "")
+
+	s.addPods(t, "127.0.0.1", "pod1", "sa1", map[string]string{"app": "a"}, nil, true, corev1.PodRunning)
+	s.assertAddresses(t, s.addrXdsName("127.0.0.1"), "pod1")
+
+	s.addPods(t, "127.0.0.2", "pod2", "sa2", map[string]string{"app": "a"}, nil, true, corev1.PodRunning)
+	s.assertAddresses(t, s.addrXdsName("127.0.0.2"), "pod2")
+
+	createOrUpdateNamespace(t, s.controller, testNS, "")
+	createOrUpdateNamespace(t, s.controller, systemNS, "")
+
+	expectWorkloadNetwork := func(t *testing.T, c *FakeController, network string) {
+		podNames := sets.New[string]("pod1", "pod2")
+		addresses := c.ambientIndex.All()
+		for _, addr := range addresses {
+			wl := addr.GetWorkload()
+			if wl == nil {
+				continue
+			}
+			if !podNames.Contains(wl.Name) {
+				continue
+			}
+			assert.Equal(t, network, addr.GetWorkload().Network)
+		}
+	}
+
+	t.Run("change namespace network to nw1", func(t *testing.T) {
+		createOrUpdateNamespace(t, s.controller, systemNS, "nw1")
+		retry.UntilSuccessOrFail(t, func() error {
+			if s.controller.networkFromSystemNamespace() != "nw1" {
+				return fmt.Errorf("network not updated")
+			}
+			return nil
+		}, retry.Timeout(5*time.Second), retry.Delay(10*time.Millisecond))
+		expectWorkloadNetwork(t, s.controller, "nw1")
+	})
+
+	t.Run("change namespace network to nw2", func(t *testing.T) {
+		createOrUpdateNamespace(t, s.controller, systemNS, "nw2")
+		retry.UntilSuccessOrFail(t, func() error {
+			if s.controller.networkFromSystemNamespace() != "nw2" {
+				return fmt.Errorf("network not updated")
+			}
+			return nil
+		}, retry.Timeout(5*time.Second), retry.Delay(10*time.Millisecond))
+		expectWorkloadNetwork(t, s.controller, "nw2")
+	})
+
+	t.Run("manually change namespace network to nw3, and update meshNetworks", func(t *testing.T) {
+		s.controller.setNetworkFromNamespace(&corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: systemNS,
+				Labels: map[string]string{
+					label.TopologyNetwork.Name: "nw3",
+				},
+			},
+		})
+		addMeshNetworksFromRegistryGateway(t, s.controller, s.controller.meshNetworksWatcher)
+		expectWorkloadNetwork(t, s.controller, "nw3")
+	})
+}
+
+func createOrUpdateNamespace(t *testing.T, c *FakeController, name, network string) {
+	namespace := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+			Labels: map[string]string{
+				label.TopologyNetwork.Name: network,
+			},
+		},
+	}
+	clienttest.Wrap(t, c.namespaces).CreateOrUpdate(namespace)
+}
diff --git a/releasenotes/notes/47574.yaml b/releasenotes/notes/47574.yaml
@@ -0,0 +1,9 @@
+apiVersion: release-notes/v2
+kind: feature
+area: traffic-management
+releaseNotes:
+- |
+  **Added** support for automatically set default network to Ambient workloads if they are added to the Ambient before the network topology is set.
+  Before, when you set `topology.istio.io/network` on your Istio root namespace, you need to manually rollout the Ambient workloads to make the network change take effect. 
+  Now, the network of Ambient workloads will be automatically updated even if they do not have a network label.
+  Note that if your Ztunnel is not in the same network as what you set in the `topology.istio.io/network` label in your Istio root namespace, your Ambient workloads will not be able to communicate with each other.