From 0b837ec3b7f6704d1efa974e66fead9a5ec6d9be Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Thu, 25 Jul 2024 17:43:28 +0800 Subject: [PATCH 1/2] Fix TestPodSidecarLabelChangeTriggersAddIptablesAction flake Signed-off-by: Zhonghu Xu --- pkg/controller/bypass/bypass_controller.go | 6 +-- pkg/controller/bypass/bypass_test.go | 54 ++++++++++++---------- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/pkg/controller/bypass/bypass_controller.go b/pkg/controller/bypass/bypass_controller.go index 186514dbc..cfea1602b 100644 --- a/pkg/controller/bypass/bypass_controller.go +++ b/pkg/controller/bypass/bypass_controller.go @@ -74,7 +74,7 @@ func NewByPassController(client kubernetes.Interface) *Controller { return } - log.Debugf("%s/%s: enable bypass control", pod.GetNamespace(), pod.GetName()) + log.Infof("%s/%s: bypass sidecar control", pod.GetNamespace(), pod.GetName()) nspath, _ := ns.GetPodNSpath(pod) if err := addIptables(nspath); err != nil { log.Errorf("failed to add iptables rules for %s: %v", nspath, err) @@ -100,7 +100,7 @@ func NewByPassController(client kubernetes.Interface) *Controller { } if shouldBypass(oldPod) && !shouldBypass(newPod) { - log.Debugf("%s/%s: restore sidecar control", newPod.GetNamespace(), newPod.GetName()) + log.Infof("%s/%s: restore sidecar control", newPod.GetNamespace(), newPod.GetName()) nspath, _ := ns.GetPodNSpath(newPod) if err := deleteIptables(nspath); err != nil { log.Errorf("failed to delete iptables rules for %s: %v", nspath, err) @@ -108,7 +108,7 @@ func NewByPassController(client kubernetes.Interface) *Controller { } } if !shouldBypass(oldPod) && shouldBypass(newPod) { - log.Debugf("%s/%s: enable bypass control", newPod.GetNamespace(), newPod.GetName()) + log.Infof("%s/%s: bypass sidecar control", newPod.GetNamespace(), newPod.GetName()) nspath, _ := ns.GetPodNSpath(newPod) if err := addIptables(nspath); err != nil { log.Errorf("failed to add iptables rules for %s: %v", nspath, err) diff --git a/pkg/controller/bypass/bypass_test.go b/pkg/controller/bypass/bypass_test.go index ad178ff73..d03b4124b 100644 --- a/pkg/controller/bypass/bypass_test.go +++ b/pkg/controller/bypass/bypass_test.go @@ -25,24 +25,39 @@ import ( "github.com/agiledragon/gomonkey/v2" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "istio.io/api/annotation" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" ) -func TestPodSidecarLabelChangeTriggersAddIptablesAction(t *testing.T) { - client := fake.NewSimpleClientset() - +func TestBypassController(t *testing.T) { err := os.Setenv("NODE_NAME", "test_node") - require.NoError(t, err) + assert.NoError(t, err) t.Cleanup(func() { os.Unsetenv("NODE_NAME") }) - stopCh := make(<-chan struct{}) + stopCh := make(chan struct{}) + defer close(stopCh) + namespaceName := "default" + namespace := &corev1.Namespace{ + TypeMeta: metav1.TypeMeta{ + Kind: "Pod", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + Labels: map[string]string{ + "istio-injection": "enabled", + }, + }, + } + client := fake.NewSimpleClientset(namespace) c := NewByPassController(client) go c.Run(stopCh) + cache.WaitForCacheSync(stopCh, c.pod.HasSynced) + enabled := atomic.Bool{} disabled := atomic.Bool{} @@ -57,30 +72,18 @@ func TestPodSidecarLabelChangeTriggersAddIptablesAction(t *testing.T) { wg.Done() return nil }) - - patches2 := gomonkey.NewPatches() - defer patches2.Reset() - - patches2.ApplyFunc(deleteIptables, func(ns string) error { + patches1.ApplyFunc(deleteIptables, func(ns string) error { disabled.Store(true) // Signal that addIptables has been called wg.Done() return nil }) - namespaceName := "default" - namespace := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespaceName, - Labels: map[string]string{ - "istio-injection": "enabled", - }, - }, - } - _, err = client.CoreV1().Namespaces().Create(context.TODO(), namespace, metav1.CreateOptions{}) - require.NoError(t, err) - podWithBypassButNoSidecar := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "Pod", + APIVersion: "v1", + }, ObjectMeta: metav1.ObjectMeta{ Name: "test-pod-no-sidecar", Namespace: namespaceName, @@ -93,6 +96,7 @@ func TestPodSidecarLabelChangeTriggersAddIptablesAction(t *testing.T) { }, } + // case 1: pod with bypass label but no sidecar _, err = client.CoreV1().Pods(namespaceName).Create(context.TODO(), podWithBypassButNoSidecar, metav1.CreateOptions{}) assert.NoError(t, err) assert.Equal(t, false, enabled.Load(), "unexpected value for enabled flag") @@ -114,6 +118,7 @@ func TestPodSidecarLabelChangeTriggersAddIptablesAction(t *testing.T) { }, } + // case 2: pod with bypass label and sidecar wg.Add(1) _, err = client.CoreV1().Pods(namespaceName).Create(context.TODO(), podWithBypass, metav1.CreateOptions{}) assert.NoError(t, err) @@ -124,6 +129,7 @@ func TestPodSidecarLabelChangeTriggersAddIptablesAction(t *testing.T) { enabled.Store(false) disabled.Store(false) + // case 3: pod update by removing bypass label // Update pod by removing the bypass label newPod := podWithBypass.DeepCopy() delete(newPod.Labels, ByPassLabel) @@ -136,7 +142,7 @@ func TestPodSidecarLabelChangeTriggersAddIptablesAction(t *testing.T) { enabled.Store(false) disabled.Store(false) - // Update pod by adding the bypass label + // case 4: Update pod by adding the bypass label newPod = podWithBypass.DeepCopy() newPod.Labels[ByPassLabel] = ByPassValue wg.Add(1) From 7fc23fb59754956cd81820455ada01f3404dbfcc Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Thu, 25 Jul 2024 20:29:06 +0800 Subject: [PATCH 2/2] update Signed-off-by: Zhonghu Xu --- pkg/controller/bypass/bypass_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/controller/bypass/bypass_test.go b/pkg/controller/bypass/bypass_test.go index d03b4124b..1d8d1b11d 100644 --- a/pkg/controller/bypass/bypass_test.go +++ b/pkg/controller/bypass/bypass_test.go @@ -29,11 +29,11 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" - "k8s.io/client-go/tools/cache" ) func TestBypassController(t *testing.T) { - err := os.Setenv("NODE_NAME", "test_node") + nodeName := "test_node" + err := os.Setenv("NODE_NAME", nodeName) assert.NoError(t, err) t.Cleanup(func() { os.Unsetenv("NODE_NAME") @@ -55,8 +55,7 @@ func TestBypassController(t *testing.T) { } client := fake.NewSimpleClientset(namespace) c := NewByPassController(client) - go c.Run(stopCh) - cache.WaitForCacheSync(stopCh, c.pod.HasSynced) + c.Run(stopCh) enabled := atomic.Bool{} disabled := atomic.Bool{} @@ -92,7 +91,7 @@ func TestBypassController(t *testing.T) { }, }, Spec: corev1.PodSpec{ - NodeName: "test-node", + NodeName: nodeName, }, } @@ -114,7 +113,7 @@ func TestBypassController(t *testing.T) { }, }, Spec: corev1.PodSpec{ - NodeName: "test-node", + NodeName: nodeName, }, }