From ea389005a1841b5964a2bdd28ab4df74ad8d9a09 Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Thu, 15 Jan 2026 13:04:13 -0500 Subject: [PATCH] Adds support for CNI STATUS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes-Include: - Add STATUS handling for delegate requests and single‑plugin - Invoke STATUS for conf/conflist delegates via libcni - Preserve CNI error codes/messages through daemon API and shim - Add tests for STATUS error propagation Signed-off-by: Tim Rozet --- pkg/multus/multus.go | 118 ++++++++++++++++++++++++++++++- pkg/multus/multus_cni100_test.go | 76 ++++++++++++++++++++ pkg/server/api/api.go | 6 ++ pkg/server/api/shim.go | 5 ++ pkg/server/server.go | 50 +++++++++++-- 5 files changed, 247 insertions(+), 8 deletions(-) diff --git a/pkg/multus/multus.go b/pkg/multus/multus.go index a7941fe62..b6b06ff47 100644 --- a/pkg/multus/multus.go +++ b/pkg/multus/multus.go @@ -18,6 +18,7 @@ package multus import ( "context" "encoding/json" + stderrors "errors" "fmt" "net" "os" @@ -30,6 +31,7 @@ import ( "github.com/containernetworking/cni/pkg/skel" cnitypes "github.com/containernetworking/cni/pkg/types" cni100 "github.com/containernetworking/cni/pkg/types/100" + cniversion "github.com/containernetworking/cni/pkg/version" "github.com/containernetworking/plugins/pkg/ns" nettypes "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" nadutils "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/utils" @@ -258,6 +260,42 @@ func confDel(rt *libcni.RuntimeConf, rawNetconf []byte, multusNetconf *types.Net return err } +func confStatus(rt *libcni.RuntimeConf, rawNetconf []byte, multusNetconf *types.NetConf, exec invoke.Exec) error { + logging.Debugf("confStatus: %v, %s", rt, string(rawNetconf)) + + binDirs := filepath.SplitList(os.Getenv("CNI_PATH")) + binDirs = append([]string{multusNetconf.BinDir}, binDirs...) + cniNet := libcni.NewCNIConfigWithCacheDir(binDirs, multusNetconf.CNIDir, exec) + + conf, err := libcni.ConfFromBytes(rawNetconf) + if err != nil { + return logging.Errorf("error in converting the raw bytes to conf: %v", err) + } + + if gt, _ := cniversion.GreaterThanOrEqualTo(conf.Network.CNIVersion, "1.1.0"); !gt { + logging.Debugf("confStatus: skipping STATUS for network %q type %q cniVersion %q (< 1.1.0)", + conf.Network.Name, conf.Network.Type, conf.Network.CNIVersion) + return nil + } + + confList := &libcni.NetworkConfigList{ + Name: conf.Network.Name, + CNIVersion: conf.Network.CNIVersion, + Plugins: []*libcni.PluginConfig{conf}, + } + + err = cniNet.GetStatusNetworkList(context.Background(), confList) + if err != nil { + var cniErr *cnitypes.Error + if stderrors.As(err, &cniErr) { + return err + } + return logging.Errorf("error in getting result from StatusNetworkList: %v", err) + } + + return err +} + func conflistAdd(rt *libcni.RuntimeConf, rawnetconflist []byte, cniConfList *libcni.NetworkConfigList, multusNetconf *types.NetConf, exec invoke.Exec) (cnitypes.Result, error) { logging.Debugf("conflistAdd: %v, %s", rt, string(rawnetconflist)) // In part, adapted from K8s pkg/kubelet/dockershim/network/cni/cni.go @@ -327,6 +365,33 @@ func conflistDel(rt *libcni.RuntimeConf, rawnetconflist []byte, multusNetconf *t return err } +func conflistStatus(rt *libcni.RuntimeConf, rawnetconflist []byte, multusNetconf *types.NetConf, exec invoke.Exec) error { + logging.Debugf("conflistStatus: %v, %s", rt, string(rawnetconflist)) + + binDirs := filepath.SplitList(os.Getenv("CNI_PATH")) + binDirs = append([]string{multusNetconf.BinDir}, binDirs...) + cniNet := libcni.NewCNIConfigWithCacheDir(binDirs, multusNetconf.CNIDir, exec) + + confList, err := libcni.ConfListFromBytes(rawnetconflist) + if err != nil { + return logging.Errorf("conflistStatus: error converting the raw bytes into a conflist: %v", err) + } + if gt, _ := cniversion.GreaterThanOrEqualTo(confList.CNIVersion, "1.1.0"); !gt { + logging.Debugf("conflistStatus: skipping STATUS for network list %q cniVersion %q (< 1.1.0)", confList.Name, confList.CNIVersion) + } + + err = cniNet.GetStatusNetworkList(context.Background(), confList) + if err != nil { + var cniErr *cnitypes.Error + if stderrors.As(err, &cniErr) { + return err + } + return logging.Errorf("conflistStatus: error in getting result from StatusNetworkList: %v", err) + } + + return err +} + // DelegateAdd ... func DelegateAdd(exec invoke.Exec, kubeClient *k8s.ClientInfo, pod *v1.Pod, delegate *types.DelegateNetConf, rt *libcni.RuntimeConf, multusNetconf *types.NetConf) (cnitypes.Result, error) { logging.Debugf("DelegateAdd: %v, %v, %v", exec, delegate, rt) @@ -456,6 +521,46 @@ func DelegateCheck(exec invoke.Exec, delegateConf *types.DelegateNetConf, rt *li return err } +// DelegateStatus ... +func DelegateStatus(exec invoke.Exec, delegateConf *types.DelegateNetConf, rt *libcni.RuntimeConf, multusNetconf *types.NetConf) error { + logging.Debugf("DelegateStatus: %v, %v, %v", exec, delegateConf, rt) + + isConfList := delegateConf.ConfListPlugin + if !isConfList && delegateConf.Conf.Type == "" && delegateConf.ConfList.Name != "" { + isConfList = true + } + + if logging.GetLoggingLevel() >= logging.VerboseLevel { + var cniConfName string + if isConfList { + cniConfName = delegateConf.ConfList.Name + } else { + cniConfName = delegateConf.Conf.Name + } + logging.Verbosef("Status: %s:%s:%s(%s):%s %s", rt.Args[1][1], rt.Args[2][1], delegateConf.Name, cniConfName, rt.IfName, string(delegateConf.Bytes)) + } + + var err error + if isConfList { + err = conflistStatus(rt, delegateConf.Bytes, multusNetconf, exec) + } else { + err = confStatus(rt, delegateConf.Bytes, multusNetconf, exec) + } + + if err != nil { + var cniErr *cnitypes.Error + if stderrors.As(err, &cniErr) { + return err + } + if isConfList { + return logging.Errorf("DelegateStatus: error invoking ConflistStatus - %q: %v", delegateConf.ConfList.Name, err) + } + return logging.Errorf("DelegateStatus: error invoking ConfStatus - %q: %v", delegateConf.Conf.Type, err) + } + + return err +} + // DelegateDel ... func DelegateDel(exec invoke.Exec, pod *v1.Pod, delegateConf *types.DelegateNetConf, rt *libcni.RuntimeConf, multusNetconf *types.NetConf) error { logging.Debugf("DelegateDel: %v, %v, %v, %v", exec, pod, delegateConf, rt) @@ -1050,17 +1155,26 @@ func CmdStatus(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) // invoke delegate's STATUS command // we only need to check cluster network status + delegate := n.Delegates[0] + if !delegate.ConfListPlugin { + return confStatus(&libcni.RuntimeConf{}, delegate.Bytes, n, exec) + } + binDirs := filepath.SplitList(os.Getenv("CNI_PATH")) binDirs = append([]string{n.BinDir}, binDirs...) cniNet := libcni.NewCNIConfigWithCacheDir(binDirs, n.CNIDir, exec) - conf, err := libcni.ConfListFromBytes(n.Delegates[0].Bytes) + conf, err := libcni.ConfListFromBytes(delegate.Bytes) if err != nil { return logging.Errorf("error in converting the raw bytes to conf: %v", err) } - err = cniNet.GetStatusNetworkList(context.TODO(), conf) + err = cniNet.GetStatusNetworkList(context.Background(), conf) if err != nil { + var cniErr *cnitypes.Error + if stderrors.As(err, &cniErr) { + return err + } return logging.Errorf("error in STATUS command: %v", err) } diff --git a/pkg/multus/multus_cni100_test.go b/pkg/multus/multus_cni100_test.go index f5a520af6..cf164d088 100644 --- a/pkg/multus/multus_cni100_test.go +++ b/pkg/multus/multus_cni100_test.go @@ -18,6 +18,7 @@ package multus //revive:disable:dot-imports import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -26,11 +27,13 @@ import ( "time" "github.com/containernetworking/cni/pkg/skel" + cnitypes "github.com/containernetworking/cni/pkg/types" cni100 "github.com/containernetworking/cni/pkg/types/100" "github.com/containernetworking/plugins/pkg/ns" "github.com/containernetworking/plugins/pkg/testutils" "gopkg.in/k8snetworkplumbingwg/multus-cni.v4/pkg/logging" testhelpers "gopkg.in/k8snetworkplumbingwg/multus-cni.v4/pkg/testing" + "gopkg.in/k8snetworkplumbingwg/multus-cni.v4/pkg/types" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -1244,6 +1247,79 @@ var _ = Describe("multus operations cniVersion 1.1.0 config", func() { Expect(fExec.statusIndex).To(Equal(1)) }) + It("propagates delegate STATUS errors", func() { + args := &skel.CmdArgs{ + ContainerID: "123456789", + Netns: testNS.Path(), + IfName: "eth0", + } + k8sArgs := &types.K8sArgs{ + K8S_POD_NAMESPACE: cnitypes.UnmarshallableString("default"), + K8S_POD_NAME: cnitypes.UnmarshallableString("pod"), + K8S_POD_INFRA_CONTAINER_ID: cnitypes.UnmarshallableString("sandbox"), + K8S_POD_UID: cnitypes.UnmarshallableString("uid"), + } + + delegateConf, err := types.LoadDelegateNetConf([]byte(`{ + "name": "weave1", + "cniVersion": "1.1.0", + "type": "weave-net" + }`), nil, "", "") + Expect(err).NotTo(HaveOccurred()) + rt, _ := types.CreateCNIRuntimeConf(args, k8sArgs, args.IfName, nil, delegateConf) + + fExec := newFakeExec() + expectedConf := `{ + "name": "weave1", + "cniVersion": "1.1.0", + "type": "weave-net" + }` + fExec.addPlugin100(nil, "", expectedConf, nil, &cnitypes.Error{Code: 50, Msg: "status failed"}) + + err = DelegateStatus(fExec, delegateConf, rt, &types.NetConf{BinDir: "/bin", CNIDir: tmpDir}) + Expect(err).To(HaveOccurred()) + var cniErr *cnitypes.Error + Expect(errors.As(err, &cniErr)).To(BeTrue()) + Expect(cniErr.Code).To(Equal(uint(50))) + Expect(cniErr.Msg).To(Equal("status failed")) + }) + + It("propagates CmdStatus errors for single plugin delegates", func() { + args := &skel.CmdArgs{ + ContainerID: "123456789", + Netns: testNS.Path(), + IfName: "eth0", + StdinData: []byte(`{ + "name": "node-cni-network", + "type": "multus", + "defaultnetworkfile": "/tmp/foo.multus.conf", + "defaultnetworkwaitseconds": 3, + "delegates": [{ + "name": "weave1", + "cniVersion": "1.1.0", + "type": "weave-net" + }] + }`), + } + + logging.SetLogLevel("verbose") + + fExec := newFakeExec() + expectedConf := `{ + "name": "weave1", + "cniVersion": "1.1.0", + "type": "weave-net" + }` + fExec.addPlugin100(nil, "", expectedConf, nil, &cnitypes.Error{Code: 50, Msg: "status failed"}) + + err := CmdStatus(args, fExec, nil) + Expect(err).To(HaveOccurred()) + var cniErr *cnitypes.Error + Expect(errors.As(err, &cniErr)).To(BeTrue()) + Expect(cniErr.Code).To(Equal(uint(50))) + Expect(cniErr.Msg).To(Equal("status failed")) + }) + It("executes delegates with CNI GC", func() { tmpCNIDir := tmpDir + "/cniData" err := os.Mkdir(tmpCNIDir, 0777) diff --git a/pkg/server/api/api.go b/pkg/server/api/api.go index 995676c38..75aa18043 100644 --- a/pkg/server/api/api.go +++ b/pkg/server/api/api.go @@ -24,6 +24,8 @@ import ( "strings" "time" + cnitypes "github.com/containernetworking/cni/pkg/types" + utilwait "k8s.io/apimachinery/pkg/util/wait" ) @@ -71,6 +73,10 @@ func DoCNI(url string, req interface{}, socketPath string) ([]byte, error) { } if resp.StatusCode != http.StatusOK { + cniErr := &cnitypes.Error{} + if err := json.Unmarshal(body, cniErr); err == nil && cniErr.Msg != "" { + return nil, cniErr + } return nil, fmt.Errorf("CNI request failed with status %v: '%s'", resp.StatusCode, string(body)) } diff --git a/pkg/server/api/shim.go b/pkg/server/api/shim.go index 6f27763bc..bc10c47e2 100644 --- a/pkg/server/api/shim.go +++ b/pkg/server/api/shim.go @@ -16,6 +16,7 @@ package api import ( "encoding/json" + stderrors "errors" "fmt" "os" "strings" @@ -111,6 +112,10 @@ func postRequest(args *skel.CmdArgs, readinessCheck readyCheckFunc) (*Response, var body []byte body, err = DoCNI("http://dummy/cni", cniRequest, SocketPath(multusShimConfig.MultusSocketDir)) if err != nil { + var cniErr *cnitypes.Error + if stderrors.As(err, &cniErr) { + return nil, multusShimConfig.CNIVersion, err + } return nil, multusShimConfig.CNIVersion, fmt.Errorf("%s: StdinData: %s", err.Error(), string(args.StdinData)) } diff --git a/pkg/server/server.go b/pkg/server/server.go index 0d3b013d2..121f7c14f 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -17,6 +17,7 @@ package server import ( "context" "encoding/json" + "errors" "fmt" "io" "net" @@ -125,6 +126,8 @@ func (s *Server) HandleDelegateRequest(cmd string, k8sArgs *types.K8sArgs, cniCm err = s.cmdDelegateDel(cniCmdArgs, k8sArgs, multusConfig) case "CHECK": err = s.cmdDelegateCheck(cniCmdArgs, k8sArgs, multusConfig) + case "STATUS": + err = s.cmdDelegateStatus(cniCmdArgs, k8sArgs, multusConfig) default: return []byte(""), fmt.Errorf("unknown cmd type: %s", cmd) } @@ -302,7 +305,7 @@ func newCNIServer(rundir string, kubeClient *k8s.ClientInfo, exec invoke.Exec, s result, err := s.handleCNIRequest(r) if err != nil { - http.Error(w, fmt.Sprintf("%v", err), http.StatusBadRequest) + s.writeCNIErrorResponse(w, err) return } @@ -324,7 +327,7 @@ func newCNIServer(rundir string, kubeClient *k8s.ClientInfo, exec invoke.Exec, s result, err := s.handleDelegateRequest(r) if err != nil { - http.Error(w, fmt.Sprintf("%v", err), http.StatusBadRequest) + s.writeCNIErrorResponse(w, err) return } @@ -389,6 +392,34 @@ func (s *Server) Start(ctx context.Context, l net.Listener) { }() } +func (s *Server) writeCNIErrorResponse(w http.ResponseWriter, err error) { + var cniErr *cnitypes.Error + if errors.As(err, &cniErr) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + errBytes, marshalErr := json.Marshal(cniErr) + if marshalErr != nil { + http.Error(w, fmt.Sprintf("%v", err), http.StatusBadRequest) + return + } + if _, writeErr := w.Write(errBytes); writeErr != nil { + _ = logging.Errorf("Error writing HTTP response: %v", writeErr) + } + return + } + http.Error(w, fmt.Sprintf("%v", err), http.StatusBadRequest) +} + +func (s *Server) wrapCNIRequestError(cmdArgs *skel.CmdArgs, err error) error { + var cniErr *cnitypes.Error + if errors.As(err, &cniErr) { + _ = logging.Errorf("%s ERRORED: %v", printCmdArgs(cmdArgs), err) + return err + } + // Prefix error with request information for easier debugging. + return fmt.Errorf("%s ERRORED: %v", printCmdArgs(cmdArgs), err) +} + func (s *Server) handleCNIRequest(r *http.Request) ([]byte, error) { var cr api.Request b, err := io.ReadAll(r.Body) @@ -410,8 +441,7 @@ func (s *Server) handleCNIRequest(r *http.Request) ([]byte, error) { result, err := s.HandleCNIRequest(cmdType, k8sArgs, cniCmdArgs) if err != nil { - // Prefix error with request information for easier debugging - return nil, fmt.Errorf("%s ERRORED: %v", printCmdArgs(cniCmdArgs), err) + return nil, s.wrapCNIRequestError(cniCmdArgs, err) } return result, nil } @@ -437,8 +467,7 @@ func (s *Server) handleDelegateRequest(r *http.Request) ([]byte, error) { result, err := s.HandleDelegateRequest(cmdType, k8sArgs, cniCmdArgs, cr.InterfaceAttributes) if err != nil { - // Prefix error with request information for easier debugging - return nil, fmt.Errorf("%s ERRORED: %v", printCmdArgs(cniCmdArgs), err) + return nil, s.wrapCNIRequestError(cniCmdArgs, err) } return result, nil } @@ -705,6 +734,15 @@ func (s *Server) cmdDelegateCheck(cmdArgs *skel.CmdArgs, k8sArgs *types.K8sArgs, return multus.DelegateCheck(s.exec, delegateCNIConf, rt, multusConfig) } +func (s *Server) cmdDelegateStatus(cmdArgs *skel.CmdArgs, k8sArgs *types.K8sArgs, multusConfig *types.NetConf) error { + delegateCNIConf, err := types.LoadDelegateNetConf(cmdArgs.StdinData, nil, "", "") + if err != nil { + return err + } + rt, _ := types.CreateCNIRuntimeConf(cmdArgs, k8sArgs, cmdArgs.IfName, nil, delegateCNIConf) + return multus.DelegateStatus(s.exec, delegateCNIConf, rt, multusConfig) +} + // note: this function may send back error to the client. In cni spec, command DEL should NOT send any error // because container deletion follows cni DEL command. But in delegateDel case, container is not removed by // this delegateDel, hence we decide to send error message to the request sender.