fix the per package lb flag

Signed-off-by: l1b0k <[email protected]>

cmd/terway-cli/cni_linux.go

@@ -63,15 +63,3 @@ func allowEBPFNetworkPolicy(require bool) (bool, error) {
 func checkKernelVersion(k, major, minor int) bool {
 	return kernel.CheckKernelVersion(k, major, minor)
 }
-
-func isOldNode() (bool, error) {
-	_, err := netlink.LinkByName("cilium_net")
-	if err == nil {
-		fmt.Printf("link cilium_net exist\n")
-		return true, nil
-	}
-	if !errors.As(err, &netlink.LinkNotFoundError{}) {
-		return false, err
-	}
-	return false, nil
-}

cmd/terway-cli/policy.go

@@ -16,6 +16,8 @@ import (
 	"github.com/AliyunContainerService/terway/types"
 )
 
+var readFunc func(name string) ([]byte, error)
+
 type PolicyConfig struct {
 	Datapath             string
 	EnableNetworkPolicy  bool
@@ -39,6 +41,8 @@ var policyCmd = &cobra.Command{
 	Use:          "policy",
 	SilenceUsage: true,
 	Run: func(cmd *cobra.Command, args []string) {
+		readFunc = os.ReadFile
+
 		err := initPolicy(cmd, args)
 		if err != nil {
 			_, _ = fmt.Fprintf(os.Stderr, "failed to init policy: %v\n", err)
@@ -285,19 +289,19 @@ func policyConfig(container *gabs.Container) ([]string, error) {
 		ciliumArgs = append(ciliumArgs, extractArgs(h.CiliumExtraArgs)...)
 	}
 
-	old, err := isOldNode()
-	if err != nil {
-		return nil, err
-	}
-
-	// check the extra args
-	lo.Filter(ciliumArgs, func(item string, index int) bool {
+	var err error
+	ciliumArgs = lo.Filter(ciliumArgs, func(item string, index int) bool {
 		if strings.Contains(item, "disable-per-package-lb") {
-			return old
+			should, innerErr := shouldAppend()
+			if innerErr != nil {
+				err = innerErr
+			}
+			return should
 		}
-		return true
+		return false
 	})
-	return ciliumArgs, nil
+
+	return ciliumArgs, err
 }
 
 func extractArgs(in string) []string {
@@ -353,3 +357,15 @@ func runSocat(cfg *PolicyConfig) error {
 	}
 	return syscall.Exec(binary, args, env)
 }
+
+// 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")
+	if err != nil {
+		if os.IsNotExist(err) {
+			return false, nil
+		}
+		return false, err
+	}
+	return strings.Contains(string(out), "DISABLE_PER_PACKET_LB"), nil
+}

cmd/terway-cli/policy_test.go

@@ -1,8 +1,11 @@
 package main
 
 import (
+	"fmt"
+	"os"
 	"testing"
 
+	"github.com/Jeffail/gabs/v2"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -29,3 +32,129 @@ func Test_extractArgs(t *testing.T) {
 		})
 	}
 }
+
+func Test_shouldAppend(t *testing.T) {
+	tests := []struct {
+		name     string
+		want     bool
+		readFunc func(name string) ([]byte, error)
+		wantErr  assert.ErrorAssertionFunc
+	}{
+		{
+			name: "not found",
+			want: false,
+			readFunc: func(name string) ([]byte, error) {
+				return nil, os.ErrNotExist
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "exists",
+			want: true,
+			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 := shouldAppend()
+			if !tt.wantErr(t, err, fmt.Sprintf("shouldAppend()")) {
+				return
+			}
+			assert.Equalf(t, tt.want, got, "shouldAppend()")
+		})
+	}
+}
+
+func Test_policyConfig(t *testing.T) {
+	type args struct {
+		container *gabs.Container
+	}
+	tests := []struct {
+		name      string
+		args      args
+		readFunc  func(name string) ([]byte, error)
+		checkFunc func(*testing.T, []string, error)
+	}{
+		{
+			name: "per-package-lb should exist",
+			args: args{container: func() *gabs.Container {
+				cniJSON, _ := gabs.ParseJSON([]byte(`{
+  "cniVersion": "0.4.0",
+  "name": "terway-chainer",
+  "plugins": [
+    {
+      "bandwidth_mode": "edt",
+      "capabilities": {
+        "bandwidth": true
+      },
+      "cilium_args": "disable-per-package-lb=true",
+      "eniip_virtual_type": "datapathv2",
+      "network_policy_provider": "ebpf",
+      "type": "terway"
+    },
+    {
+      "data-path": "datapathv2",
+      "enable-debug": false,
+      "log-file": "/var/run/cilium/cilium-cni.log",
+      "type": "cilium-cni"
+    }
+  ]
+}`))
+				return cniJSON
+			}()},
+			readFunc: func(name string) ([]byte, error) {
+				return []byte("#define DIRECT_ROUTING_DEV_IFINDEX 0\n#define DISABLE_PER_PACKET_LB 1\n"), nil
+			},
+			checkFunc: func(t *testing.T, strings []string, err error) {
+				assert.NoError(t, err)
+				assert.Contains(t, strings, "--disable-per-package-lb=true")
+			},
+		},
+		{
+			name: "per-package-lb should exist",
+			args: args{container: func() *gabs.Container {
+				cniJSON, _ := gabs.ParseJSON([]byte(`{
+  "cniVersion": "0.4.0",
+  "name": "terway-chainer",
+  "plugins": [
+    {
+      "bandwidth_mode": "edt",
+      "capabilities": {
+        "bandwidth": true
+      },
+      "cilium_args": "disable-per-package-lb=true",
+      "eniip_virtual_type": "datapathv2",
+      "network_policy_provider": "ebpf",
+      "type": "terway"
+    },
+    {
+      "data-path": "datapathv2",
+      "enable-debug": false,
+      "log-file": "/var/run/cilium/cilium-cni.log",
+      "type": "cilium-cni"
+    }
+  ]
+}`))
+				return cniJSON
+			}()},
+			readFunc: func(name string) ([]byte, error) {
+				return nil, nil
+			},
+			checkFunc: func(t *testing.T, strings []string, err error) {
+				assert.NoError(t, err)
+				assert.NotContains(t, strings, "--disable-per-package-lb=true")
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			readFunc = tt.readFunc
+			got, err := policyConfig(tt.args.container)
+			tt.checkFunc(t, got, err)
+		})
+	}
+}