From 6997d8ec3bc52de175e4e395a50bf84d6f3733c3 Mon Sep 17 00:00:00 2001
From: l1b0k <libokang.lbk@alibaba-inc.com>
Date: Mon, 13 Jan 2025 19:00:48 +0800
Subject: [PATCH] policy: fix legacy args check

Signed-off-by: l1b0k <libokang.lbk@alibaba-inc.com>
---
 cmd/terway-cli/policy.go      | 36 ++++++++++++++++++++-
 cmd/terway-cli/policy_test.go | 59 ++++++++++++++++++++++++++++++++++-
 2 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/cmd/terway-cli/policy.go b/cmd/terway-cli/policy.go
index f8504b97..189811db 100644
--- a/cmd/terway-cli/policy.go
+++ b/cmd/terway-cli/policy.go
@@ -230,6 +230,12 @@ func runCilium(cfg *PolicyConfig) error {
 	}
 
 	args = append(args, extraArgs...)
+
+	args, err = mutateCiliumArgs(args)
+	if err != nil {
+		return err
+	}
+
 	env := os.Environ()
 	binary, err := exec.LookPath("cilium-agent")
 	if err != nil {
@@ -298,7 +304,7 @@ func policyConfig(container *gabs.Container) ([]string, error) {
 			}
 			return should
 		}
-		return false
+		return true
 	})
 
 	return ciliumArgs, err
@@ -358,6 +364,34 @@ func runSocat(cfg *PolicyConfig) error {
 	return syscall.Exec(binary, args, env)
 }
 
+func mutateCiliumArgs(in []string) ([]string, error) {
+	var err error
+	hasLegacy := false
+
+	args := lo.Filter(in, func(item string, index int) bool {
+		if strings.Contains(item, "disable-per-package-lb") {
+			var innerErr error
+			hasLegacy, innerErr = shouldAppend()
+			if innerErr != nil {
+				err = innerErr
+			}
+
+			return hasLegacy
+		}
+		return true
+	})
+	if err != nil {
+		return nil, err
+	}
+	if hasLegacy {
+		args = lo.Filter(args, func(item string, index int) bool {
+			return !strings.Contains(item, "datapath-mode=veth")
+		})
+	}
+
+	return args, nil
+}
+
 // shouldAppend check whether disable-per-package-lb should be appended
 func shouldAppend() (bool, error) {
 	out, err := readFunc("/var/run/cilium/state/globals/node_config.h")
diff --git a/cmd/terway-cli/policy_test.go b/cmd/terway-cli/policy_test.go
index fa3c40ed..1659aa0e 100644
--- a/cmd/terway-cli/policy_test.go
+++ b/cmd/terway-cli/policy_test.go
@@ -126,7 +126,7 @@ func Test_policyConfig(t *testing.T) {
       "capabilities": {
         "bandwidth": true
       },
-      "cilium_args": "disable-per-package-lb=true",
+      "cilium_args": "disable-per-package-lb=true --other=false",
       "eniip_virtual_type": "datapathv2",
       "network_policy_provider": "ebpf",
       "type": "terway"
@@ -147,6 +147,7 @@ func Test_policyConfig(t *testing.T) {
 			checkFunc: func(t *testing.T, strings []string, err error) {
 				assert.NoError(t, err)
 				assert.NotContains(t, strings, "--disable-per-package-lb=true")
+				assert.Contains(t, strings, "--other=false")
 			},
 		},
 	}
@@ -158,3 +159,59 @@ func Test_policyConfig(t *testing.T) {
 		})
 	}
 }
+
+func Test_mutateCiliumArgs(t *testing.T) {
+	tests := []struct {
+		name     string
+		args     []string
+		want     []string
+		readFunc func(name string) ([]byte, error)
+		wantErr  assert.ErrorAssertionFunc
+	}{
+		{
+			name: "not found",
+			args: []string{
+				"cilium-agent",
+				"--cni-chaining-mode=terway-chainer",
+				"--datapath-mode=veth",
+			},
+			want: []string{
+				"cilium-agent",
+				"--cni-chaining-mode=terway-chainer",
+				"--datapath-mode=veth",
+			},
+			readFunc: func(name string) ([]byte, error) {
+				return nil, os.ErrNotExist
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "exists should not enable veth datapath",
+			args: []string{
+				"cilium-agent",
+				"--cni-chaining-mode=terway-chainer",
+				"--datapath-mode=veth",
+				"--disable-per-package-lb",
+			},
+			want: []string{
+				"cilium-agent",
+				"--cni-chaining-mode=terway-chainer",
+				"--disable-per-package-lb",
+			},
+			readFunc: func(name string) ([]byte, error) {
+				return []byte("#define DIRECT_ROUTING_DEV_IFINDEX 0\n#define DISABLE_PER_PACKET_LB 1\n#define EGRESS_POLICY_MAP cilium_egress_gw_policy_v4\n#define EGRESS_POLICY_MAP_SIZE 16384\n#define ENABLE_BANDWIDTH_MANAGER 1"), nil
+			},
+			wantErr: assert.NoError,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			readFunc = tt.readFunc
+			got, err := mutateCiliumArgs(tt.args)
+			if !tt.wantErr(t, err, fmt.Sprintf("shouldAppend()")) {
+				return
+			}
+			assert.Equalf(t, tt.want, got, "shouldAppend()")
+		})
+	}
+}