From d8282804f847daf2b105b7fd7f58914e0fa36085 Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Fri, 24 May 2024 21:43:53 +0900 Subject: [PATCH] Call GC command with valid attachments from multus cache This code changes CNI's GC command argument. Previously it just passes from parent CNI runtime, however, it may causes unexpected resource deletion if one CNI plugin is used in both cluster network and net-attach-def. This change generates valid attachments from multus CNI cache and passed to delegate CNI plugin. --- pkg/multus/multus.go | 53 +++++++++++++++++++++++++++----- pkg/multus/multus_cni020_test.go | 14 --------- pkg/multus/multus_cni100_test.go | 43 +++++++++++++++++++++----- 3 files changed, 81 insertions(+), 29 deletions(-) diff --git a/pkg/multus/multus.go b/pkg/multus/multus.go index b2f72c96e..19d91d655 100644 --- a/pkg/multus/multus.go +++ b/pkg/multus/multus.go @@ -131,15 +131,49 @@ func saveDelegates(containerID, dataDir string, delegates []*types.DelegateNetCo return err } -func deleteDelegates(containerID, dataDir string) error { - logging.Debugf("deleteDelegates: %s, %s", containerID, dataDir) +func getValidAttachmentFromCache(b []byte) (string, string, error) { + type simpleCacheV1 struct { + Kind string `json:"kind"` + ContainerID string `json:"containerId"` + IfName string `json:"ifName"` + } - path := filepath.Join(dataDir, containerID) - if err := os.Remove(path); err != nil { - return logging.Errorf("deleteDelegates: error in deleting the delegates : %v", err) + cache := &simpleCacheV1{} + if err := json.Unmarshal(b, cache); err != nil { + return "", "", fmt.Errorf("getValidAttachmentFromCache: invalid json: %v", err) } - return nil + if cache.ContainerID == "" || cache.IfName == "" { + return "", "", fmt.Errorf("invalid cache: containerID:%q, ifName:%q", cache.ContainerID, cache.IfName) + } + + return cache.ContainerID, cache.IfName, nil +} + +func gatherValidAttachmentsFromCache(cniDir string) ([]cnitypes.GCAttachment, error) { + cacheDir := filepath.Join(cniDir, "results") + dirEntries, err := os.ReadDir(cacheDir) + if err != nil { + return nil, err + } + + allAttachments := []cnitypes.GCAttachment{} + for _, dirEnt := range dirEntries { + path := filepath.Join(cacheDir, dirEnt.Name()) + delegatesBytes, err := os.ReadFile(path) + // if delegates cannot read that, skipped for now (because cannot recover). + if err != nil { + logging.Errorf("gatherSavedDelegates: cannot read %q, skipped to add", path) + continue + } + containerID, ifName, err := getValidAttachmentFromCache(delegatesBytes) + if err != nil { + logging.Errorf("gatherSavedDelegates: cannot read cache, skipped to add: %v", err) + continue + } + allAttachments = append(allAttachments, cnitypes.GCAttachment{ContainerID: containerID, IfName: ifName}) + } + return allAttachments, nil } func validateIfName(nsname string, ifname string) error { @@ -1009,8 +1043,13 @@ func CmdGC(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) err return logging.Errorf("error in converting the raw bytes to conf: %v", err) } + validAttachments, err := gatherValidAttachmentsFromCache(n.CNIDir) + if err != nil { + return logging.Errorf("error in gather valid attachments: %v", err) + } + err = cniNet.GCNetworkList(context.TODO(), conf, &libcni.GCArgs{ - ValidAttachments: n.ValidAttachments, + ValidAttachments: validAttachments, }) if err != nil { return logging.Errorf("error in GC command: %v", err) diff --git a/pkg/multus/multus_cni020_test.go b/pkg/multus/multus_cni020_test.go index 5a1f271f5..b6863a610 100644 --- a/pkg/multus/multus_cni020_test.go +++ b/pkg/multus/multus_cni020_test.go @@ -43,20 +43,6 @@ var _ = Describe("multus operations", func() { err := saveScratchNetConf("123456789", "", meme) Expect(err).To(HaveOccurred()) }) - - It("fails to delete delegates with bad filepath", func() { - err := deleteDelegates("123456789", "bad!file!~?Path$^") - Expect(err).To(HaveOccurred()) - }) - - It("delete delegates given good filepath", func() { - os.MkdirAll("/opt/cni/bin", 0755) - d1 := []byte("blah") - os.WriteFile("/opt/cni/bin/123456789", d1, 0644) - - err := deleteDelegates("123456789", "/opt/cni/bin") - Expect(err).NotTo(HaveOccurred()) - }) }) var _ = Describe("multus operations cniVersion 0.2.0 config", func() { diff --git a/pkg/multus/multus_cni100_test.go b/pkg/multus/multus_cni100_test.go index 633d72a32..cc809170c 100644 --- a/pkg/multus/multus_cni100_test.go +++ b/pkg/multus/multus_cni100_test.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "os" + "path/filepath" "reflect" "sync" "time" @@ -1309,15 +1310,30 @@ var _ = Describe("multus operations cniVersion 1.1.0 config", func() { }) It("executes delegates with CNI GC", func() { + tmpCNIDir := tmpDir + "/cniData" + err := os.Mkdir(tmpCNIDir, 0777) + Expect(err).NotTo(HaveOccurred()) + + cniCacheDir := filepath.Join(tmpCNIDir, "/results") + err = os.Mkdir(cniCacheDir, 0777) + Expect(err).NotTo(HaveOccurred()) + + //create fake cniResult file + err = os.WriteFile(filepath.Join(cniCacheDir, "cbr0-3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755-eth0"), []byte(`{"kind":"cniCacheV1","containerId":"3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755","config":"eyJjbmlWZXJzaW9uIjoiMC4zLjEiLCJuYW1lIjoiY2JyMCIsInBsdWdpbnMiOlt7ImNhcGFiaWxpdGllcyI6eyJpby5rdWJlcm5ldGVzLmNyaS5wb2QtYW5ub3RhdGlvbnMiOnRydWV9LCJkZWxlZ2F0ZSI6eyJoYWlycGluTW9kZSI6dHJ1ZSwiaXNEZWZhdWx0R2F0ZXdheSI6dHJ1ZX0sInR5cGUiOiJmbGFubmVsIn0seyJjYXBhYmlsaXRpZXMiOnsicG9ydE1hcHBpbmdzIjp0cnVlfSwidHlwZSI6InBvcnRtYXAifV19","ifName":"eth0","networkName":"cbr0","netns":"/var/run/netns/8b8677c8-8929-4746-8206-514069760f6e","cniArgs":[["IgnoreUnknown","true"],["K8S_POD_NAMESPACE","default"],["K8S_POD_NAME","macvlan"],["K8S_POD_INFRA_CONTAINER_ID","3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755"],["K8S_POD_UID","f0bfbd5b-096d-48ef-998c-da26743dd0cb"],["IgnoreUnknown","1"],["K8S_POD_NAMESPACE","default"],["K8S_POD_NAME","macvlan"],["K8S_POD_INFRA_CONTAINER_ID","3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755"],["K8S_POD_UID","f0bfbd5b-096d-48ef-998c-da26743dd0cb"]],"result":{"cniVersion":"0.3.1","dns":{},"interfaces":[{"mac":"ea:19:25:a2:a1:93","name":"cni0"},{"mac":"ba:76:61:2f:8b:ca","name":"vethc42d3d18"},{"mac":"7e:57:6a:9b:6b:b5","name":"eth0","sandbox":"/var/run/netns/8b8677c8-8929-4746-8206-514069760f6e"}],"ips":[{"address":"10.244.1.4/24","gateway":"10.244.1.1","interface":2,"version":"4"}],"routes":[{"dst":"10.244.0.0/16"},{"dst":"0.0.0.0/0","gw":"10.244.1.1"}]}}`), 0666) + Expect(err).NotTo(HaveOccurred()) + err = os.WriteFile(filepath.Join(cniCacheDir, "macvlan-conf-1-3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755-net1"), []byte(`{"kind":"cniCacheV1","containerId":"3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755","config":"eyJjbmlWZXJzaW9uIjoiMC4zLjEiLCJpcGFtIjp7ImFkZHJlc3NlcyI6W3siYWRkcmVzcyI6IjEwLjEuMS4xMDEvMjQifV0sInR5cGUiOiJzdGF0aWMifSwibWFzdGVyIjoiZXRoMSIsIm1vZGUiOiJicmlkZ2UiLCJuYW1lIjoibWFjdmxhbi1jb25mLTEiLCJ0eXBlIjoibWFjdmxhbiJ9","ifName":"net1","networkName":"macvlan-conf-1","netns":"/var/run/netns/8b8677c8-8929-4746-8206-514069760f6e","cniArgs":[["IgnoreUnknown","true"],["K8S_POD_NAMESPACE","default"],["K8S_POD_NAME","macvlan"],["K8S_POD_INFRA_CONTAINER_ID","3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755"],["K8S_POD_UID","f0bfbd5b-096d-48ef-998c-da26743dd0cb"],["IgnoreUnknown","1"],["K8S_POD_NAMESPACE","default"],["K8S_POD_NAME","macvlan"],["K8S_POD_INFRA_CONTAINER_ID","3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755"],["K8S_POD_UID","f0bfbd5b-096d-48ef-998c-da26743dd0cb"]],"result":{"cniVersion":"0.3.1","dns":{},"interfaces":[{"mac":"36:b3:c5:29:ad:b8","name":"net1","sandbox":"/var/run/netns/8b8677c8-8929-4746-8206-514069760f6e"}],"ips":[{"address":"10.1.1.101/24","interface":0,"version":"4"}]}}`), 0666) + Expect(err).NotTo(HaveOccurred()) + args := &skel.CmdArgs{ ContainerID: "123456789", Netns: testNS.Path(), IfName: "eth0", - StdinData: []byte(`{ + StdinData: []byte(fmt.Sprintf(`{ "name": "node-cni-network", "type": "multus", "defaultnetworkfile": "/tmp/foo.multus.conf", "defaultnetworkwaitseconds": 3, + "cniDir": "%s", "delegates": [{ "name": "weave1", "cniVersion": "1.1.0", @@ -1331,23 +1347,34 @@ var _ = Describe("multus operations cniVersion 1.1.0 config", func() { "type": "other-plugin" }] }] - }`), + }`, tmpCNIDir)), } logging.SetLogLevel("verbose") fExec := newFakeExec() expectedConf1 := `{ - "cni.dev/valid-attachments": null, - "name": "weave1", - "cniVersion": "1.1.0", - "type": "weave-net" - }` + "cni.dev/valid-attachments": [ + { + "containerID": "3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755", + "ifname": "eth0" + }, + { + "containerID": "3f6940ab5ab43bc522569d15b23f8c1bbde1d7678b080398506924fc01d72755", + "ifname": "net1" + } + ], + "name": "weave1", + "cniVersion": "1.1.0", + "type": "weave-net" + }` fExec.addPlugin100(nil, "", expectedConf1, nil, nil) - err := CmdGC(args, fExec, nil) + err = CmdGC(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) // we only execute once for cluster network, not additional one Expect(fExec.gcIndex).To(Equal(1)) + err = os.RemoveAll(tmpCNIDir) + Expect(err).NotTo(HaveOccurred()) }) })