Skip to content

Commit

Permalink
policy: fix legacy args check
Browse files Browse the repository at this point in the history
Signed-off-by: l1b0k <libokang.lbk@alibaba-inc.com>
  • Loading branch information
l1b0k committed Jan 15, 2025
1 parent b2e1146 commit 6997d8e
Showing 2 changed files with 93 additions and 2 deletions.
36 changes: 35 additions & 1 deletion cmd/terway-cli/policy.go
Original file line number Diff line number Diff line change
@@ -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")
59 changes: 58 additions & 1 deletion cmd/terway-cli/policy_test.go
Original file line number Diff line number Diff line change
@@ -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()")
})
}
}

0 comments on commit 6997d8e

Please sign in to comment.