From 580b72a5b2013857ca23590d53112ca88a4f6736 Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Wed, 8 Jun 2022 13:49:17 +0900 Subject: [PATCH] Add unit-test to increase test coverage --- cmd/multus-daemon/main.go | 2 +- pkg/logging/logging_test.go | 41 +++++++++++++++++++++- pkg/server/config/manager.go | 4 +-- pkg/server/config/manager_test.go | 58 +++++++++++++++++++++++++++++++ pkg/server/exec_chroot_test.go | 58 +++++++++++++++++++++++++++++++ pkg/server/server_suite_test.go | 28 +++++++++++++++ pkg/server/shim.go | 8 +++-- pkg/server/thick_cni_test.go | 38 ++++++++------------ 8 files changed, 206 insertions(+), 31 deletions(-) create mode 100644 pkg/server/exec_chroot_test.go create mode 100644 pkg/server/server_suite_test.go diff --git a/cmd/multus-daemon/main.go b/cmd/multus-daemon/main.go index 2c7e77a09..27f5d16c6 100644 --- a/cmd/multus-daemon/main.go +++ b/cmd/multus-daemon/main.go @@ -198,7 +198,7 @@ func main() { defer func() { stopChannel <- struct{}{} }() - if err := configManager.MonitorPluginConfiguration(stopChannel, configWatcherDoneChannel); err != nil { + if err := configManager.MonitorPluginConfiguration(configWatcherDoneChannel, stopChannel); err != nil { _ = logging.Errorf("error watching file: %v", err) } }(make(chan struct{}), configWatcherDoneChannel) diff --git a/pkg/logging/logging_test.go b/pkg/logging/logging_test.go index 16391bdda..0c192a6f2 100644 --- a/pkg/logging/logging_test.go +++ b/pkg/logging/logging_test.go @@ -15,9 +15,13 @@ package logging import ( + "fmt" + "io/ioutil" + "os" + "testing" + testutils "gopkg.in/k8snetworkplumbingwg/multus-cni.v3/pkg/testing" "gopkg.in/natefinch/lumberjack.v2" - "testing" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -56,12 +60,16 @@ var _ = Describe("logging operations", func() { It("Check loglevel setter", func() { SetLogLevel("debug") Expect(loggingLevel).To(Equal(DebugLevel)) + Expect(loggingLevel.String()).To(Equal("debug")) SetLogLevel("Error") Expect(loggingLevel).To(Equal(ErrorLevel)) + Expect(loggingLevel.String()).To(Equal("error")) SetLogLevel("VERbose") Expect(loggingLevel).To(Equal(VerboseLevel)) + Expect(loggingLevel.String()).To(Equal("verbose")) SetLogLevel("PANIC") Expect(loggingLevel).To(Equal(PanicLevel)) + Expect(loggingLevel.String()).To(Equal("panic")) }) It("Check loglevel setter with invalid level", func() { @@ -76,6 +84,37 @@ var _ = Describe("logging operations", func() { Expect(loggingStderr).NotTo(Equal(currentVal)) }) + It("Check log function is worked", func() { + Debugf("foobar") + Verbosef("foobar") + Expect(Errorf("foobar")).NotTo(BeNil()) + Panicf("foobar") + }) + + It("Check log function is worked with stderr", func() { + SetLogStderr(true) + Debugf("foobar") + Verbosef("foobar") + Expect(Errorf("foobar")).NotTo(BeNil()) + Panicf("foobar") + }) + + It("Check log function is worked with stderr", func() { + tmpDir, err := ioutil.TempDir("", "multus_tmp") + SetLogFile(fmt.Sprintf("%s/log.txt", tmpDir)) + Debugf("foobar") + Verbosef("foobar") + Expect(Errorf("foobar")).NotTo(BeNil()) + Panicf("foobar") + logger.Filename = "" + loggingW = nil + err = os.RemoveAll(tmpDir) + Expect(err).NotTo(HaveOccurred()) + // Revert the log variable to init + loggingW = nil + logger = &lumberjack.Logger{} + }) + // Tests public getter It("Check getter for logging level with current level", func() { currentLevel := loggingLevel diff --git a/pkg/server/config/manager.go b/pkg/server/config/manager.go index b82a3af81..125767de5 100644 --- a/pkg/server/config/manager.go +++ b/pkg/server/config/manager.go @@ -47,7 +47,7 @@ type Manager struct { // configuration to `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 := primaryCNIPluginName(multusAutoconfigDir) + defaultCNIPluginName, err := getPrimaryCNIPluginName(multusAutoconfigDir) if err != nil { _ = logging.Errorf("failed to find the primary CNI plugin: %v", err) return nil, err @@ -218,7 +218,7 @@ func (m Manager) PersistMultusConfig(config string) error { return ioutil.WriteFile(m.multusConfigFilePath, []byte(config), UserRWPermission) } -func primaryCNIPluginName(multusAutoconfigDir string) (string, error) { +func getPrimaryCNIPluginName(multusAutoconfigDir string) (string, error) { masterCniConfigFileName, err := findMasterPlugin(multusAutoconfigDir, 120) if err != nil { return "", fmt.Errorf("failed to find the cluster master CNI plugin: %w", err) diff --git a/pkg/server/config/manager_test.go b/pkg/server/config/manager_test.go index abcf2c159..d22414cb4 100644 --- a/pkg/server/config/manager_test.go +++ b/pkg/server/config/manager_test.go @@ -16,9 +16,11 @@ package config import ( + "encoding/json" "fmt" "io/ioutil" "os" + "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -69,6 +71,62 @@ var _ = Describe("Configuration Manager", func() { Expect(config).To(Equal(expectedResult)) }) + It("Check overrideCNIVersion is worked", func() { + err := overrideCNIVersion(defaultCniConfig, "1.1.1") + Expect(err).NotTo(HaveOccurred()) + raw, err := ioutil.ReadFile(defaultCniConfig) + Expect(err).NotTo(HaveOccurred()) + + var jsonConfig map[string]interface{} + err = json.Unmarshal(raw, &jsonConfig) + Expect(err).NotTo(HaveOccurred()) + Expect(jsonConfig["cniVersion"].(string)).To(Equal("1.1.1")) + }) + + It("Check primaryCNIPlugin can be identified", func() { + fileName, err := getPrimaryCNIPluginName(multusConfigDir) + Expect(err).NotTo(HaveOccurred()) + Expect(fileName).To(Equal(primaryCNIPluginName)) + }) + + It("Check MonitorPluginConfiguration", func() { + config, err := configManager.GenerateConfig() + Expect(err).NotTo(HaveOccurred()) + + err = configManager.PersistMultusConfig(config) + Expect(err).NotTo(HaveOccurred()) + + configWatcherDoneChannel := make(chan struct{}) + go func(stopChannel chan struct{}, doneChannel chan struct{}) { + defer func() { + stopChannel <- struct{}{} + }() + err := configManager.MonitorPluginConfiguration(configWatcherDoneChannel, stopChannel) + Expect(err).NotTo(HaveOccurred()) + }(make(chan struct{}), configWatcherDoneChannel) + + updatedCNIConfig := ` +{ + "cniVersion": "0.4.0", + "name": "mycni-name", + "type": "mycni2", + "ipam": {}, + "dns": {} +} +` + // update the CNI config to update the master config + Expect(ioutil.WriteFile(defaultCniConfig, []byte(updatedCNIConfig), UserRWPermission)).To(Succeed()) + + // wait for a while to get fsnotify event + time.Sleep(100 * time.Millisecond) + file, err := ioutil.ReadFile(configManager.multusConfigFilePath) + Expect(err).NotTo(HaveOccurred()) + Expect(string(file)).To(Equal(config)) + + // stop groutine + configWatcherDoneChannel <- struct{}{} + }) + When("the user requests the name of the multus configuration to be overridden", func() { BeforeEach(func() { Expect(configManager.OverrideNetworkName()).To(Succeed()) diff --git a/pkg/server/exec_chroot_test.go b/pkg/server/exec_chroot_test.go new file mode 100644 index 000000000..0b9a08933 --- /dev/null +++ b/pkg/server/exec_chroot_test.go @@ -0,0 +1,58 @@ +// Copyright (c) 2021 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 ( + "context" + "os" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("exec_chroot", func() { + + It("Call ChrootExec.ExecPlugin with dummy", func() { + chrootExec := &ChrootExec{ + Stderr: os.Stderr, + chrootDir: "/usr", + } + + _, err := chrootExec.ExecPlugin(context.Background(), "/bin/true", nil, nil) + Expect(err).ToNot(HaveOccurred()) + }) + + It("Call invalid ChrootExec.ExecPlugin with dummy", func() { + chrootExec := &ChrootExec{ + Stderr: os.Stderr, + chrootDir: "/tmp", + } + + _, err := chrootExec.ExecPlugin(context.Background(), "/bin/true", nil, nil) + Expect(err).To(HaveOccurred()) + }) + + It("Call ChrootExec.FindInPath with dummy", func() { + chrootExec := &ChrootExec{ + Stderr: os.Stderr, + chrootDir: "/usr/bin", + } + + _, err := chrootExec.FindInPath("true", []string{"/"}) + Expect(err).NotTo(HaveOccurred()) + }) + +}) diff --git a/pkg/server/server_suite_test.go b/pkg/server/server_suite_test.go new file mode 100644 index 000000000..3c818006f --- /dev/null +++ b/pkg/server/server_suite_test.go @@ -0,0 +1,28 @@ +// Copyright (c) 2021 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" + . "github.com/onsi/gomega" + + "testing" +) + +func TestServer(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "server") +} diff --git a/pkg/server/shim.go b/pkg/server/shim.go index 8f5f7aab3..070bf0e1e 100644 --- a/pkg/server/shim.go +++ b/pkg/server/shim.go @@ -72,9 +72,11 @@ func postRequest(args *skel.CmdArgs) (*Response, string, error) { } response := &Response{} - if err = json.Unmarshal(body, response); err != nil { - err = fmt.Errorf("failed to unmarshal response '%s': %v", string(body), err) - return nil, multusShimConfig.CNIVersion, err + if len(body) != 0 { + if err = json.Unmarshal(body, response); err != nil { + err = fmt.Errorf("failed to unmarshal response '%s': %v", string(body), err) + return nil, multusShimConfig.CNIVersion, err + } } return response, multusShimConfig.CNIVersion, nil } diff --git a/pkg/server/thick_cni_test.go b/pkg/server/thick_cni_test.go index 32fd8ceee..eefe67348 100644 --- a/pkg/server/thick_cni_test.go +++ b/pkg/server/thick_cni_test.go @@ -20,7 +20,6 @@ import ( "fmt" "io/ioutil" "os" - "testing" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -43,11 +42,6 @@ import ( const suiteName = "Thick CNI architecture" -func TestMultusThickCNIArchitecture(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, suiteName) -} - type fakeExec struct{} // ExecPlugin executes the plugin @@ -132,16 +126,15 @@ var _ = Describe(suiteName, func() { context.TODO(), podName, metav1.DeleteOptions{})) }) - It("ADD works successfully", func() { + It("ADD/CHECK/DEL works successfully", func() { + Expect(os.Setenv("CNI_COMMAND", "ADD")).NotTo(HaveOccurred()) Expect(CmdAdd(cniCmdArgs(containerID, netns.Path(), ifaceName, referenceConfig(thickPluginRunDir)))).To(Succeed()) - }) - It("DEL works successfully", func() { - Expect(CmdDel(cniCmdArgs(containerID, netns.Path(), ifaceName, referenceConfig(thickPluginRunDir)))).To(Succeed()) - }) - - It("CHECK works successfully", func() { + Expect(os.Setenv("CNI_COMMAND", "CHECK")).NotTo(HaveOccurred()) Expect(CmdCheck(cniCmdArgs(containerID, netns.Path(), ifaceName, referenceConfig(thickPluginRunDir)))).To(Succeed()) + + Expect(os.Setenv("CNI_COMMAND", "DEL")).NotTo(HaveOccurred()) + Expect(CmdDel(cniCmdArgs(containerID, netns.Path(), ifaceName, referenceConfig(thickPluginRunDir)))).To(Succeed()) }) }) @@ -186,16 +179,16 @@ var _ = Describe(suiteName, func() { context.TODO(), podName, metav1.DeleteOptions{})) }) - It("ADD works successfully", func() { + It("ADD/CHECK/DEL works successfully", func() { + Expect(os.Setenv("CNI_COMMAND", "ADD")).NotTo(HaveOccurred()) Expect(CmdAdd(cniCmdArgs(containerID, netns.Path(), ifaceName, referenceConfig(thickPluginRunDir)))).To(Succeed()) - }) - It("DEL works successfully", func() { - Expect(CmdDel(cniCmdArgs(containerID, netns.Path(), ifaceName, referenceConfig(thickPluginRunDir)))).To(Succeed()) - }) - - It("CHECK works successfully", func() { + Expect(os.Setenv("CNI_COMMAND", "CHECK")).NotTo(HaveOccurred()) Expect(CmdCheck(cniCmdArgs(containerID, netns.Path(), ifaceName, referenceConfig(thickPluginRunDir)))).To(Succeed()) + + Expect(os.Setenv("CNI_COMMAND", "DEL")).NotTo(HaveOccurred()) + Expect(CmdDel(cniCmdArgs(containerID, netns.Path(), ifaceName, referenceConfig(thickPluginRunDir)))).To(Succeed()) + }) }) }) @@ -219,9 +212,6 @@ func cniCmdArgs(containerID string, netnsPath string, ifName string, stdinData s func prepareCNIEnv(netnsPath string, namespaceName string, podName string, podUID string) error { cniArgs := fmt.Sprintf("K8S_POD_NAMESPACE=%s;K8S_POD_NAME=%s;K8S_POD_INFRA_CONTAINER_ID=;K8S_POD_UID=%s", namespaceName, podName, podUID) - if err := os.Setenv("CNI_COMMAND", "ADD"); err != nil { - return err - } if err := os.Setenv("CNI_CONTAINERID", "123456789"); err != nil { return err } @@ -290,7 +280,7 @@ func referenceConfig(thickPluginSocketDir string) string { "defaultnetworkwaitseconds": 3, "delegates": [{ "name": "weave1", - "cniVersion": "0.3.1", + "cniVersion": "0.4.0", "type": "weave-net" }]}` return fmt.Sprintf(referenceConfigTemplate, thickPluginSocketDir)