diff --git a/cmd/multus-daemon/main.go b/cmd/multus-daemon/main.go index 2c7e443b8..26e862cb4 100644 --- a/cmd/multus-daemon/main.go +++ b/cmd/multus-daemon/main.go @@ -83,38 +83,42 @@ func main() { logging.Verbosef("Readiness Indicator file check done!") } - if err := startMultusDaemon(ctx, daemonConf); err != nil { - logging.Panicf("failed start the multus thick-plugin listener: %v", err) - os.Exit(3) - } - - // Wait until daemon ready - logging.Verbosef("API readiness check") - if waitUntilAPIReady(daemonConf.SocketDir) != nil { - logging.Panicf("failed to ready multus-daemon socket: %v", err) - os.Exit(1) - } - logging.Verbosef("API readiness check done!") - var configManager *config.Manager - - // Generate multus CNI config from current CNI config + var ignoreReadinessIndicator bool if multusConf.MultusConfigFile == "auto" { if multusConf.CNIVersion == "" { _ = logging.Errorf("the CNI version is a mandatory parameter when the '-multus-config-file=auto' option is used") } + // Generate multus CNI config from current CNI config configManager, err = config.NewManager(*multusConf) if err != nil { _ = logging.Errorf("failed to create the configuration manager for the primary CNI plugin: %v", err) os.Exit(2) } + // ConfigManager watches the readiness indicator file (if configured) + // and exits the daemon when that is removed. The CNIServer does + // not need to re-do that check every CNI operation + ignoreReadinessIndicator = true } else { if err := copyUserProvidedConfig(multusConf.MultusConfigFile, multusConf.CniConfigDir); err != nil { logging.Errorf("failed to copy the user provided configuration %s: %v", multusConf.MultusConfigFile, err) } } + if err := startMultusDaemon(ctx, daemonConf, ignoreReadinessIndicator); err != nil { + logging.Panicf("failed start the multus thick-plugin listener: %v", err) + os.Exit(3) + } + + // Wait until daemon ready + logging.Verbosef("API readiness check") + if waitUntilAPIReady(daemonConf.SocketDir) != nil { + logging.Panicf("failed to ready multus-daemon socket: %v", err) + os.Exit(1) + } + logging.Verbosef("API readiness check done!") + signalCh := make(chan os.Signal, 16) signal.Notify(signalCh, syscall.SIGINT, syscall.SIGTERM) go func() { @@ -146,7 +150,7 @@ func waitUntilAPIReady(socketPath string) error { }) } -func startMultusDaemon(ctx context.Context, daemonConfig *srv.ControllerNetConf) error { +func startMultusDaemon(ctx context.Context, daemonConfig *srv.ControllerNetConf, ignoreReadinessIndicator bool) error { if user, err := user.Current(); err != nil || user.Uid != "0" { return fmt.Errorf("failed to run multus-daemon with root: %v, now running in uid: %s", err, user.Uid) } @@ -155,7 +159,7 @@ func startMultusDaemon(ctx context.Context, daemonConfig *srv.ControllerNetConf) return fmt.Errorf("failed to prepare the cni-socket for communicating with the shim: %w", err) } - server, err := srv.NewCNIServer(daemonConfig, daemonConfig.ConfigFileContents) + server, err := srv.NewCNIServer(daemonConfig, daemonConfig.ConfigFileContents, ignoreReadinessIndicator) if err != nil { return fmt.Errorf("failed to create the server: %v", err) } diff --git a/pkg/server/config/generator.go b/pkg/server/config/generator.go index db3d61007..a6f15977d 100644 --- a/pkg/server/config/generator.go +++ b/pkg/server/config/generator.go @@ -128,6 +128,9 @@ func (mc *MultusConf) Generate() (string, error) { mc.MultusAutoconfigDir = "" mc.MultusMasterCni = "" mc.ForceCNIVersion = false + // Readiness indicator file existence is already handled by the + // ConfigManager via an fsnotify watch, so CmdAdd/CmdDel don't need to. + mc.ReadinessIndicatorFile = "" data, err := json.Marshal(mc) return string(data), err diff --git a/pkg/server/server.go b/pkg/server/server.go index 66317a703..fb2ddafd5 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -15,7 +15,6 @@ package server import ( - "bytes" "context" "encoding/json" "fmt" @@ -45,6 +44,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" utilwait "k8s.io/apimachinery/pkg/util/wait" informerfactory "k8s.io/client-go/informers" v1coreinformers "k8s.io/client-go/informers/core/v1" @@ -105,11 +105,9 @@ func (s *Server) HandleCNIRequest(cmd string, k8sArgs *types.K8sArgs, cniCmdArgs func (s *Server) HandleDelegateRequest(cmd string, k8sArgs *types.K8sArgs, cniCmdArgs *skel.CmdArgs, interfaceAttributes *api.DelegateInterfaceAttributes) ([]byte, error) { var result []byte var err error - var multusConfByte []byte - multusConfByte = bytes.Replace(s.serverConfig, []byte(","), []byte("{"), 1) multusConfig := types.GetDefaultNetConf() - if err = json.Unmarshal(multusConfByte, multusConfig); err != nil { + if err = json.Unmarshal(s.serverConfig, multusConfig); err != nil { return nil, err } @@ -173,7 +171,7 @@ func newPodInformer(kubeClient kubernetes.Interface, nodeName string) (internali } // NewCNIServer creates and returns a new Server object which will listen on a socket in the given path -func NewCNIServer(daemonConfig *ControllerNetConf, serverConfig []byte) (*Server, error) { +func NewCNIServer(daemonConfig *ControllerNetConf, serverConfig []byte, ignoreReadinessIndicator bool) (*Server, error) { kubeClient, err := k8s.InClusterK8sClient() if err != nil { return nil, fmt.Errorf("error getting k8s client: %v", err) @@ -190,17 +188,10 @@ func NewCNIServer(daemonConfig *ControllerNetConf, serverConfig []byte) (*Server logging.Verbosef("server configured with chroot: %s", daemonConfig.ChrootDir) } - return newCNIServer(daemonConfig.SocketDir, kubeClient, exec, serverConfig) + return newCNIServer(daemonConfig.SocketDir, kubeClient, exec, serverConfig, ignoreReadinessIndicator) } -func newCNIServer(rundir string, kubeClient *k8s.ClientInfo, exec invoke.Exec, servConfig []byte) (*Server, error) { - - // preprocess server config to be used to override multus CNI config - // see extractCniData() for the detail - if servConfig != nil { - servConfig = bytes.Replace(servConfig, []byte("{"), []byte(","), 1) - } - +func newCNIServer(rundir string, kubeClient *k8s.ClientInfo, exec invoke.Exec, servConfig []byte, ignoreReadinessIndicator bool) (*Server, error) { informerFactory, podInformer := newPodInformer(kubeClient.Client, os.Getenv("MULTUS_NODE_NAME")) router := http.NewServeMux() @@ -221,8 +212,9 @@ func newCNIServer(rundir string, kubeClient *k8s.ClientInfo, exec invoke.Exec, s []string{"handler", "code", "method"}, ), }, - informerFactory: informerFactory, - podInformer: podInformer, + informerFactory: informerFactory, + podInformer: podInformer, + ignoreReadinessIndicator: ignoreReadinessIndicator, } s.SetKeepAlivesEnabled(false) @@ -326,7 +318,7 @@ func (s *Server) handleCNIRequest(r *http.Request) ([]byte, error) { if err := json.Unmarshal(b, &cr); err != nil { return nil, err } - cmdType, cniCmdArgs, err := extractCniData(&cr, s.serverConfig) + cmdType, cniCmdArgs, err := s.extractCniData(&cr, s.serverConfig) if err != nil { return nil, fmt.Errorf("could not extract the CNI command args: %w", err) } @@ -353,7 +345,7 @@ func (s *Server) handleDelegateRequest(r *http.Request) ([]byte, error) { if err := json.Unmarshal(b, &cr); err != nil { return nil, err } - cmdType, cniCmdArgs, err := extractCniData(&cr, s.serverConfig) + cmdType, cniCmdArgs, err := s.extractCniData(&cr, s.serverConfig) if err != nil { return nil, fmt.Errorf("could not extract the CNI command args: %w", err) } @@ -371,7 +363,42 @@ func (s *Server) handleDelegateRequest(r *http.Request) ([]byte, error) { return result, nil } -func extractCniData(cniRequest *api.Request, overrideConf []byte) (string, *skel.CmdArgs, error) { +func overrideCNIConfigWithServerConfig(cniConf []byte, overrideConf []byte, ignoreReadinessIndicator bool) ([]byte, error) { + if len(overrideConf) == 0 { + return cniConf, nil + } + + var cni map[string]interface{} + if err := json.Unmarshal(cniConf, &cni); err != nil { + return nil, fmt.Errorf("failed to unmarshall CNI config: %w", err) + } + + var override map[string]interface{} + if err := json.Unmarshal(overrideConf, &override); err != nil { + return nil, fmt.Errorf("failed to unmarshall CNI override config: %w", err) + } + + // Copy each key of the override config into the CNI config except for + // a few specific keys + ignoreKeys := sets.NewString() + if ignoreReadinessIndicator { + ignoreKeys.Insert("readinessindicatorfile") + } + for overrideKey, overrideVal := range override { + if !ignoreKeys.Has(overrideKey) { + cni[overrideKey] = overrideVal + } + } + + newBytes, err := json.Marshal(cni) + if err != nil { + return nil, fmt.Errorf("failed ot marshall new CNI config with overrides: %w", err) + } + + return newBytes, nil +} + +func (s *Server) extractCniData(cniRequest *api.Request, overrideConf []byte) (string, *skel.CmdArgs, error) { cmd, ok := cniRequest.Env["CNI_COMMAND"] if !ok { return "", nil, fmt.Errorf("unexpected or missing CNI_COMMAND") @@ -398,18 +425,10 @@ func extractCniData(cniRequest *api.Request, overrideConf []byte) (string, *skel } cniCmdArgs.Args = cniArgs - if overrideConf != nil { - // trim the close bracket from multus CNI config and put the server config - // to override CNI config with server config. - // note: if there are two or more value in same key, then the - // latest one is used at golang json implementation - idx := bytes.LastIndex(cniRequest.Config, []byte("}")) - if idx == -1 { - return "", nil, fmt.Errorf("invalid CNI config") - } - cniCmdArgs.StdinData = append(cniRequest.Config[:idx], overrideConf...) - } else { - cniCmdArgs.StdinData = cniRequest.Config + var err error + cniCmdArgs.StdinData, err = overrideCNIConfigWithServerConfig(cniRequest.Config, overrideConf, s.ignoreReadinessIndicator) + if err != nil { + return "", nil, err } return cmd, cniCmdArgs, nil diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go new file mode 100644 index 000000000..597fdb382 --- /dev/null +++ b/pkg/server/server_test.go @@ -0,0 +1,102 @@ +// Copyright (c) 2022 Multus Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Server", func() { + cniConf := []byte(`{ + "binDir": "/var/lib/cni/bin", + "clusterNetwork": "/host/run/multus/cni/net.d/10-ovn-kubernetes.conf", + "cniVersion": "0.3.1", + "daemonSocketDir": "/run/multus/socket", + "globalNamespaces": "default,openshift-multus,openshift-sriov-network-operator", + "logLevel": "verbose", + "logToStderr": true, + "name": "multus-cni-network", + "namespaceIsolation": true, + "type": "multus-shim" +}`) + + serverConf := []byte(`{ + "cniVersion": "0.4.0", + "chrootDir": "/hostroot", + "logToStderr": false, + "logLevel": "debug", + "binDir": "/foo/bar", + "cniConfigDir": "/host/etc/cni/net.d", + "multusConfigFile": "auto", + "multusAutoconfigDir": "/host/run/multus/cni/net.d", + "namespaceIsolation": false, + "globalNamespaces": "other,namespace", + "readinessindicatorfile": "/host/run/multus/cni/net.d/10-ovn-kubernetes.conf", + "daemonSocketDir": "/somewhere/socket", + "socketDir": "/host/run/multus/socket" +}`) + + Context("correctly overrides incoming CNI config with server config", func() { + newConf, err := overrideCNIConfigWithServerConfig(cniConf, serverConf, false) + Expect(err).ToNot(HaveOccurred()) + + // All server options except readinessindicatorfile should exist + // in the returned config + Expect(newConf).To(MatchJSON(`{ + "clusterNetwork": "/host/run/multus/cni/net.d/10-ovn-kubernetes.conf", + "name": "multus-cni-network", + "type": "multus-shim", + "cniVersion": "0.4.0", + "chrootDir": "/hostroot", + "logToStderr": false, + "logLevel": "debug", + "binDir": "/foo/bar", + "cniConfigDir": "/host/etc/cni/net.d", + "multusConfigFile": "auto", + "multusAutoconfigDir": "/host/run/multus/cni/net.d", + "namespaceIsolation": false, + "globalNamespaces": "other,namespace", + "readinessindicatorfile": "/host/run/multus/cni/net.d/10-ovn-kubernetes.conf", + "daemonSocketDir": "/somewhere/socket", + "socketDir": "/host/run/multus/socket" +}`)) + }) + + Context("correctly overrides incoming CNI config with server config and ignores readinessindicatorfile", func() { + newConf, err := overrideCNIConfigWithServerConfig(cniConf, serverConf, true) + Expect(err).ToNot(HaveOccurred()) + + // All server options except readinessindicatorfile should exist + // in the returned config + Expect(newConf).To(MatchJSON(`{ + "clusterNetwork": "/host/run/multus/cni/net.d/10-ovn-kubernetes.conf", + "name": "multus-cni-network", + "type": "multus-shim", + "cniVersion": "0.4.0", + "chrootDir": "/hostroot", + "logToStderr": false, + "logLevel": "debug", + "binDir": "/foo/bar", + "cniConfigDir": "/host/etc/cni/net.d", + "multusConfigFile": "auto", + "multusAutoconfigDir": "/host/run/multus/cni/net.d", + "namespaceIsolation": false, + "globalNamespaces": "other,namespace", + "daemonSocketDir": "/somewhere/socket", + "socketDir": "/host/run/multus/socket" +}`)) + }) +}) diff --git a/pkg/server/thick_cni_test.go b/pkg/server/thick_cni_test.go index 4b0591bc7..917c3e6ea 100644 --- a/pkg/server/thick_cni_test.go +++ b/pkg/server/thick_cni_test.go @@ -256,7 +256,7 @@ func createFakePod(k8sClient *k8s.ClientInfo, podName string) error { func startCNIServer(ctx context.Context, runDir string, k8sClient *k8s.ClientInfo, servConfig []byte) (*Server, error) { const period = 0 - cniServer, err := newCNIServer(runDir, k8sClient, &fakeExec{}, servConfig) + cniServer, err := newCNIServer(runDir, k8sClient, &fakeExec{}, servConfig, true) if err != nil { return nil, err } diff --git a/pkg/server/types.go b/pkg/server/types.go index 9dd318352..b7cf11d7d 100644 --- a/pkg/server/types.go +++ b/pkg/server/types.go @@ -52,6 +52,8 @@ type Server struct { metrics *Metrics informerFactory internalinterfaces.SharedInformerFactory podInformer cache.SharedIndexInformer + + ignoreReadinessIndicator bool } // ControllerNetConf for the controller cni configuration