Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance and efficiency improvements in daemon/server mode #1154

Merged
merged 13 commits into from
Sep 15, 2023
Merged
Prev Previous commit
Next Next commit
server/config: simplify ConfigManager creation
A couple of the setup variables for NewManager*() are already in the
multus config that it gets passed, so use those instead of passing
explicitly.

Signed-off-by: Dan Williams <[email protected]>
dcbw committed Sep 14, 2023
commit 4ade85669bf514da2fd6d0c2fbbe5612f67422aa
12 changes: 4 additions & 8 deletions cmd/multus-daemon/main.go
Original file line number Diff line number Diff line change
@@ -98,19 +98,15 @@ func main() {
}
logging.Verbosef("API readiness check done!")

var configManager *config.Manager

// Generate multus CNI config from current CNI config
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")
}

var configManager *config.Manager
if multusConf.MultusMasterCni == "" {
configManager, err = config.NewManager(*multusConf, multusConf.MultusAutoconfigDir, multusConf.ForceCNIVersion)
} else {
configManager, err = config.NewManagerWithExplicitPrimaryCNIPlugin(
*multusConf, multusConf.MultusAutoconfigDir, multusConf.MultusMasterCni, multusConf.ForceCNIVersion)
}
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)
@@ -154,7 +150,7 @@ func main() {
}()

var wg sync.WaitGroup
if multusConf.MultusConfigFile == "auto" {
if configManager != nil {
wg.Add(1)
go func() {
<-configWatcherDoneChannel
2 changes: 2 additions & 0 deletions pkg/server/config/generator.go
Original file line number Diff line number Diff line change
@@ -126,6 +126,8 @@ func (mc *MultusConf) Generate() (string, error) {
mc.CniConfigDir = ""
mc.MultusConfigFile = ""
mc.MultusAutoconfigDir = ""
mc.MultusMasterCni = ""
mc.ForceCNIVersion = false

data, err := json.Marshal(mc)
return string(data), err
44 changes: 21 additions & 23 deletions pkg/server/config/manager.go
Original file line number Diff line number Diff line change
@@ -46,23 +46,21 @@ type Manager struct {
}

// NewManager returns a config manager object, configured to read the
// primary CNI configuration in `multusAutoconfigDir`. This constructor will auto-discover
// the primary CNI for which it will delegate.
func NewManager(config MultusConf, multusAutoconfigDir string, forceCNIVersion bool) (*Manager, error) {
defaultCNIPluginName, err := getPrimaryCNIPluginName(multusAutoconfigDir)
if err != nil {
_ = logging.Errorf("failed to find the primary CNI plugin: %v", err)
return nil, err
// primary CNI configuration in `config.MultusAutoconfigDir`. If
// `config.MultusMasterCni` is empty, this constructor will auto-discover the
// primary CNI for which it will delegate.
func NewManager(config MultusConf) (*Manager, error) {
var err error
defaultPluginName := config.MultusMasterCni
if defaultPluginName == "" {
defaultPluginName, err = getPrimaryCNIPluginName(config.MultusAutoconfigDir)
if err != nil {
_ = logging.Errorf("failed to find the primary CNI plugin: %v", err)
return nil, err
}
}
return newManager(config, multusAutoconfigDir, defaultCNIPluginName, forceCNIVersion)
}

// NewManagerWithExplicitPrimaryCNIPlugin returns a config manager object,
// configured to persist the configuration to `multusAutoconfigDir`. This
// constructor will use the primary CNI plugin indicated by the user, via the
// primaryCNIPluginName variable.
func NewManagerWithExplicitPrimaryCNIPlugin(config MultusConf, multusAutoconfigDir, primaryCNIPluginName string, forceCNIVersion bool) (*Manager, error) {
return newManager(config, multusAutoconfigDir, primaryCNIPluginName, forceCNIVersion)
return newManager(config, defaultPluginName)
}

// overrideCNIVersion overrides cniVersion in cniConfigFile, it should be used only in kind case
@@ -90,9 +88,9 @@ func overrideCNIVersion(cniConfigFile string, multusCNIVersion string) error {
return nil
}

func newManager(config MultusConf, multusConfigDir, defaultCNIPluginName string, forceCNIVersion bool) (*Manager, error) {
if forceCNIVersion {
err := overrideCNIVersion(cniPluginConfigFilePath(multusConfigDir, defaultCNIPluginName), config.CNIVersion)
func newManager(config MultusConf, defaultCNIPluginName string) (*Manager, error) {
if config.ForceCNIVersion {
err := overrideCNIVersion(cniPluginConfigFilePath(config.MultusAutoconfigDir, defaultCNIPluginName), config.CNIVersion)
if err != nil {
return nil, err
}
@@ -103,21 +101,21 @@ func newManager(config MultusConf, multusConfigDir, defaultCNIPluginName string,
readinessIndicatorPath = filepath.Dir(config.ReadinessIndicatorFile)
}

watcher, err := newWatcher(multusConfigDir, readinessIndicatorPath)
watcher, err := newWatcher(config.MultusAutoconfigDir, readinessIndicatorPath)
if err != nil {
return nil, err
}

if defaultCNIPluginName == fmt.Sprintf("%s/%s", multusConfigDir, multusConfigFileName) {
return nil, logging.Errorf("cannot specify %s/%s to prevent recursive config load", multusConfigDir, multusConfigFileName)
if defaultCNIPluginName == fmt.Sprintf("%s/%s", config.MultusAutoconfigDir, multusConfigFileName) {
return nil, logging.Errorf("cannot specify %s/%s to prevent recursive config load", config.MultusAutoconfigDir, multusConfigFileName)
}

configManager := &Manager{
configWatcher: watcher,
multusConfig: &config,
multusConfigDir: multusConfigDir,
multusConfigDir: config.MultusAutoconfigDir,
multusConfigFilePath: cniPluginConfigFilePath(config.CniConfigDir, multusConfigFileName),
primaryCNIConfigPath: cniPluginConfigFilePath(multusConfigDir, defaultCNIPluginName),
primaryCNIConfigPath: cniPluginConfigFilePath(config.MultusAutoconfigDir, defaultCNIPluginName),
readinessIndicatorFilePath: config.ReadinessIndicatorFile,
}

18 changes: 12 additions & 6 deletions pkg/server/config/manager_test.go
Original file line number Diff line number Diff line change
@@ -54,15 +54,18 @@ var _ = Describe("Configuration Manager", func() {

multusConfFile := fmt.Sprintf(`{
"name": %q,
"cniVersion": %q
}`, defaultCniConfig, cniVersion)
"cniVersion": %q,
"multusAutoconfigDir": %q,
"multusMasterCNI": %q,
"forceCNIVersion": false
}`, defaultCniConfig, cniVersion, multusConfigDir, primaryCNIPluginName)
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)
configManager, err = NewManager(*multusConf)
Expect(err).NotTo(HaveOccurred())
})

@@ -173,14 +176,17 @@ var _ = Describe("Configuration Manager with mismatched cniVersion", func() {

multusConfFile := fmt.Sprintf(`{
"name": %q,
"cniVersion": %q
}`, defaultCniConfig, cniVersion)
"cniVersion": %q,
"multusAutoconfigDir": %q,
"multusMasterCNI": %q,
"forceCNIVersion": false
}`, defaultCniConfig, cniVersion, multusConfigDir, primaryCNIPluginName)
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)
_, err = NewManager(*multusConf)
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'"))
})