diff --git a/.gitignore b/.gitignore index ac260ece45..a5cb39d2a2 100644 --- a/.gitignore +++ b/.gitignore @@ -16,4 +16,7 @@ ipam-*.xml .idea/* # Logs -*.log \ No newline at end of file +*.log + +# debug test files +*.test \ No newline at end of file diff --git a/go.mod b/go.mod index 569f9a778a..d0b81e3cdd 100644 --- a/go.mod +++ b/go.mod @@ -40,6 +40,7 @@ require ( k8s.io/apimachinery v0.18.2 k8s.io/client-go v0.18.2 k8s.io/klog v1.0.0 + k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89 sigs.k8s.io/controller-runtime v0.6.0 software.sslmate.com/src/go-pkcs12 v0.0.0-20201102150903-66718f75db0e // indirect ) diff --git a/npm/ipsm/ipsm.go b/npm/ipsm/ipsm.go index f2aeba6032..e74b776ee5 100644 --- a/npm/ipsm/ipsm.go +++ b/npm/ipsm/ipsm.go @@ -14,6 +14,7 @@ import ( "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/util" + utilexec "k8s.io/utils/exec" ) type ipsEntry struct { @@ -25,6 +26,7 @@ type ipsEntry struct { // IpsetManager stores ipset states. type IpsetManager struct { + exec utilexec.Interface ListMap map[string]*Ipset //tracks all set lists. SetMap map[string]*Ipset //label -> []ip } @@ -45,8 +47,9 @@ func NewIpset(setName string) *Ipset { } // NewIpsetManager creates a new instance for IpsetManager object. -func NewIpsetManager() *IpsetManager { +func NewIpsetManager(exec utilexec.Interface) *IpsetManager { return &IpsetManager{ + exec: exec, ListMap: make(map[string]*Ipset), SetMap: make(map[string]*Ipset), } @@ -408,7 +411,7 @@ func (ipsMgr *IpsetManager) DeleteFromSet(setName, ip, podKey string) error { return nil } - metrics.SendErrorLogAndMetric(util.IpsmID, "Error: failed to delete ipset entry. Entry: %+v", entry) + metrics.SendErrorLogAndMetric(util.IpsmID, "Error: failed to delete ipset entry: [%+v] err: [%v]", entry, err) return err } @@ -480,14 +483,18 @@ func (ipsMgr *IpsetManager) Run(entry *ipsEntry) (int, error) { cmdArgs = util.DropEmptyFields(cmdArgs) log.Logf("Executing ipset command %s %v", cmdName, cmdArgs) - _, err := exec.Command(cmdName, cmdArgs...).Output() - if msg, failed := err.(*exec.ExitError); failed { - errCode := msg.Sys().(syscall.WaitStatus).ExitStatus() - if errCode > 0 { - metrics.SendErrorLogAndMetric(util.IpsmID, "Error: There was an error running command: [%s %v] Stderr: [%v, %s]", cmdName, strings.Join(cmdArgs, " "), err, strings.TrimSuffix(string(msg.Stderr), "\n")) + + cmd := ipsMgr.exec.Command(cmdName, cmdArgs...) + output, err := cmd.CombinedOutput() + + if result, isExitError := err.(utilexec.ExitError); isExitError { + exitCode := result.ExitStatus() + errfmt := fmt.Errorf("Error: There was an error running command: [%s %v] Stderr: [%v, %s]", cmdName, strings.Join(cmdArgs, " "), err, strings.TrimSuffix(string(output), "\n")) + if exitCode > 0 { + metrics.SendErrorLogAndMetric(util.IpsmID, errfmt.Error()) } - return errCode, err + return exitCode, errfmt } return 0, nil @@ -499,9 +506,10 @@ func (ipsMgr *IpsetManager) Save(configFile string) error { configFile = util.IpsetConfigFile } - cmd := exec.Command(util.Ipset, util.IpsetSaveFlag, util.IpsetFileFlag, configFile) - if err := cmd.Start(); err != nil { - metrics.SendErrorLogAndMetric(util.IpsmID, "Error: failed to save ipset to file.") + cmd := ipsMgr.exec.Command(util.Ipset, util.IpsetSaveFlag, util.IpsetFileFlag, configFile) + output, err := cmd.CombinedOutput() + if err != nil { + metrics.SendErrorLogAndMetric(util.IpsmID, "Error: failed to save ipset: [%s] Stderr: [%v, %s]", cmd, err, strings.TrimSuffix(string(output), "\n")) return err } cmd.Wait() @@ -527,12 +535,12 @@ func (ipsMgr *IpsetManager) Restore(configFile string) error { } } - cmd := exec.Command(util.Ipset, util.IpsetRestoreFlag, util.IpsetFileFlag, configFile) - if err := cmd.Start(); err != nil { - metrics.SendErrorLogAndMetric(util.IpsmID, "Error: failed to to restore ipset from file.") + cmd := ipsMgr.exec.Command(util.Ipset, util.IpsetRestoreFlag, util.IpsetFileFlag, configFile) + output, err := cmd.CombinedOutput() + if err != nil { + metrics.SendErrorLogAndMetric(util.IpsmID, "Error: failed to to restore ipset from file: [%s] Stderr: [%v, %s]", cmd, err, strings.TrimSuffix(string(output), "\n")) return err } - cmd.Wait() //TODO based on the set name and number of entries in the config file, update IPSetInventory @@ -541,11 +549,10 @@ func (ipsMgr *IpsetManager) Restore(configFile string) error { // DestroyNpmIpsets destroys only ipsets created by NPM func (ipsMgr *IpsetManager) DestroyNpmIpsets() error { - cmdName := util.Ipset cmdArgs := util.IPsetCheckListFlag - reply, err := exec.Command(cmdName, cmdArgs).Output() + reply, err := ipsMgr.exec.Command(cmdName, cmdArgs).CombinedOutput() if msg, failed := err.(*exec.ExitError); failed { errCode := msg.Sys().(syscall.WaitStatus).ExitStatus() if errCode > 0 { diff --git a/npm/ipsm/ipsm_test.go b/npm/ipsm/ipsm_test.go index 5f98876d9f..ec09c4b225 100644 --- a/npm/ipsm/ipsm_test.go +++ b/npm/ipsm/ipsm_test.go @@ -4,34 +4,40 @@ package ipsm import ( "fmt" + "log" "os" "testing" + "github.com/Azure/azure-container-networking/npm/iptm" "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/metrics/promutil" "github.com/Azure/azure-container-networking/npm/util" + testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/stretchr/testify/require" + "k8s.io/utils/exec" ) func TestSave(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestSave failed @ ipsMgr.Save") } } func TestRestore(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) + if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { - t.Errorf("TestRestore failed @ ipsMgr.Save") + t.Errorf("TestRestore failed @ ipsMgr.Save with err %v", err) } if err := ipsMgr.Restore(util.IpsetTestConfigFile); err != nil { - t.Errorf("TestRestore failed @ ipsMgr.Restore") + t.Errorf("TestRestore failed @ ipsMgr.Restore with err %v", err) } } func TestCreateList(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestCreateList failed @ ipsMgr.Save") } @@ -48,7 +54,7 @@ func TestCreateList(t *testing.T) { } func TestDeleteList(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestDeleteList failed @ ipsMgr.Save") } @@ -69,7 +75,7 @@ func TestDeleteList(t *testing.T) { } func TestAddToList(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestAddToList failed @ ipsMgr.Save") } @@ -90,16 +96,23 @@ func TestAddToList(t *testing.T) { } func TestDeleteFromList(t *testing.T) { - ipsMgr := NewIpsetManager() - if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { - t.Errorf("TestDeleteFromList failed @ ipsMgr.Save") + var calls = []testutils.TestCmd{ + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("test-set"), "nethash"}}, + {Cmd: []string{"ipset", "list", "-exist", util.GetHashedName("test-set")}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("test-list"), "setlist"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("test-list"), util.GetHashedName("test-set")}}, + {Cmd: []string{"ipset", "test", "-exist", util.GetHashedName("test-list"), util.GetHashedName("test-set")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("test-list"), util.GetHashedName("test-set")}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("test-list")}}, + {Cmd: []string{"ipset", "test", "-exist", util.GetHashedName("test-list"), util.GetHashedName("test-set")}, Stderr: "ipset still exists", ExitCode: 2}, + {Cmd: []string{"ipset", "list", "-exist", util.GetHashedName("test-list")}, Stderr: "ipset still exists", ExitCode: 2}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("test-set")}}, + {Cmd: []string{"ipset", "list", "-exist", util.GetHashedName("test-set")}, Stderr: "ipset still exists", ExitCode: 2}, } - defer func() { - if err := ipsMgr.Restore(util.IpsetTestConfigFile); err != nil { - t.Errorf("TestDeleteFromList failed @ ipsMgr.Restore") - } - }() + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + ipsMgr := NewIpsetManager(fexec) // Create set and validate set is created. setName := "test-set" @@ -185,11 +198,13 @@ func TestDeleteFromList(t *testing.T) { if _, err := ipsMgr.Run(entry); err == nil { t.Errorf("TestDeleteFromList failed @ ipsMgr.DeleteSet since %s still exist in kernel", setName) } + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestCreateSet(t *testing.T) { metrics.NumIPSetEntries.Set(0) - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestCreateSet failed @ ipsMgr.Save") } @@ -243,7 +258,7 @@ func TestCreateSet(t *testing.T) { func TestDeleteSet(t *testing.T) { metrics.NumIPSetEntries.Set(0) - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestDeleteSet failed @ ipsMgr.Save") } @@ -279,7 +294,7 @@ func TestDeleteSet(t *testing.T) { func TestAddToSet(t *testing.T) { metrics.NumIPSetEntries.Set(0) - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Fatalf("TestAddToSet failed @ ipsMgr.Save") } @@ -328,7 +343,7 @@ func TestAddToSet(t *testing.T) { } func TestAddToSetWithCachePodInfo(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestAddToSetWithCachePodInfo failed @ ipsMgr.Save") } @@ -369,7 +384,7 @@ func TestAddToSetWithCachePodInfo(t *testing.T) { func TestDeleteFromSet(t *testing.T) { metrics.NumIPSetEntries.Set(0) - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestDeleteFromSet failed @ ipsMgr.Save") } @@ -407,7 +422,7 @@ func TestDeleteFromSet(t *testing.T) { } func TestDeleteFromSetWithPodCache(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestDeleteFromSetWithPodCache failed @ ipsMgr.Save") } @@ -466,7 +481,7 @@ func TestDeleteFromSetWithPodCache(t *testing.T) { } func TestClean(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestClean failed @ ipsMgr.Save") } @@ -487,7 +502,7 @@ func TestClean(t *testing.T) { } func TestDestroy(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestDestroy failed @ ipsMgr.Save") } @@ -530,14 +545,14 @@ func TestDestroy(t *testing.T) { } func TestRun(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { - t.Errorf("TestRun failed @ ipsMgr.Save") + t.Errorf("TestRun failed @ ipsMgr.Save with err %v", err) } defer func() { if err := ipsMgr.Restore(util.IpsetTestConfigFile); err != nil { - t.Errorf("TestRun failed @ ipsMgr.Restore") + t.Errorf("TestRun failed @ ipsMgr.Restore with err %v", err) } }() @@ -551,8 +566,30 @@ func TestRun(t *testing.T) { } } +func TestRunError(t *testing.T) { + setname := "test-set" + + var calls = []testutils.TestCmd{ + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName(setname), "nethash"}, Stderr: "test failure", ExitCode: 2}, + } + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + ipsMgr := NewIpsetManager(fexec) + entry := &ipsEntry{ + operationFlag: util.IpsetCreationFlag, + set: util.GetHashedName(setname), + spec: append([]string{util.IpsetNetHashFlag}), + } + if _, err := ipsMgr.Run(entry); err != nil { + require.Error(t, err) + } + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) +} + func TestDestroyNpmIpsets(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) err := ipsMgr.CreateSet("azure-npm-123456", []string{"nethash"}) if err != nil { @@ -589,7 +626,7 @@ func GetIPSetName() string { // "Set cannot be destroyed: it is in use by a kernel component" func TestSetCannotBeDestroyed(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestAddToList failed @ ipsMgr.Save") } @@ -624,7 +661,7 @@ func TestSetCannotBeDestroyed(t *testing.T) { } func TestElemSeparatorSupportsNone(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestAddToList failed @ ipsMgr.Save") } @@ -653,7 +690,7 @@ func TestElemSeparatorSupportsNone(t *testing.T) { } func TestIPSetWithGivenNameDoesNotExist(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestAddToList failed @ ipsMgr.Save with err %+v", err) } @@ -680,7 +717,7 @@ func TestIPSetWithGivenNameDoesNotExist(t *testing.T) { } func TestIPSetWithGivenNameAlreadyExists(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestAddToList failed @ ipsMgr.Save with err %+v", err) } @@ -719,7 +756,7 @@ func TestIPSetWithGivenNameAlreadyExists(t *testing.T) { } func TestIPSetSecondElementIsMissingWhenAddingIpWithNoPort(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestAddToList failed @ ipsMgr.Save with err: %+v", err) } @@ -749,7 +786,7 @@ func TestIPSetSecondElementIsMissingWhenAddingIpWithNoPort(t *testing.T) { } func TestIPSetMissingSecondMandatoryArgument(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestAddToList failed @ ipsMgr.Save") } @@ -779,7 +816,7 @@ func TestIPSetMissingSecondMandatoryArgument(t *testing.T) { } func TestIPSetCannotBeAddedAsElementDoesNotExist(t *testing.T) { - ipsMgr := NewIpsetManager() + ipsMgr := NewIpsetManager(exec.New()) if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { t.Errorf("TestAddToList failed @ ipsMgr.Save") } @@ -818,12 +855,15 @@ func TestIPSetCannotBeAddedAsElementDoesNotExist(t *testing.T) { */ func TestMain(m *testing.M) { metrics.InitializeAll() - ipsMgr := NewIpsetManager() - ipsMgr.Save(util.IpsetConfigFile) - exitCode := m.Run() + log.Printf("Uniniting iptables") + iptm := iptm.NewIptablesManager() + iptm.UninitNpmChains() + log.Printf("Uniniting ipsets") + ipsMgr := NewIpsetManager(exec.New()) + ipsMgr.Destroy() - ipsMgr.Restore(util.IpsetConfigFile) + exitCode := m.Run() os.Exit(exitCode) } diff --git a/npm/nameSpaceController.go b/npm/nameSpaceController.go index 2e1eeafe02..fcdefb8eae 100644 --- a/npm/nameSpaceController.go +++ b/npm/nameSpaceController.go @@ -23,6 +23,7 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" "k8s.io/klog" + utilexec "k8s.io/utils/exec" ) type LabelAppendOperation bool @@ -41,13 +42,12 @@ type Namespace struct { } // newNS constructs a new namespace object. -// (TODO): need to change newNS function. It always returns "nil" -func newNs(name string) (*Namespace, error) { +func newNs(name string, exec utilexec.Interface) (*Namespace, error) { ns := &Namespace{ name: name, LabelsMap: make(map[string]string), SetMap: make(map[string]string), - IpsMgr: ipsm.NewIpsetManager(), + IpsMgr: ipsm.NewIpsetManager(exec), iptMgr: iptm.NewIptablesManager(), } @@ -337,7 +337,7 @@ func (nsc *nameSpaceController) syncAddNameSpace(nsObj *corev1.Namespace) error return err } - npmNs, _ := newNs(corev1NsName) + npmNs, _ := newNs(corev1NsName, nsc.npMgr.Exec) nsc.npMgr.NsMap[corev1NsName] = npmNs // Add the namespace to its label's ipset list. diff --git a/npm/nameSpaceController_test.go b/npm/nameSpaceController_test.go index 57c76ad67a..d83897e98a 100644 --- a/npm/nameSpaceController_test.go +++ b/npm/nameSpaceController_test.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-container-networking/npm/ipsm" "github.com/Azure/azure-container-networking/npm/util" corev1 "k8s.io/api/core/v1" + "k8s.io/utils/exec" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -48,13 +49,13 @@ type nameSpaceFixture struct { kubeInformer kubeinformers.SharedInformerFactory } -func newNsFixture(t *testing.T) *nameSpaceFixture { +func newNsFixture(t *testing.T, utilexec exec.Interface) *nameSpaceFixture { f := &nameSpaceFixture{ t: t, nsLister: []*corev1.Namespace{}, kubeobjects: []runtime.Object{}, - npMgr: newNPMgr(t), - ipsMgr: ipsm.NewIpsetManager(), + npMgr: newNPMgr(t, utilexec), + ipsMgr: ipsm.NewIpsetManager(utilexec), } return f } @@ -151,13 +152,15 @@ func deleteNamespace(t *testing.T, f *nameSpaceFixture, nsObj *corev1.Namespace, } func TestNewNs(t *testing.T) { - if _, err := newNs("test"); err != nil { + fexec := exec.New() + if _, err := newNs("test", fexec); err != nil { t.Errorf("TestnewNs failed @ newNs") } } func TestAddNamespace(t *testing.T) { - f := newNsFixture(t) + fexec := exec.New() + f := newNsFixture(t, fexec) f.ipSetSave(util.IpsetTestConfigFile) defer f.ipSetRestore(util.IpsetTestConfigFile) @@ -188,7 +191,8 @@ func TestAddNamespace(t *testing.T) { } func TestUpdateNamespace(t *testing.T) { - f := newNsFixture(t) + fexec := exec.New() + f := newNsFixture(t, fexec) f.ipSetSave(util.IpsetTestConfigFile) defer f.ipSetRestore(util.IpsetTestConfigFile) @@ -233,7 +237,8 @@ func TestUpdateNamespace(t *testing.T) { } func TestAddNamespaceLabel(t *testing.T) { - f := newNsFixture(t) + fexec := exec.New() + f := newNsFixture(t, fexec) f.ipSetSave(util.IpsetTestConfigFile) defer f.ipSetRestore(util.IpsetTestConfigFile) @@ -278,7 +283,8 @@ func TestAddNamespaceLabel(t *testing.T) { } func TestAddNamespaceLabelSameRv(t *testing.T) { - f := newNsFixture(t) + fexec := exec.New() + f := newNsFixture(t, fexec) f.ipSetSave(util.IpsetTestConfigFile) defer f.ipSetRestore(util.IpsetTestConfigFile) @@ -324,7 +330,8 @@ func TestAddNamespaceLabelSameRv(t *testing.T) { } func TestDeleteandUpdateNamespaceLabel(t *testing.T) { - f := newNsFixture(t) + fexec := exec.New() + f := newNsFixture(t, fexec) f.ipSetSave(util.IpsetTestConfigFile) defer f.ipSetRestore(util.IpsetTestConfigFile) @@ -375,7 +382,8 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) { // this happens when NSA delete event is missed and deleted from NPMLocalCache, // but NSA gets added again. This will result in an update event with old and new with different UUIDs func TestNewNameSpaceUpdate(t *testing.T) { - f := newNsFixture(t) + fexec := exec.New() + f := newNsFixture(t, fexec) f.ipSetSave(util.IpsetTestConfigFile) defer f.ipSetRestore(util.IpsetTestConfigFile) @@ -425,7 +433,8 @@ func TestNewNameSpaceUpdate(t *testing.T) { } func TestDeleteNamespace(t *testing.T) { - f := newNsFixture(t) + fexec := exec.New() + f := newNsFixture(t, fexec) f.ipSetSave(util.IpsetTestConfigFile) defer f.ipSetRestore(util.IpsetTestConfigFile) @@ -455,7 +464,8 @@ func TestDeleteNamespace(t *testing.T) { } func TestDeleteNamespaceWithTombstone(t *testing.T) { - f := newNsFixture(t) + fexec := exec.New() + f := newNsFixture(t, fexec) f.ipSetSave(util.IpsetTestConfigFile) defer f.ipSetRestore(util.IpsetTestConfigFile) stopCh := make(chan struct{}) @@ -490,8 +500,8 @@ func TestDeleteNamespaceWithTombstoneAfterAddingNameSpace(t *testing.T) { "app": "test-namespace", }, ) - - f := newNsFixture(t) + fexec := exec.New() + f := newNsFixture(t, fexec) f.nsLister = append(f.nsLister, nsObj) f.kubeobjects = append(f.kubeobjects, nsObj) stopCh := make(chan struct{}) @@ -506,7 +516,8 @@ func TestDeleteNamespaceWithTombstoneAfterAddingNameSpace(t *testing.T) { } func TestGetNamespaceObjFromNsObj(t *testing.T) { - ns, _ := newNs("test-ns") + fexec := exec.New() + ns, _ := newNs("test-ns", fexec) ns.LabelsMap = map[string]string{ "test": "new", } diff --git a/npm/networkPolicyController_test.go b/npm/networkPolicyController_test.go index 5ac4f74837..e6f13b8a25 100644 --- a/npm/networkPolicyController_test.go +++ b/npm/networkPolicyController_test.go @@ -21,6 +21,7 @@ import ( k8sfake "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" + "k8s.io/utils/exec" ) type netPolFixture struct { @@ -46,13 +47,13 @@ type netPolFixture struct { isEnqueueEventIntoWorkQueue bool } -func newNetPolFixture(t *testing.T) *netPolFixture { +func newNetPolFixture(t *testing.T, utilexec exec.Interface) *netPolFixture { f := &netPolFixture{ t: t, netPolLister: []*networkingv1.NetworkPolicy{}, kubeobjects: []runtime.Object{}, - npMgr: newNPMgr(t), - ipsMgr: ipsm.NewIpsetManager(), + npMgr: newNPMgr(t, utilexec), + ipsMgr: ipsm.NewIpsetManager(utilexec), iptMgr: iptm.NewIptablesManager(), isEnqueueEventIntoWorkQueue: true, } @@ -271,7 +272,8 @@ func TestAddMultipleNetworkPolicies(t *testing.T) { // namedPort netPolObj2.Spec.Ingress[0].Ports[0].Port = &intstr.IntOrString{StrVal: fmt.Sprintf("%s", netPolObj2.Name)} - f := newNetPolFixture(t) + fexec := exec.New() + f := newNetPolFixture(t, fexec) f.netPolLister = append(f.netPolLister, netPolObj1, netPolObj2) f.kubeobjects = append(f.kubeobjects, netPolObj1, netPolObj2) stopCh := make(chan struct{}) @@ -290,7 +292,8 @@ func TestAddMultipleNetworkPolicies(t *testing.T) { func TestAddNetworkPolicy(t *testing.T) { netPolObj := createNetPol() - f := newNetPolFixture(t) + fexec := exec.New() + f := newNetPolFixture(t, fexec) f.netPolLister = append(f.netPolLister, netPolObj) f.kubeobjects = append(f.kubeobjects, netPolObj) stopCh := make(chan struct{}) @@ -308,7 +311,8 @@ func TestAddNetworkPolicy(t *testing.T) { func TestDeleteNetworkPolicy(t *testing.T) { netPolObj := createNetPol() - f := newNetPolFixture(t) + fexec := exec.New() + f := newNetPolFixture(t, fexec) f.netPolLister = append(f.netPolLister, netPolObj) f.kubeobjects = append(f.kubeobjects, netPolObj) stopCh := make(chan struct{}) @@ -325,7 +329,8 @@ func TestDeleteNetworkPolicy(t *testing.T) { func TestDeleteNetworkPolicyWithTombstone(t *testing.T) { netPolObj := createNetPol() - f := newNetPolFixture(t) + fexec := exec.New() + f := newNetPolFixture(t, fexec) f.isEnqueueEventIntoWorkQueue = false f.netPolLister = append(f.netPolLister, netPolObj) f.kubeobjects = append(f.kubeobjects, netPolObj) @@ -349,7 +354,8 @@ func TestDeleteNetworkPolicyWithTombstone(t *testing.T) { func TestDeleteNetworkPolicyWithTombstoneAfterAddingNetworkPolicy(t *testing.T) { netPolObj := createNetPol() - f := newNetPolFixture(t) + fexec := exec.New() + f := newNetPolFixture(t, fexec) f.netPolLister = append(f.netPolLister, netPolObj) f.kubeobjects = append(f.kubeobjects, netPolObj) stopCh := make(chan struct{}) @@ -368,7 +374,8 @@ func TestDeleteNetworkPolicyWithTombstoneAfterAddingNetworkPolicy(t *testing.T) func TestUpdateNetworkPolicy(t *testing.T) { oldNetPolObj := createNetPol() - f := newNetPolFixture(t) + fexec := exec.New() + f := newNetPolFixture(t, fexec) f.netPolLister = append(f.netPolLister, oldNetPolObj) f.kubeobjects = append(f.kubeobjects, oldNetPolObj) stopCh := make(chan struct{}) @@ -390,7 +397,8 @@ func TestUpdateNetworkPolicy(t *testing.T) { func TestLabelUpdateNetworkPolicy(t *testing.T) { oldNetPolObj := createNetPol() - f := newNetPolFixture(t) + fexec := exec.New() + f := newNetPolFixture(t, fexec) f.netPolLister = append(f.netPolLister, oldNetPolObj) f.kubeobjects = append(f.kubeobjects, oldNetPolObj) stopCh := make(chan struct{}) diff --git a/npm/npm.go b/npm/npm.go index 2f6429308a..99ed6138c9 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -24,6 +24,7 @@ import ( networkinginformers "k8s.io/client-go/informers/networking/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" + utilexec "k8s.io/utils/exec" ) var aiMetadata string @@ -42,6 +43,8 @@ const ( // NetworkPolicyManager contains informers for pod, namespace and networkpolicy. type NetworkPolicyManager struct { sync.Mutex + + Exec utilexec.Interface clientset *kubernetes.Clientset informerFactory informers.SharedInformerFactory @@ -198,14 +201,14 @@ func (npMgr *NetworkPolicyManager) Start(stopCh <-chan struct{}) error { } // NewNetworkPolicyManager creates a NetworkPolicyManager -func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory informers.SharedInformerFactory, npmVersion string) *NetworkPolicyManager { +func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory informers.SharedInformerFactory, exec utilexec.Interface, npmVersion string) *NetworkPolicyManager { // Clear out left over iptables states log.Logf("Azure-NPM creating, cleaning iptables") iptMgr := iptm.NewIptablesManager() iptMgr.UninitNpmChains() log.Logf("Azure-NPM creating, cleaning existing Azure NPM IPSets") - ipsm.NewIpsetManager().DestroyNpmIpsets() + ipsm.NewIpsetManager(exec).DestroyNpmIpsets() var ( podInformer = informerFactory.Core().V1().Pods() @@ -234,6 +237,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in } npMgr := &NetworkPolicyManager{ + Exec: exec, clientset: clientset, informerFactory: informerFactory, podInformer: podInformer, @@ -254,7 +258,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in TelemetryEnabled: true, } - allNs, _ := newNs(util.KubeAllNamespacesFlag) + allNs, _ := newNs(util.KubeAllNamespacesFlag, npMgr.Exec) npMgr.NsMap[util.KubeAllNamespacesFlag] = allNs // Create ipset for the namespace. diff --git a/npm/npm_test.go b/npm/npm_test.go index 10978d5183..0bf248fcd4 100644 --- a/npm/npm_test.go +++ b/npm/npm_test.go @@ -9,6 +9,8 @@ import ( "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/util" "k8s.io/client-go/tools/cache" + "k8s.io/utils/exec" + utilexec "k8s.io/utils/exec" ) // To indicate the object is needed to be DeletedFinalStateUnknown Object @@ -28,15 +30,16 @@ func getKey(obj interface{}, t *testing.T) string { return key } -func newNPMgr(t *testing.T) *NetworkPolicyManager { +func newNPMgr(t *testing.T, exec utilexec.Interface) *NetworkPolicyManager { npMgr := &NetworkPolicyManager{ + Exec: exec, NsMap: make(map[string]*Namespace), PodMap: make(map[string]*NpmPod), TelemetryEnabled: false, } // This initialization important as without this NPM will panic - allNs, _ := newNs(util.KubeAllNamespacesFlag) + allNs, _ := newNs(util.KubeAllNamespacesFlag, npMgr.Exec) npMgr.NsMap[util.KubeAllNamespacesFlag] = allNs return npMgr } @@ -44,15 +47,12 @@ func newNPMgr(t *testing.T) *NetworkPolicyManager { func TestMain(m *testing.M) { metrics.InitializeAll() iptMgr := iptm.NewIptablesManager() - iptMgr.Save(util.IptablesConfigFile) + iptMgr.UninitNpmChains() - ipsMgr := ipsm.NewIpsetManager() - ipsMgr.Save(util.IpsetConfigFile) + ipsMgr := ipsm.NewIpsetManager(exec.New()) + ipsMgr.Destroy() exitCode := m.Run() - iptMgr.Restore(util.IptablesConfigFile) - ipsMgr.Restore(util.IpsetConfigFile) - os.Exit(exitCode) } diff --git a/npm/plugin/main.go b/npm/plugin/main.go index 31a3c5adae..b36a7b8059 100644 --- a/npm/plugin/main.go +++ b/npm/plugin/main.go @@ -14,6 +14,7 @@ import ( "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/utils/exec" ) const ( @@ -73,7 +74,7 @@ func main() { log.Logf("[INFO] Resync period for NPM pod is set to %d.", int(resyncPeriod/time.Minute)) factory := informers.NewSharedInformerFactory(clientset, resyncPeriod) - npMgr := npm.NewNetworkPolicyManager(clientset, factory, version) + npMgr := npm.NewNetworkPolicyManager(clientset, factory, exec.New(), version) metrics.CreateTelemetryHandle(npMgr.GetAppVersion(), npm.GetAIMetadata()) restserver := restserver.NewNpmRestServer(restserver.DefaultHTTPListeningAddress) diff --git a/npm/podController.go b/npm/podController.go index 61e9806064..68b622f99e 100644 --- a/npm/podController.go +++ b/npm/podController.go @@ -439,7 +439,7 @@ func (c *podController) syncAddAndUpdatePod(newPodObj *corev1.Pod) error { } // Add namespace object into NsMap cache only when two ipset operations are successful. - npmNs, _ := newNs(newPodObjNs) + npmNs, _ := newNs(newPodObjNs, c.npMgr.Exec) c.npMgr.NsMap[newPodObjNs] = npmNs } diff --git a/npm/podController_test.go b/npm/podController_test.go index 8543959896..896ae5a221 100644 --- a/npm/podController_test.go +++ b/npm/podController_test.go @@ -9,7 +9,10 @@ import ( "testing" "github.com/Azure/azure-container-networking/npm/ipsm" + "github.com/Azure/azure-container-networking/npm/util" + testutils "github.com/Azure/azure-container-networking/test/utils" corev1 "k8s.io/api/core/v1" + utilexec "k8s.io/utils/exec" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -42,13 +45,13 @@ type podFixture struct { kubeInformer kubeinformers.SharedInformerFactory } -func newFixture(t *testing.T) *podFixture { +func newFixture(t *testing.T, exec utilexec.Interface) *podFixture { f := &podFixture{ t: t, podLister: []*corev1.Pod{}, kubeobjects: []runtime.Object{}, - npMgr: newNPMgr(t), - ipsMgr: ipsm.NewIpsetManager(), + npMgr: newNPMgr(t, exec), + ipsMgr: ipsm.NewIpsetManager(exec), } return f } @@ -211,7 +214,27 @@ func TestAddMultiplePods(t *testing.T) { podObj1 := createPod("test-pod-1", "test-namespace", "0", "1.2.3.4", labels, NonHostNetwork, corev1.PodRunning) podObj2 := createPod("test-pod-2", "test-namespace", "0", "1.2.3.5", labels, NonHostNetwork, corev1.PodRunning) - f := newFixture(t) + var calls = []testutils.TestCmd{ + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("ns-test-namespace"), "nethash"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("all-namespaces"), "setlist"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("all-namespaces"), util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app:test-pod"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("namedport:app:test-pod-1"), "hash:ip,port"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("namedport:app:test-pod-1"), "1.2.3.4,8080"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.5"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app"), "1.2.3.5"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.5"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("namedport:app:test-pod-2"), "hash:ip,port"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("namedport:app:test-pod-2"), "1.2.3.5,8080"}}, + } + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) f.podLister = append(f.podLister, podObj1, podObj2) f.kubeobjects = append(f.kubeobjects, podObj1, podObj2) stopCh := make(chan struct{}) @@ -227,6 +250,8 @@ func TestAddMultiplePods(t *testing.T) { checkPodTestResult("TestAddMultiplePods", f, testCases) checkNpmPodWithInput("TestAddMultiplePods", f, podObj1) checkNpmPodWithInput("TestAddMultiplePods", f, podObj2) + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestAddPod(t *testing.T) { @@ -235,7 +260,22 @@ func TestAddPod(t *testing.T) { } podObj := createPod("test-pod", "test-namespace", "0", "1.2.3.4", labels, NonHostNetwork, corev1.PodRunning) - f := newFixture(t) + var calls = []testutils.TestCmd{ + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("ns-test-namespace"), "nethash"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("all-namespaces"), "setlist"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("all-namespaces"), util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app:test-pod"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("namedport:app:test-pod"), "hash:ip,port"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + } + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) f.podLister = append(f.podLister, podObj) f.kubeobjects = append(f.kubeobjects, podObj) stopCh := make(chan struct{}) @@ -248,6 +288,8 @@ func TestAddPod(t *testing.T) { } checkPodTestResult("TestAddPod", f, testCases) checkNpmPodWithInput("TestAddPod", f, podObj) + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestAddHostNetworkPod(t *testing.T) { @@ -257,7 +299,11 @@ func TestAddHostNetworkPod(t *testing.T) { podObj := createPod("test-pod", "test-namespace", "0", "1.2.3.4", labels, HostNetwork, corev1.PodRunning) podKey := getKey(podObj, t) - f := newFixture(t) + var calls = []testutils.TestCmd{} + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) f.podLister = append(f.podLister, podObj) f.kubeobjects = append(f.kubeobjects, podObj) stopCh := make(chan struct{}) @@ -273,6 +319,8 @@ func TestAddHostNetworkPod(t *testing.T) { if _, exists := f.npMgr.PodMap[podKey]; exists { t.Error("TestAddHostNetworkPod failed @ cached pod obj exists check") } + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestDeletePod(t *testing.T) { @@ -282,7 +330,33 @@ func TestDeletePod(t *testing.T) { podObj := createPod("test-pod", "test-namespace", "0", "1.2.3.4", labels, NonHostNetwork, corev1.PodRunning) podKey := getKey(podObj, t) - f := newFixture(t) + var calls = []testutils.TestCmd{ + // add pod + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("ns-test-namespace"), "nethash"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("all-namespaces"), "setlist"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("all-namespaces"), util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app:test-pod"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("namedport:app:test-pod"), "hash:ip,port"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + + // delete pod + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("app")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("app:test-pod")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("namedport:app:test-pod")}}, + } + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) f.podLister = append(f.podLister, podObj) f.kubeobjects = append(f.kubeobjects, podObj) stopCh := make(chan struct{}) @@ -293,10 +367,13 @@ func TestDeletePod(t *testing.T) { testCases := []expectedValues{ {0, 2, 0}, } + checkPodTestResult("TestDeletePod", f, testCases) if _, exists := f.npMgr.PodMap[podKey]; exists { t.Error("TestDeletePod failed @ cached pod obj exists check") } + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestDeleteHostNetworkPod(t *testing.T) { @@ -306,7 +383,11 @@ func TestDeleteHostNetworkPod(t *testing.T) { podObj := createPod("test-pod", "test-namespace", "0", "1.2.3.4", labels, HostNetwork, corev1.PodRunning) podKey := getKey(podObj, t) - f := newFixture(t) + var calls = []testutils.TestCmd{} + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) f.podLister = append(f.podLister, podObj) f.kubeobjects = append(f.kubeobjects, podObj) stopCh := make(chan struct{}) @@ -321,6 +402,8 @@ func TestDeleteHostNetworkPod(t *testing.T) { if _, exists := f.npMgr.PodMap[podKey]; exists { t.Error("TestDeleteHostNetworkPod failed @ cached pod obj exists check") } + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestDeletePodWithTombstone(t *testing.T) { @@ -328,7 +411,12 @@ func TestDeletePodWithTombstone(t *testing.T) { "app": "test-pod", } podObj := createPod("test-pod", "test-namespace", "0", "1.2.3.4", labels, NonHostNetwork, corev1.PodRunning) - f := newFixture(t) + + var calls = []testutils.TestCmd{} + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) stopCh := make(chan struct{}) defer close(stopCh) f.newPodController(stopCh) @@ -344,6 +432,8 @@ func TestDeletePodWithTombstone(t *testing.T) { {0, 1, 0}, } checkPodTestResult("TestDeletePodWithTombstone", f, testCases) + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestDeletePodWithTombstoneAfterAddingPod(t *testing.T) { @@ -352,7 +442,33 @@ func TestDeletePodWithTombstoneAfterAddingPod(t *testing.T) { } podObj := createPod("test-pod", "test-namespace", "0", "1.2.3.4", labels, NonHostNetwork, corev1.PodRunning) - f := newFixture(t) + var calls = []testutils.TestCmd{ + // add pod + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("ns-test-namespace"), "nethash"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("all-namespaces"), "setlist"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("all-namespaces"), util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app:test-pod"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("namedport:app:test-pod"), "hash:ip,port"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + + // delete pod + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("app")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("app:test-pod")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("namedport:app:test-pod")}}, + } + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) f.podLister = append(f.podLister, podObj) f.kubeobjects = append(f.kubeobjects, podObj) stopCh := make(chan struct{}) @@ -364,6 +480,8 @@ func TestDeletePodWithTombstoneAfterAddingPod(t *testing.T) { {0, 2, 0}, } checkPodTestResult("TestDeletePodWithTombstoneAfterAddingPod", f, testCases) + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestLabelUpdatePod(t *testing.T) { @@ -372,7 +490,29 @@ func TestLabelUpdatePod(t *testing.T) { } oldPodObj := createPod("test-pod", "test-namespace", "0", "1.2.3.4", labels, NonHostNetwork, corev1.PodRunning) - f := newFixture(t) + var calls = []testutils.TestCmd{ + // add pod + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("ns-test-namespace"), "nethash"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("all-namespaces"), "setlist"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("all-namespaces"), util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app:test-pod"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("namedport:app:test-pod"), "hash:ip,port"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + + // update pod + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("app:test-pod")}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app:new-test-pod"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:new-test-pod"), "1.2.3.4"}}, + } + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) f.podLister = append(f.podLister, oldPodObj) f.kubeobjects = append(f.kubeobjects, oldPodObj) stopCh := make(chan struct{}) @@ -393,6 +533,8 @@ func TestLabelUpdatePod(t *testing.T) { } checkPodTestResult("TestLabelUpdatePod", f, testCases) checkNpmPodWithInput("TestLabelUpdatePod", f, newPodObj) + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestIPAddressUpdatePod(t *testing.T) { @@ -401,7 +543,41 @@ func TestIPAddressUpdatePod(t *testing.T) { } oldPodObj := createPod("test-pod", "test-namespace", "0", "1.2.3.4", labels, NonHostNetwork, corev1.PodRunning) - f := newFixture(t) + var calls = []testutils.TestCmd{ + // add pod + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("ns-test-namespace"), "nethash"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("all-namespaces"), "setlist"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("all-namespaces"), util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app:test-pod"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("namedport:app:test-pod"), "hash:ip,port"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + + // update pod + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("app")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("app:test-pod")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("namedport:app:test-pod")}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("ns-test-namespace"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("ns-test-namespace"), "4.3.2.1"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app"), "4.3.2.1"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app:test-pod"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:test-pod"), "4.3.2.1"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("namedport:app:test-pod"), "hash:ip,port"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("namedport:app:test-pod"), "4.3.2.1,8080"}}, + } + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) f.podLister = append(f.podLister, oldPodObj) f.kubeobjects = append(f.kubeobjects, oldPodObj) stopCh := make(chan struct{}) @@ -421,6 +597,8 @@ func TestIPAddressUpdatePod(t *testing.T) { } checkPodTestResult("TestIPAddressUpdatePod", f, testCases) checkNpmPodWithInput("TestIPAddressUpdatePod", f, newPodObj) + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestPodStatusUpdatePod(t *testing.T) { @@ -430,7 +608,33 @@ func TestPodStatusUpdatePod(t *testing.T) { oldPodObj := createPod("test-pod", "test-namespace", "0", "1.2.3.4", labels, NonHostNetwork, corev1.PodRunning) podKey := getKey(oldPodObj, t) - f := newFixture(t) + var calls = []testutils.TestCmd{ + // add pod + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("ns-test-namespace"), "nethash"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("all-namespaces"), "setlist"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("all-namespaces"), util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("app:test-pod"), "nethash"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName("namedport:app:test-pod"), "hash:ip,port"}}, + {Cmd: []string{"ipset", "-A", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + + // update pod + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("ns-test-namespace"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("ns-test-namespace")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("app"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("app")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("app:test-pod"), "1.2.3.4"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("app:test-pod")}}, + {Cmd: []string{"ipset", "-D", "-exist", util.GetHashedName("namedport:app:test-pod"), "1.2.3.4,8080"}}, + {Cmd: []string{"ipset", "-X", "-exist", util.GetHashedName("namedport:app:test-pod")}}, + } + + fexec, fcmd := testutils.GetFakeExecWithScripts(calls) + + f := newFixture(t, fexec) f.podLister = append(f.podLister, oldPodObj) f.kubeobjects = append(f.kubeobjects, oldPodObj) stopCh := make(chan struct{}) @@ -453,6 +657,8 @@ func TestPodStatusUpdatePod(t *testing.T) { if _, exists := f.npMgr.PodMap[podKey]; exists { t.Error("TestPodStatusUpdatePod failed @ cached pod obj exists check") } + + testutils.VerifyCallsMatch(t, calls, fexec, fcmd) } func TestHasValidPodIP(t *testing.T) { diff --git a/test/utils/utils.go b/test/utils/utils.go new file mode 100644 index 0000000000..0d33b491ea --- /dev/null +++ b/test/utils/utils.go @@ -0,0 +1,46 @@ +package testingutils + +import ( + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/utils/exec" + + fakeexec "k8s.io/utils/exec/testing" +) + +type TestCmd struct { + Cmd []string + Stderr string + ExitCode int +} + +func GetFakeExecWithScripts(calls []TestCmd) (*fakeexec.FakeExec, *fakeexec.FakeCmd) { + fexec := &fakeexec.FakeExec{ExactOrder: false, DisableScripts: false} + + fcmd := &fakeexec.FakeCmd{} + + for _, call := range calls { + if call.Stderr != "" || call.ExitCode != 0 { + stderr := call.Stderr + err := &fakeexec.FakeExitError{Status: call.ExitCode} + fcmd.CombinedOutputScript = append(fcmd.CombinedOutputScript, func() ([]byte, []byte, error) { return []byte(stderr), nil, err }) + } else { + fcmd.CombinedOutputScript = append(fcmd.CombinedOutputScript, func() ([]byte, []byte, error) { return []byte{}, nil, nil }) + } + } + + for range calls { + fexec.CommandScript = append(fexec.CommandScript, func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(fcmd, cmd, args...) }) + } + + return fexec, fcmd +} + +func VerifyCallsMatch(t *testing.T, calls []TestCmd, fexec *fakeexec.FakeExec, fcmd *fakeexec.FakeCmd) { + require.Equal(t, len(calls), fexec.CommandCalls) + + for i, call := range calls { + require.Equalf(t, call.Cmd, fcmd.CombinedOutputLog[i], "Call [%d] doesn't match expected", i) + } +}