From c279938e219be34c9351921b310dde6bb44e583d Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Sat, 8 Apr 2023 01:03:40 +0900 Subject: [PATCH] Refactoring thick daemon config processing to damonset config file, hence command line option parsing is no longer used. This change removes these parts. Fix #1058. --- cmd/multus-daemon/main.go | 93 +----- deployments/multus-daemonset-thick.yml | 4 - pkg/server/config/generator.go | 270 +++------------- pkg/server/config/generator_test.go | 421 +++++-------------------- pkg/server/config/manager.go | 10 +- pkg/server/config/manager_test.go | 29 +- pkg/server/server.go | 25 +- pkg/server/types.go | 25 ++ pkg/types/conf.go | 30 -- pkg/types/types.go | 16 - 10 files changed, 208 insertions(+), 715 deletions(-) diff --git a/cmd/multus-daemon/main.go b/cmd/multus-daemon/main.go index 40b8a7506..7954182c8 100644 --- a/cmd/multus-daemon/main.go +++ b/cmd/multus-daemon/main.go @@ -34,35 +34,17 @@ import ( srv "gopkg.in/k8snetworkplumbingwg/multus-cni.v3/pkg/server" "gopkg.in/k8snetworkplumbingwg/multus-cni.v3/pkg/server/api" "gopkg.in/k8snetworkplumbingwg/multus-cni.v3/pkg/server/config" - "gopkg.in/k8snetworkplumbingwg/multus-cni.v3/pkg/types" "github.com/prometheus/client_golang/prometheus/promhttp" ) -const ( - multusPluginName = "multus-shim" - defaultCniConfigDir = "/etc/cni/net.d" - defaultMultusGlobalNamespaces = "" - defaultMultusLogFile = "" - defaultMultusLogMaxSize = 100 // megabytes - defaultMultusLogMaxAge = 5 // days - defaultMultusLogMaxBackups = 5 - defaultMultusLogCompress = true - defaultMultusLogLevel = "" - defaultMultusLogToStdErr = false - defaultMultusMasterCNIFile = "" - defaultMultusNamespaceIsolation = false - defaultMultusReadinessIndicatorFile = "" - defaultSocketDir = "/run/multus/" -) - func main() { flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) // keep in command line option version := flag.Bool("version", false, "Show version") - configFilePath := flag.String("config", types.DefaultMultusDaemonConfigFile, "Specify the path to the multus-daemon configuration") + configFilePath := flag.String("config", srv.DefaultMultusDaemonConfigFile, "Specify the path to the multus-daemon configuration") flag.Parse() @@ -98,75 +80,14 @@ func main() { _ = logging.Errorf("the CNI version is a mandatory parameter when the '-multus-config-file=auto' option is used") } - var configurationOptions []config.Option - - if multusConf.NamespaceIsolation { - configurationOptions = append( - configurationOptions, config.WithNamespaceIsolation()) - } - - if multusConf.RawNonIsolatedNamespaces != defaultMultusGlobalNamespaces { - configurationOptions = append( - configurationOptions, config.WithGlobalNamespaces(multusConf.RawNonIsolatedNamespaces)) - } - - if multusConf.LogToStderr != defaultMultusLogToStdErr { - configurationOptions = append( - configurationOptions, config.WithLogToStdErr()) - } - - if multusConf.LogLevel != defaultMultusLogLevel { - configurationOptions = append( - configurationOptions, config.WithLogLevel(multusConf.LogLevel)) - } - - if multusConf.LogFile != defaultMultusLogFile { - configurationOptions = append( - configurationOptions, config.WithLogFile(multusConf.LogFile)) - } - - if multusConf.ReadinessIndicatorFile != defaultMultusReadinessIndicatorFile { - configurationOptions = append( - configurationOptions, config.WithReadinessFileIndicator(multusConf.ReadinessIndicatorFile)) - } - - configurationOptions = append( - configurationOptions, - config.WithCniConfigDir(multusConf.CniConfigDir), - config.WithSocketDir(daemonConf.SocketDir)) - - var logOptionFuncs []config.LogOptionFunc - if multusConf.LogMaxAge != defaultMultusLogMaxAge { - logOptionFuncs = append(logOptionFuncs, config.WithLogMaxAge(&multusConf.LogMaxAge)) - } - if multusConf.LogMaxSize != defaultMultusLogMaxSize { - logOptionFuncs = append(logOptionFuncs, config.WithLogMaxSize(&multusConf.LogMaxSize)) - } - if multusConf.LogMaxBackups != defaultMultusLogMaxBackups { - logOptionFuncs = append(logOptionFuncs, config.WithLogMaxBackups(&multusConf.LogMaxBackups)) - } - if multusConf.LogCompress != defaultMultusLogCompress { - logOptionFuncs = append(logOptionFuncs, config.WithLogCompress(&multusConf.LogCompress)) - } - - if len(logOptionFuncs) > 0 { - logOptions := &config.LogOptions{} - config.MutateLogOptions(logOptions, logOptionFuncs...) - configurationOptions = append(configurationOptions, config.WithLogOptions(logOptions)) - } - - multusConfig, err := config.NewMultusConfig(multusPluginName, multusConf.CNIVersion, configurationOptions...) - if err != nil { - _ = logging.Errorf("Failed to create multus config: %v", err) - os.Exit(3) - } + multusConf.SocketDir = daemonConf.SocketDir var configManager *config.Manager if multusConf.MultusMasterCni == "" { - configManager, err = config.NewManager(*multusConfig, multusConf.MultusAutoconfigDir, multusConf.ForceCNIVersion) + configManager, err = config.NewManager(*multusConf, multusConf.MultusAutoconfigDir, multusConf.ForceCNIVersion) } else { configManager, err = config.NewManagerWithExplicitPrimaryCNIPlugin( - *multusConfig, multusConf.MultusAutoconfigDir, multusConf.MultusMasterCni, multusConf.ForceCNIVersion) + *multusConf, multusConf.MultusAutoconfigDir, multusConf.MultusMasterCni, multusConf.ForceCNIVersion) } if err != nil { _ = logging.Errorf("failed to create the configuration manager for the primary CNI plugin: %v", err) @@ -221,7 +142,7 @@ func main() { // never reached } -func startMultusDaemon(daemonConfig *types.ControllerNetConf, stopCh chan struct{}, done chan struct{}) error { +func startMultusDaemon(daemonConfig *srv.ControllerNetConf, stopCh chan struct{}, done chan struct{}) 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) } @@ -263,12 +184,12 @@ func startMultusDaemon(daemonConfig *types.ControllerNetConf, stopCh chan struct return nil } -func cniServerConfig(configFilePath string) (*types.ControllerNetConf, error) { +func cniServerConfig(configFilePath string) (*srv.ControllerNetConf, error) { configFileContents, err := os.ReadFile(configFilePath) if err != nil { return nil, err } - return types.LoadDaemonNetConf(configFileContents) + return srv.LoadDaemonNetConf(configFileContents) } func copyUserProvidedConfig(multusConfigPath string, cniConfigDir string) error { diff --git a/deployments/multus-daemonset-thick.yml b/deployments/multus-daemonset-thick.yml index ba3b82e4a..eaa92ece9 100644 --- a/deployments/multus-daemonset-thick.yml +++ b/deployments/multus-daemonset-thick.yml @@ -112,11 +112,7 @@ data: { "chrootDir": "/hostroot", "confDir": "/host/etc/cni/net.d", - "logToStderr": true, "logLevel": "verbose", - "logFile": "/tmp/multus.log", - "binDir": "/opt/cni/bin", - "cniDir": "/var/lib/cni/multus", "socketDir": "/host/run/multus/", "cniVersion": "0.3.1", "cniConfigDir": "/host/etc/cni/net.d", diff --git a/pkg/server/config/generator.go b/pkg/server/config/generator.go index 1a2a0d18b..fcfac33af 100644 --- a/pkg/server/config/generator.go +++ b/pkg/server/config/generator.go @@ -25,68 +25,42 @@ import ( "time" "github.com/blang/semver" + + "gopkg.in/k8snetworkplumbingwg/multus-cni.v3/pkg/logging" ) const ( configListCapabilityKey = "plugins" + multusPluginName = "multus-shim" singleConfigCapabilityKey = "capabilities" ) -// LogOptionFunc mutates the `LoggingOptions` object -type LogOptionFunc func(logOptions *LogOptions) - // Option mutates the `conf` object type Option func(conf *MultusConf) error -// MultusConf holds the multus configuration, and persists it to disk +// MultusConf holds the multus configuration type MultusConf struct { - BinDir string `json:"binDir,omitempty"` - Capabilities map[string]bool `json:"capabilities,omitempty"` - CNIVersion string `json:"cniVersion"` - LogFile string `json:"logFile,omitempty"` - LogLevel string `json:"logLevel,omitempty"` - LogToStderr bool `json:"logToStderr,omitempty"` - LogOptions *LogOptions `json:"logOptions,omitempty"` - Name string `json:"name"` - ClusterNetwork string `json:"clusterNetwork,omitempty"` - NamespaceIsolation bool `json:"namespaceIsolation,omitempty"` - RawNonIsolatedNamespaces string `json:"globalNamespaces,omitempty"` - ReadinessIndicatorFile string `json:"readinessindicatorfile,omitempty"` - Type string `json:"type"` - CniDir string `json:"cniDir,omitempty"` - CniConfigDir string `json:"cniConfigDir,omitempty"` - SocketDir string `json:"socketDir,omitempty"` - MultusConfigFile string `json:"multusConfigFile,omitempty"` - LogMaxAge int `json:"logMaxAge,omitempty"` - LogMaxSize int `json:"logMaxSize,omitempty"` - LogMaxBackups int `json:"logMaxBackups,omitempty"` - LogCompress bool `json:"logCompress,omitempty"` - MultusMasterCni string `json:"multusMasterCNI,omitempty"` - MultusAutoconfigDir string `json:"multusAutoconfigDir,omitempty"` - ForceCNIVersion bool `json:"forceCNIVersion,omitempty"` - OverrideNetworkName bool `json:"overrideNetworkName,omitempty"` -} - -// LogOptions specifies the configuration of the log -type LogOptions struct { - MaxAge *int `json:"maxAge,omitempty"` - MaxSize *int `json:"maxSize,omitempty"` - MaxBackups *int `json:"maxBackups,omitempty"` - Compress *bool `json:"compress,omitempty"` -} - -// NewMultusConfig creates a basic configuration generator. It can be mutated -// via the `With...` methods. -func NewMultusConfig(pluginName string, cniVersion string, configurationOptions ...Option) (*MultusConf, error) { - multusConfig := &MultusConf{ - Name: MultusDefaultNetworkName, - CNIVersion: cniVersion, - Type: pluginName, - Capabilities: map[string]bool{}, - } - - err := multusConfig.Mutate(configurationOptions...) - return multusConfig, err + BinDir string `json:"binDir,omitempty"` + Capabilities map[string]bool `json:"capabilities,omitempty"` + CNIVersion string `json:"cniVersion"` + LogFile string `json:"logFile,omitempty"` + LogLevel string `json:"logLevel,omitempty"` + LogToStderr bool `json:"logToStderr,omitempty"` + LogOptions *logging.LogOptions `json:"logOptions,omitempty"` + Name string `json:"name"` + ClusterNetwork string `json:"clusterNetwork,omitempty"` + NamespaceIsolation bool `json:"namespaceIsolation,omitempty"` + RawNonIsolatedNamespaces string `json:"globalNamespaces,omitempty"` + ReadinessIndicatorFile string `json:"readinessindicatorfile,omitempty"` + Type string `json:"type"` + CniDir string `json:"cniDir,omitempty"` + CniConfigDir string `json:"cniConfigDir,omitempty"` + SocketDir string `json:"socketDir,omitempty"` + MultusConfigFile string `json:"multusConfigFile,omitempty"` + MultusMasterCni string `json:"multusMasterCNI,omitempty"` + MultusAutoconfigDir string `json:"multusAutoconfigDir,omitempty"` + ForceCNIVersion bool `json:"forceCNIVersion,omitempty"` + OverrideNetworkName bool `json:"overrideNetworkName,omitempty"` } // ParseMultusConfig parses multus config from configPath and create MultusConf. @@ -96,11 +70,17 @@ func ParseMultusConfig(configPath string) (*MultusConf, error) { return nil, fmt.Errorf("ParseMultusConfig failed to read the config file's contents: %w", err) } - multusconf := MultusConf{MultusConfigFile: "auto"} + multusconf := MultusConf{ + MultusConfigFile: "auto", + Type: multusPluginName, + Capabilities: map[string]bool{}, + CniConfigDir: "/etc/cni/net.d", + } if err := json.Unmarshal(config, &multusconf); err != nil { return nil, fmt.Errorf("failed to unmarshall the daemon configuration: %w", err) } + multusconf.Name = MultusDefaultNetworkName // change name return &multusconf, nil } @@ -142,138 +122,16 @@ func CheckVersionCompatibility(mc *MultusConf, delegate interface{}) error { // Generate generates the multus configuration from whatever state is currently // held func (mc *MultusConf) Generate() (string, error) { + // before marshal, flush variables which is not required for multus-shim config + mc.CniConfigDir = "" + mc.MultusConfigFile = "" + mc.MultusAutoconfigDir = "" + data, err := json.Marshal(mc) return string(data), err } -// Mutate updates the MultusConf attributes according to the provided -// configuration `Option`s -func (mc *MultusConf) Mutate(configurationOptions ...Option) error { - for _, configOption := range configurationOptions { - err := configOption(mc) - if err != nil { - return err - } - } - - return nil -} - -// WithNamespaceIsolation mutates the inner state to enable the -// NamespaceIsolation attribute -func WithNamespaceIsolation() Option { - return func(conf *MultusConf) error { - conf.NamespaceIsolation = true - return nil - } -} - -// WithGlobalNamespaces mutates the inner state to set the -// RawNonIsolatedNamespaces attribute -func WithGlobalNamespaces(globalNamespaces string) Option { - return func(conf *MultusConf) error { - conf.RawNonIsolatedNamespaces = globalNamespaces - return nil - } -} - -// WithLogToStdErr mutates the inner state to enable the -// WithLogToStdErr attribute -func WithLogToStdErr() Option { - return func(conf *MultusConf) error { - conf.LogToStderr = true - return nil - } -} - -// WithLogLevel mutates the inner state to set the -// LogLevel attribute -func WithLogLevel(logLevel string) Option { - return func(conf *MultusConf) error { - conf.LogLevel = logLevel - return nil - } -} - -// WithLogFile mutates the inner state to set the -// logFile attribute -func WithLogFile(logFile string) Option { - return func(conf *MultusConf) error { - conf.LogFile = logFile - return nil - } -} - -// WithLogOptions mutates the inner state to set the -// LogOptions attribute -func WithLogOptions(logOptions *LogOptions) Option { - return func(conf *MultusConf) error { - conf.LogOptions = logOptions - return nil - } -} - -// WithReadinessFileIndicator mutates the inner state to set the -// ReadinessIndicatorFile attribute -func WithReadinessFileIndicator(path string) Option { - return func(conf *MultusConf) error { - conf.ReadinessIndicatorFile = path - return nil - } -} - -// WithAdditionalBinaryFileDir mutates the inner state to set the -// BinDir attribute -func WithAdditionalBinaryFileDir(directoryPath string) Option { - return func(conf *MultusConf) error { - conf.BinDir = directoryPath - return nil - } -} - -// WithOverriddenName mutates the inner state to set the -// Name attribute -func WithOverriddenName(networkName string) Option { - return func(conf *MultusConf) error { - conf.Name = networkName - return nil - } -} - -// WithCniDir mutates the inner state to set the -// multus CNI cache directory -func WithCniDir(cniDir string) Option { - return func(conf *MultusConf) error { - conf.CniDir = cniDir - return nil - } -} - -// WithCniConfigDir mutates the inner state to set the -// multus CNI configuration directory -func WithCniConfigDir(confDir string) Option { - return func(conf *MultusConf) error { - conf.CniConfigDir = confDir - return nil - } -} - -// WithSocketDir mutates the socket directory -func WithSocketDir(sockDir string) Option { - return func(conf *MultusConf) error { - conf.SocketDir = sockDir - return nil - } -} - -func withClusterNetwork(clusterNetwork string) Option { - return func(conf *MultusConf) error { - conf.ClusterNetwork = clusterNetwork - return nil - } -} - -func withCapabilities(cniData interface{}) Option { +func (mc *MultusConf) setCapabilities(cniData interface{}) error { var enabledCapabilities []string var pluginsList []interface{} cniDataMap, ok := cniData.(map[string]interface{}) @@ -282,9 +140,7 @@ func withCapabilities(cniData interface{}) Option { pluginsList = pluginsListEntry.([]interface{}) } } else { - return func(conf *MultusConf) error { - return errors.New("couldn't get cni config from delegate") - } + return errors.New("couldn't get cni config from delegate") } if len(pluginsList) > 0 { @@ -297,52 +153,10 @@ func withCapabilities(cniData interface{}) Option { enabledCapabilities = extractCapabilities(cniData) } - return func(conf *MultusConf) error { - for _, capability := range enabledCapabilities { - conf.Capabilities[capability] = true - } - return nil - } -} - -// MutateLogOptions update the LoggingOptions of the MultusConf according -// to the provided configuration `loggingOptions` -func MutateLogOptions(logOption *LogOptions, logOptionFunc ...LogOptionFunc) { - for _, loggingOption := range logOptionFunc { - loggingOption(logOption) - } -} - -// WithLogMaxSize mutates the inner state to set the -// logMaxSize attribute -func WithLogMaxSize(maxSize *int) LogOptionFunc { - return func(logOptions *LogOptions) { - logOptions.MaxSize = maxSize - } -} - -// WithLogMaxAge mutates the inner state to set the -// logMaxAge attribute -func WithLogMaxAge(maxAge *int) LogOptionFunc { - return func(logOptions *LogOptions) { - logOptions.MaxAge = maxAge - } -} - -// WithLogMaxBackups mutates the inner state to set the -// logMaxBackups attribute -func WithLogMaxBackups(maxBackups *int) LogOptionFunc { - return func(logOptions *LogOptions) { - logOptions.MaxBackups = maxBackups - } -} - -// WithLogCompress mutates the inner state to set the -// logCompress attribute -func WithLogCompress(compress *bool) LogOptionFunc { - return func(logOptions *LogOptions) { - logOptions.Compress = compress + for _, capability := range enabledCapabilities { + mc.Capabilities[capability] = true } + return nil } func extractCapabilities(capabilitiesInterface interface{}) []string { diff --git a/pkg/server/config/generator_test.go b/pkg/server/config/generator_test.go index 800cdcc51..a21a59c90 100644 --- a/pkg/server/config/generator_test.go +++ b/pkg/server/config/generator_test.go @@ -20,8 +20,6 @@ import ( "os" "testing" - testutils "gopkg.in/k8snetworkplumbingwg/multus-cni.v3/pkg/testing" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -50,14 +48,6 @@ var primaryCNIConfig = map[string]interface{}{ } var primaryCNIFile = "/etc/cni/net.d/10-flannel.conf" -func newMultusConfigWithDelegates(pluginName string, cniVersion string, primaryCNIFile string, configOptions ...Option) (*MultusConf, error) { - multusConfig, err := NewMultusConfig(pluginName, cniVersion, configOptions...) - if err != nil { - return multusConfig, err - } - return multusConfig, multusConfig.Mutate(withClusterNetwork(primaryCNIFile)) -} - var _ = Describe("Configuration Generator", func() { var tmpDir string var err error @@ -73,280 +63,38 @@ var _ = Describe("Configuration Generator", func() { }) It("basic multus config", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) + multusConfFile := fmt.Sprintf(`{ + "name": %q, + "cniVersion": %q, + "clusterNetwork": %q + }`, primaryCNIName, cniVersion, primaryCNIFile) + multusConfFileName := fmt.Sprintf("%s/10-testcni.conf", tmpDir) + Expect(os.WriteFile(multusConfFileName, []byte(multusConfFile), 0755)).To(Succeed()) - It("multus config with namespaceisolation", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithNamespaceIsolation()) + multusConfig, err := ParseMultusConfig(multusConfFileName) Expect(err).NotTo(HaveOccurred()) expectedResult := fmt.Sprintf(` { "cniVersion":"0.4.0", "clusterNetwork":"%s", "name":"multus-cni-network", - "namespaceIsolation":true, - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("multus config with readinessindicator", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithReadinessFileIndicator("/a/b/u/it-lives")) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "readinessindicatorfile":"/a/b/u/it-lives", - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("multus config with logging configuration", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithLogLevel("notice"), - WithLogToStdErr(), - WithLogFile("/u/y/w/log.1")) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "logFile":"/u/y/w/log.1", - "logLevel":"notice", - "logToStderr":true, - "name":"multus-cni-network", - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("multus config with logging options configuration", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithLogOptions(&LogOptions{ - MaxAge: testutils.Int(5), - MaxSize: testutils.Int(100), - MaxBackups: testutils.Int(5), - Compress: testutils.Bool(true), - })) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "logOptions": { - "maxAge": 5, - "maxSize": 100, - "maxBackups": 5, - "compress": true - }, - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("multus config with logging options with max age", func() { - logOption := &LogOptions{} - MutateLogOptions(logOption, WithLogMaxAge(testutils.Int(5))) - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithLogOptions(logOption)) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "logOptions": { - "maxAge": 5 - }, - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("multus config with logging options with max size", func() { - logOption := &LogOptions{} - MutateLogOptions(logOption, WithLogMaxSize(testutils.Int(100))) - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithLogOptions(logOption)) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "logOptions": { - "maxSize": 100 - }, - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("multus config with logging options with log max backups", func() { - logOption := &LogOptions{} - MutateLogOptions(logOption, WithLogMaxBackups(testutils.Int(5))) - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithLogOptions(logOption)) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "logOptions": { - "maxBackups": 5 - }, - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("multus config with logging options with log compress", func() { - logOption := &LogOptions{} - MutateLogOptions(logOption, WithLogCompress(testutils.Bool(true))) - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithLogOptions(logOption)) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "logOptions": { - "compress": true - }, - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("generates a multus config with CNI configuration directory", func() { - cniConfigDir := "/host/etc/cni/net.d" - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithCniConfigDir(cniConfigDir)) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "cniConfigDir":"/host/etc/cni/net.d", - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("generates a multus config with a custom socket directory", func() { - socketDir := "/var/run/multus/multussock/" - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithSocketDir(socketDir)) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "socketDir":"/var/run/multus/multussock/", - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("multus config with global namespace", func() { - const globalNamespace = "come-along-ns" - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithGlobalNamespaces(globalNamespace)) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "globalNamespaces":"come-along-ns", - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - - It("multus config with additional binDir", func() { - const anotherCNIBinDir = "a-dir-somewhere" - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithAdditionalBinaryFileDir(anotherCNIBinDir)) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "binDir":"a-dir-somewhere", - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"multus-cni-network", - "type":"myCNI" + "type":"multus-shim" }`, primaryCNIFile) Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) }) It("multus config with capabilities", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - withCapabilities( - documentHelper(`{"capabilities": {"portMappings": true}}`)), - ) + multusConfFile := fmt.Sprintf(`{ + "name": %q, + "cniVersion": %q, + "clusterNetwork": %q + }`, primaryCNIName, cniVersion, primaryCNIFile) + multusConfFileName := fmt.Sprintf("%s/10-testcni.conf", tmpDir) + Expect(os.WriteFile(multusConfFileName, []byte(multusConfFile), 0755)).To(Succeed()) + + multusConfig, err := ParseMultusConfig(multusConfFileName) Expect(err).NotTo(HaveOccurred()) + Expect(multusConfig.setCapabilities(documentHelper(`{"capabilities": {"portMappings": true}}`))).To(Succeed()) expectedResult := fmt.Sprintf(` { "capabilities":{ @@ -355,78 +103,97 @@ var _ = Describe("Configuration Generator", func() { "cniVersion":"0.4.0", "clusterNetwork":"%s", "name":"multus-cni-network", - "type":"myCNI" + "type":"multus-shim" }`, primaryCNIFile) Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) }) It("multus config with multiple capabilities", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - withCapabilities( - documentHelper(`{"capabilities": {"portMappings": true, "tuning": true}}`)), - ) + multusConfFile := fmt.Sprintf(`{ + "name": %q, + "cniVersion": %q, + "clusterNetwork": %q + }`, primaryCNIName, cniVersion, primaryCNIFile) + multusConfFileName := fmt.Sprintf("%s/10-testcni.conf", tmpDir) + Expect(os.WriteFile(multusConfFileName, []byte(multusConfFile), 0755)).To(Succeed()) + + multusConfig, err := ParseMultusConfig(multusConfFileName) Expect(err).NotTo(HaveOccurred()) + Expect(multusConfig.setCapabilities( + documentHelper(`{"capabilities": {"portMappings": true, "tuning": true}}`))).To(Succeed()) expectedResult := fmt.Sprintf(` { "capabilities":{"portMappings":true,"tuning":true}, "cniVersion":"0.4.0", "clusterNetwork":"%s", "name":"multus-cni-network", - "type":"myCNI" + "type":"multus-shim" }`, primaryCNIFile) Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) }) It("multus config with multiple capabilities filter only enabled", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - withCapabilities( - documentHelper(`{"capabilities": {"portMappings": true, "tuning": false}}`)), - ) + multusConfFile := fmt.Sprintf(`{ + "name": %q, + "cniVersion": %q, + "clusterNetwork": %q + }`, primaryCNIName, cniVersion, primaryCNIFile) + multusConfFileName := fmt.Sprintf("%s/10-testcni.conf", tmpDir) + Expect(os.WriteFile(multusConfFileName, []byte(multusConfFile), 0755)).To(Succeed()) + + multusConfig, err := ParseMultusConfig(multusConfFileName) Expect(err).NotTo(HaveOccurred()) + Expect(multusConfig.setCapabilities( + documentHelper(`{"capabilities": {"portMappings": true, "tuning": false}}`))).To(Succeed()) expectedResult := fmt.Sprintf(` { "capabilities":{"portMappings":true}, "cniVersion":"0.4.0", "clusterNetwork":"%s", "name":"multus-cni-network", - "type":"myCNI" + "type":"multus-shim" }`, primaryCNIFile) Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) }) It("multus config with multiple capabilities defined on a plugin", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - withCapabilities( - documentHelper(`{"plugins": [ {"capabilities": {"portMappings": true, "tuning": true}} ] }`)), - ) + multusConfFile := fmt.Sprintf(`{ + "name": %q, + "cniVersion": %q, + "clusterNetwork": %q + }`, primaryCNIName, cniVersion, primaryCNIFile) + multusConfFileName := fmt.Sprintf("%s/10-testcni.conf", tmpDir) + Expect(os.WriteFile(multusConfFileName, []byte(multusConfFile), 0755)).To(Succeed()) + + multusConfig, err := ParseMultusConfig(multusConfFileName) Expect(err).NotTo(HaveOccurred()) + Expect(multusConfig.setCapabilities( + documentHelper( + `{"plugins": [ {"capabilities": {"portMappings": true, "tuning": true}} ] }`))).To(Succeed()) expectedResult := fmt.Sprintf(` { "capabilities":{"portMappings":true,"tuning":true}, "cniVersion":"0.4.0", "clusterNetwork":"%s", "name":"multus-cni-network", - "type":"myCNI" + "type":"multus-shim" }`, primaryCNIFile) Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) }) It("multus config with multiple capabilities defined on multiple plugins", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - withCapabilities( - documentHelper(` + multusConfFile := fmt.Sprintf(`{ + "name": %q, + "cniVersion": %q, + "clusterNetwork": %q + }`, primaryCNIName, cniVersion, primaryCNIFile) + multusConfFileName := fmt.Sprintf("%s/10-testcni.conf", tmpDir) + Expect(os.WriteFile(multusConfFileName, []byte(multusConfFile), 0755)).To(Succeed()) + + multusConfig, err := ParseMultusConfig(multusConfFileName) + Expect(err).NotTo(HaveOccurred()) + Expect(multusConfig.setCapabilities( + documentHelper(` { "plugins": [ { @@ -436,27 +203,31 @@ var _ = Describe("Configuration Generator", func() { "capabilities": { "tuning": true } } ] - }`)), - ) - Expect(err).NotTo(HaveOccurred()) + }`))).To(Succeed()) expectedResult := fmt.Sprintf(` { "capabilities":{"portMappings":true,"tuning":true}, "cniVersion":"0.4.0", "clusterNetwork":"%s", "name":"multus-cni-network", - "type":"myCNI" + "type":"multus-shim" }`, primaryCNIFile) Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) }) It("multus config with multiple capabilities defined on multiple plugins filter only enabled", func() { - multusConfig, err := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - withCapabilities( - documentHelper(` + multusConfFile := fmt.Sprintf(`{ + "name": %q, + "cniVersion": %q, + "clusterNetwork": %q + }`, primaryCNIName, cniVersion, primaryCNIFile) + multusConfFileName := fmt.Sprintf("%s/10-testcni.conf", tmpDir) + Expect(os.WriteFile(multusConfFileName, []byte(multusConfFile), 0755)).To(Succeed()) + + multusConfig, err := ParseMultusConfig(multusConfFileName) + Expect(err).NotTo(HaveOccurred()) + Expect(multusConfig.setCapabilities( + documentHelper(` { "plugins": [ { @@ -470,39 +241,17 @@ var _ = Describe("Configuration Generator", func() { } } ] - }`), - ), - ) - Expect(err).NotTo(HaveOccurred()) + }`))).To(Succeed()) expectedResult := fmt.Sprintf(` { "capabilities":{"portMappings":true}, "cniVersion":"0.4.0", "clusterNetwork":"%s", "name":"multus-cni-network", - "type":"myCNI" + "type":"multus-shim" }`, primaryCNIFile) Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) }) - - It("multus config with overridden name", func() { - newNetworkName := "mega-net-2000" - multusConfig, _ := newMultusConfigWithDelegates( - primaryCNIName, - cniVersion, - primaryCNIFile, - WithOverriddenName(newNetworkName)) - Expect(err).NotTo(HaveOccurred()) - expectedResult := fmt.Sprintf(` - { - "cniVersion":"0.4.0", - "clusterNetwork":"%s", - "name":"mega-net-2000", - "type":"myCNI" - }`, primaryCNIFile) - Expect(multusConfig.Generate()).Should(MatchJSON(expectedResult)) - }) - }) func documentHelper(pluginInfo string) interface{} { diff --git a/pkg/server/config/manager.go b/pkg/server/config/manager.go index db0ff624b..610ae0fb4 100644 --- a/pkg/server/config/manager.go +++ b/pkg/server/config/manager.go @@ -108,7 +108,7 @@ func newManager(config MultusConf, multusConfigDir, defaultCNIPluginName string, configWatcher: watcher, multusConfig: &config, multusConfigDir: multusConfigDir, - multusConfigFilePath: cniPluginConfigFilePath(config.CniConfigDir, multusConfigFileName), + multusConfigFilePath: cniPluginConfigFilePath(multusConfigDir, multusConfigFileName), primaryCNIConfigPath: cniPluginConfigFilePath(multusConfigDir, defaultCNIPluginName), } @@ -144,16 +144,16 @@ func (m *Manager) OverrideNetworkName() error { if networkName == "" { return fmt.Errorf("the primary CNI Configuration does not feature the network name: %v", m.cniConfigData) } - return m.multusConfig.Mutate(WithOverriddenName(networkName)) + m.multusConfig.Name = networkName + return nil } func (m *Manager) loadPrimaryCNIConfigurationData(primaryCNIConfigData interface{}) error { cniConfigData := primaryCNIConfigData.(map[string]interface{}) m.cniConfigData = cniConfigData - return m.multusConfig.Mutate( - withClusterNetwork(m.primaryCNIConfigPath), - withCapabilities(cniConfigData)) + m.multusConfig.ClusterNetwork = m.primaryCNIConfigPath + return m.multusConfig.setCapabilities(cniConfigData) } // GenerateConfig generates a multus configuration from its current state diff --git a/pkg/server/config/manager_test.go b/pkg/server/config/manager_test.go index 0e1896647..2ffb231c6 100644 --- a/pkg/server/config/manager_test.go +++ b/pkg/server/config/manager_test.go @@ -51,9 +51,16 @@ var _ = Describe("Configuration Manager", func() { defaultCniConfig = fmt.Sprintf("%s/%s", multusConfigDir, primaryCNIPluginName) Expect(os.WriteFile(defaultCniConfig, []byte(primaryCNIPluginTemplate), UserRWPermission)).To(Succeed()) - multusConf, _ := NewMultusConfig( - primaryCNIName, - cniVersion) + multusConfFile := fmt.Sprintf(`{ + "name": %q, + "cniVersion": %q + }`, defaultCniConfig, cniVersion) + multusConfFileName := fmt.Sprintf("%s/10-testcni.conf", multusConfigDir) + Expect(os.WriteFile(multusConfFileName, []byte(multusConfFile), 0755)).To(Succeed()) + + multusConf, err := ParseMultusConfig(multusConfFileName) + Expect(err).NotTo(HaveOccurred()) + configManager, err = NewManagerWithExplicitPrimaryCNIPlugin(*multusConf, multusConfigDir, primaryCNIPluginName, false) Expect(err).NotTo(HaveOccurred()) }) @@ -63,7 +70,7 @@ var _ = Describe("Configuration Manager", func() { }) It("Generates a configuration, based on the contents of the delegated CNI config file", func() { - expectedResult := fmt.Sprintf("{\"cniVersion\":\"0.4.0\",\"name\":\"multus-cni-network\",\"clusterNetwork\":\"%s\",\"type\":\"myCNI\"}", defaultCniConfig) + expectedResult := fmt.Sprintf("{\"cniVersion\":\"0.4.0\",\"name\":\"multus-cni-network\",\"clusterNetwork\":\"%s\",\"type\":\"multus-shim\"}", defaultCniConfig) config, err := configManager.GenerateConfig() Expect(err).NotTo(HaveOccurred()) Expect(config).To(Equal(expectedResult)) @@ -128,7 +135,7 @@ var _ = Describe("Configuration Manager", func() { }) It("Overrides the name of the multus configuration when requested", func() { - expectedResult := fmt.Sprintf("{\"cniVersion\":\"0.4.0\",\"name\":\"mycni-name\",\"clusterNetwork\":\"%s\",\"type\":\"myCNI\"}", defaultCniConfig) + expectedResult := fmt.Sprintf("{\"cniVersion\":\"0.4.0\",\"name\":\"mycni-name\",\"clusterNetwork\":\"%s\",\"type\":\"multus-shim\"}", defaultCniConfig) config, err := configManager.GenerateConfig() Expect(err).NotTo(HaveOccurred()) Expect(config).To(Equal(expectedResult)) @@ -162,9 +169,15 @@ var _ = Describe("Configuration Manager with mismatched cniVersion", func() { defaultCniConfig = fmt.Sprintf("%s/%s", multusConfigDir, primaryCNIPluginName) Expect(os.WriteFile(defaultCniConfig, []byte(primaryCNIPluginTemplate), UserRWPermission)).To(Succeed()) - multusConf, _ := NewMultusConfig( - primaryCNIName, - cniVersion) + multusConfFile := fmt.Sprintf(`{ + "name": %q, + "cniVersion": %q + }`, defaultCniConfig, cniVersion) + multusConfFileName := fmt.Sprintf("%s/10-testcni.conf", multusConfigDir) + Expect(os.WriteFile(multusConfFileName, []byte(multusConfFile), 0755)).To(Succeed()) + + multusConf, err := ParseMultusConfig(multusConfFileName) + Expect(err).NotTo(HaveOccurred()) _, err = NewManagerWithExplicitPrimaryCNIPlugin(*multusConf, multusConfigDir, primaryCNIPluginName, false) Expect(err).To(MatchError("failed to load the primary CNI configuration as a multus delegate with error 'delegate cni version is 0.3.1 while top level cni version is 0.4.0'")) }) diff --git a/pkg/server/server.go b/pkg/server/server.go index f1b57d7b8..40917faee 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -128,7 +128,7 @@ func GetListener(socketPath string) (net.Listener, error) { } // NewCNIServer creates and returns a new Server object which will listen on a socket in the given path -func NewCNIServer(daemonConfig *types.ControllerNetConf, serverConfig []byte) (*Server, error) { +func NewCNIServer(daemonConfig *ControllerNetConf, serverConfig []byte) (*Server, error) { kubeClient, err := k8s.InClusterK8sClient() if err != nil { return nil, fmt.Errorf("error getting k8s client: %v", err) @@ -278,7 +278,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) // not override config + cmdType, cniCmdArgs, err := extractCniData(&cr, s.serverConfig) if err != nil { return nil, fmt.Errorf("could not extract the CNI command args: %w", err) } @@ -530,3 +530,24 @@ func cmdDelegateDel(cmdArgs *skel.CmdArgs, k8sArgs *types.K8sArgs, exec invoke.E rt, _ := types.CreateCNIRuntimeConf(cmdArgs, k8sArgs, cmdArgs.IfName, nil, delegateCNIConf) return multus.DelegateDel(exec, pod, delegateCNIConf, rt, multusConfig) } + +// LoadDaemonNetConf loads the configuration for the multus daemon +func LoadDaemonNetConf(config []byte) (*ControllerNetConf, error) { + daemonNetConf := &ControllerNetConf{ + SocketDir: DefaultMultusRunDir, + } + if err := json.Unmarshal(config, daemonNetConf); err != nil { + return nil, fmt.Errorf("failed to unmarshall the daemon configuration: %w", err) + } + + logging.SetLogStderr(daemonNetConf.LogToStderr) + if daemonNetConf.LogFile != DefaultMultusDaemonConfigFile { + logging.SetLogFile(daemonNetConf.LogFile) + } + if daemonNetConf.LogLevel != "" { + logging.SetLogLevel(daemonNetConf.LogLevel) + } + daemonNetConf.ConfigFileContents = config + + return daemonNetConf, nil +} diff --git a/pkg/server/types.go b/pkg/server/types.go index b2b08c845..8e9b7cd2a 100644 --- a/pkg/server/types.go +++ b/pkg/server/types.go @@ -24,6 +24,15 @@ import ( "gopkg.in/k8snetworkplumbingwg/multus-cni.v3/pkg/k8sclient" ) +const ( + // const block for multus-daemon configs + + // DefaultMultusDaemonConfigFile is the Default path of the config file + DefaultMultusDaemonConfigFile = "/etc/cni/net.d/multus.d/daemon-config.json" + // DefaultMultusRunDir specifies default RunDir for multus + DefaultMultusRunDir = "/run/multus/" +) + // Metrics represents server's metrics. type Metrics struct { requestCounter *prometheus.CounterVec @@ -39,3 +48,19 @@ type Server struct { serverConfig []byte metrics *Metrics } + +// ControllerNetConf for the controller cni configuration +type ControllerNetConf struct { + ChrootDir string `json:"chrootDir,omitempty"` + LogFile string `json:"logFile"` + LogLevel string `json:"logLevel"` + LogToStderr bool `json:"logToStderr,omitempty"` + + MetricsPort *int `json:"metricsPort,omitempty"` + + // Option to point to the path of the unix domain socket through which the + // multus client / server communicate. + SocketDir string `json:"socketDir"` + + ConfigFileContents []byte `json:"-"` +} diff --git a/pkg/types/conf.go b/pkg/types/conf.go index 8c271ce5b..ae95ab121 100644 --- a/pkg/types/conf.go +++ b/pkg/types/conf.go @@ -418,36 +418,6 @@ func LoadNetConf(bytes []byte) (*NetConf, error) { return netconf, nil } -const ( - // const block for multus-daemon configs - - // DefaultMultusDaemonConfigFile is the Default path of the config file - DefaultMultusDaemonConfigFile = "/etc/cni/net.d/multus.d/daemon-config.json" - // DefaultMultusRunDir specifies default RunDir for multus - DefaultMultusRunDir = "/run/multus/" -) - -// LoadDaemonNetConf loads the configuration for the multus daemon -func LoadDaemonNetConf(config []byte) (*ControllerNetConf, error) { - daemonNetConf := &ControllerNetConf{ - SocketDir: DefaultMultusRunDir, - } - if err := json.Unmarshal(config, daemonNetConf); err != nil { - return nil, fmt.Errorf("failed to unmarshall the daemon configuration: %w", err) - } - - logging.SetLogStderr(daemonNetConf.LogToStderr) - if daemonNetConf.LogFile != DefaultMultusDaemonConfigFile { - logging.SetLogFile(daemonNetConf.LogFile) - } - if daemonNetConf.LogLevel != "" { - logging.SetLogLevel(daemonNetConf.LogLevel) - } - daemonNetConf.ConfigFileContents = config - - return daemonNetConf, nil -} - // AddDelegates appends the new delegates to the delegates list func (n *NetConf) AddDelegates(newDelegates []*DelegateNetConf) error { logging.Debugf("AddDelegates: %v", newDelegates) diff --git a/pkg/types/types.go b/pkg/types/types.go index 55e15be00..cf61d261e 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -181,19 +181,3 @@ type ResourceClient interface { // GetPodResourceMap returns an instance of a map of Pod ResourceInfo given a (Pod name, namespace) tuple GetPodResourceMap(*v1.Pod) (map[string]*ResourceInfo, error) } - -// ControllerNetConf for the controller cni configuration -type ControllerNetConf struct { - ChrootDir string `json:"chrootDir,omitempty"` - LogFile string `json:"logFile"` - LogLevel string `json:"logLevel"` - LogToStderr bool `json:"logToStderr,omitempty"` - - MetricsPort *int `json:"metricsPort,omitempty"` - - // Option to point to the path of the unix domain socket through which the - // multus client / server communicate. - SocketDir string `json:"socketDir"` - - ConfigFileContents []byte `json:"-"` -}