diff --git a/cmd/ipam-controller/app/app_suite_test.go b/cmd/ipam-controller/app/app_suite_test.go index 6b2d901..c65b1bb 100644 --- a/cmd/ipam-controller/app/app_suite_test.go +++ b/cmd/ipam-controller/app/app_suite_test.go @@ -15,21 +15,15 @@ package app_test import ( "context" - "sync" "testing" - "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" - "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" - - "github.com/Mellanox/nvidia-k8s-ipam/cmd/ipam-controller/app" - "github.com/Mellanox/nvidia-k8s-ipam/cmd/ipam-controller/app/options" ) const ( @@ -43,7 +37,6 @@ var ( testEnv *envtest.Environment cFunc context.CancelFunc ctx context.Context - wg sync.WaitGroup ) func TestApp(t *testing.T) { @@ -68,24 +61,10 @@ var _ = BeforeSuite(func() { Expect(k8sClient).NotTo(BeNil()) Expect(k8sClient.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: TestNamespace}})).To(BeNil()) - - wg.Add(1) - go func() { - err := app.RunController(logr.NewContext(ctx, klog.NewKlogr()), cfg, &options.Options{ - ConfigMapName: TestConfigMapName, - ConfigMapNamespace: TestNamespace, - }) - if err != nil { - panic(err.Error()) - } - wg.Done() - }() }) var _ = AfterSuite(func() { - By("stop controller") cFunc() - wg.Wait() By("tearing down the test environment") err := testEnv.Stop() Expect(err).NotTo(HaveOccurred()) diff --git a/cmd/ipam-controller/app/app_test.go b/cmd/ipam-controller/app/app_test.go index 728e58f..4fda43c 100644 --- a/cmd/ipam-controller/app/app_test.go +++ b/cmd/ipam-controller/app/app_test.go @@ -14,20 +14,32 @@ package app_test import ( + "context" + "fmt" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" apiErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/klog/v2" + "github.com/Mellanox/nvidia-k8s-ipam/cmd/ipam-controller/app" + "github.com/Mellanox/nvidia-k8s-ipam/cmd/ipam-controller/app/options" "github.com/Mellanox/nvidia-k8s-ipam/pkg/ipam-controller/config" "github.com/Mellanox/nvidia-k8s-ipam/pkg/pool" ) +const ( + pool1Name = "pool1" + pool2Name = "pool2" + pool3Name = "pool3" +) + func updateConfigMap(data string) { d := map[string]string{config.ConfigMapKey: data} err := k8sClient.Create(ctx, &corev1.ConfigMap{ @@ -49,24 +61,24 @@ func updateConfigMap(data string) { } } -var validConfig = ` +var validConfig = fmt.Sprintf(` { "pools": { - "pool1": { "subnet": "192.168.0.0/16", "perNodeBlockSize": 10 , "gateway": "192.168.0.1"}, - "pool2": { "subnet": "172.16.0.0/16", "perNodeBlockSize": 50 , "gateway": "172.16.0.1"} + "%s": { "subnet": "192.168.0.0/16", "perNodeBlockSize": 10 , "gateway": "192.168.0.1"}, + "%s": { "subnet": "172.16.0.0/16", "perNodeBlockSize": 50 , "gateway": "172.16.0.1"} } } -` +`, pool1Name, pool2Name) // ranges for two nodes only can be allocated -var validConfig2 = ` +var validConfig2 = fmt.Sprintf(` { "pools": { - "pool3": { "subnet": "172.17.0.0/24", "perNodeBlockSize": 100 , "gateway": "172.17.0.1"} + "%s": { "subnet": "172.17.0.0/24", "perNodeBlockSize": 100 , "gateway": "172.17.0.1"} }, "nodeSelector": {"foo": "bar"} } -` +`, pool3Name) var invalidConfig = `{{{` @@ -104,91 +116,150 @@ func WaitAndCheckForStability(check func(g Gomega), wait interface{}, stability var _ = Describe("App", func() { It("Basic tests", func() { + done := make(chan interface{}) + go func() { + testNode1 := "node1" + testNode2 := "node2" + testNode3 := "node3" + + cfg1pools := []string{pool1Name, pool2Name} + + By("Create valid cfg1") + updateConfigMap(validConfig) + + By("Set annotation with valid ranges for node1") + node1 := createNode(testNode1) + node1InitialRanges := map[string]*pool.IPPool{pool1Name: { + Name: pool1Name, + Subnet: "192.168.0.0/16", + StartIP: "192.168.0.11", + EndIP: "192.168.0.20", + Gateway: "192.168.0.1", + }, pool2Name: { + Name: pool2Name, + Subnet: "172.16.0.0/16", + StartIP: "172.16.0.1", + EndIP: "172.16.0.50", + Gateway: "172.16.0.1", + }} + Expect(pool.SetIPBlockAnnotation(node1, node1InitialRanges)).NotTo(HaveOccurred()) + Expect(updateNode(node1)) + + By("Set annotation with invalid ranges for node2") + // create annotation for node2 with invalid config (wrong IP count) + node2 := createNode(testNode2) + node2InitialRanges := map[string]*pool.IPPool{pool1Name: { + Name: pool1Name, + Subnet: "192.168.0.0/16", + StartIP: "192.168.0.11", + EndIP: "192.168.0.14", + Gateway: "192.168.0.1", + }} + Expect(pool.SetIPBlockAnnotation(node2, node2InitialRanges)).NotTo(HaveOccurred()) + Expect(updateNode(node2)) - By("Create config and wait for controller to set ranges") - - updateConfigMap(validConfig) - testNode1 := "node1" - testNode2 := "node2" - testNode3 := "node3" - - pools := []string{"pool1", "pool2"} - - createNode(testNode1) - createNode(testNode2) - createNode(testNode3) - - WaitAndCheckForStability(func(g Gomega) { - uniqStartEndIPs := map[string]struct{}{} - node1Ranges := getRangeFromNode(testNode1) - node2Ranges := getRangeFromNode(testNode2) - node3Ranges := getRangeFromNode(testNode3) - for _, r := range []map[string]*pool.IPPool{node1Ranges, node2Ranges, node3Ranges} { - g.Expect(r).To(HaveLen(len(pools))) - for _, p := range pools { - if r[p] != nil { - uniqStartEndIPs[r[p].StartIP] = struct{}{} - uniqStartEndIPs[r[p].EndIP] = struct{}{} - g.Expect(r[p].Gateway).NotTo(BeEmpty()) - g.Expect(r[p].Subnet).NotTo(BeEmpty()) + By("Start controller") + + ctrlCtx, ctrlStopFunc := context.WithCancel(ctx) + defer ctrlStopFunc() + controllerStopped := make(chan struct{}) + + go func() { + Expect(app.RunController(logr.NewContext(ctrlCtx, klog.NewKlogr()), cfg, &options.Options{ + ConfigMapName: TestConfigMapName, + ConfigMapNamespace: TestNamespace, + })).NotTo(HaveOccurred()) + close(controllerStopped) + }() + + By("Create node3 without annotation") + + createNode(testNode3) + + By("Check how controller handled the state") + WaitAndCheckForStability(func(g Gomega) { + uniqStartEndIPs := map[string]struct{}{} + node1Ranges := getRangeFromNode(testNode1) + node2Ranges := getRangeFromNode(testNode2) + node3Ranges := getRangeFromNode(testNode3) + for _, r := range []map[string]*pool.IPPool{node1Ranges, node2Ranges, node3Ranges} { + g.Expect(r).To(HaveLen(len(cfg1pools))) + for _, p := range cfg1pools { + if r[p] != nil { + uniqStartEndIPs[r[p].StartIP] = struct{}{} + uniqStartEndIPs[r[p].EndIP] = struct{}{} + g.Expect(r[p].Gateway).NotTo(BeEmpty()) + g.Expect(r[p].Subnet).NotTo(BeEmpty()) + } } } - } - // we should have unique start/end IPs for each node for each pool - g.Expect(uniqStartEndIPs).To(HaveLen(len(pools) * 3 * 2)) - }, 15, 2) - - By("Set invalid config") - updateConfigMap(invalidConfig) - time.Sleep(time.Second) - - By("Set valid config, which ignores all nodes") - - updateConfigMap(validConfig2) - - WaitAndCheckForStability(func(g Gomega) { - g.Expect(getRangeFromNode(testNode1)).To(BeNil()) - g.Expect(getRangeFromNode(testNode2)).To(BeNil()) - g.Expect(getRangeFromNode(testNode3)).To(BeNil()) - }, 15, 2) - - By("Update node to match selector") - - // node1 should have range - node1 := getNode(testNode1) - node1.Labels = map[string]string{"foo": "bar"} - updateNode(node1) - WaitAndCheckForStability(func(g Gomega) { - g.Expect(getRangeFromNode(testNode1)).NotTo(BeNil()) - }, 15, 2) - - // node2 should have range - node2 := getNode(testNode2) - node2.Labels = map[string]string{"foo": "bar"} - var node2Ranges map[string]*pool.IPPool - updateNode(node2) - WaitAndCheckForStability(func(g Gomega) { - node2Ranges = getRangeFromNode(testNode2) - g.Expect(node2Ranges).NotTo(BeNil()) - }, 15, 2) - - // node3 should have no range, because no free ranges available - node3 := getNode(testNode3) - node3.Labels = map[string]string{"foo": "bar"} - updateNode(node3) - WaitAndCheckForStability(func(g Gomega) { - g.Expect(getRangeFromNode(testNode3)).To(BeNil()) - }, 15, 5) - - // remove label from node2, node3 should have a range now - node2 = getNode(testNode2) - node2.Labels = nil - updateNode(node2) - WaitAndCheckForStability(func(g Gomega) { - node3Ranges := getRangeFromNode(testNode3) - g.Expect(node3Ranges).NotTo(BeNil()) - // should reuse ranges from node2 - g.Expect(node3Ranges).To(Equal(node2Ranges)) - }, 15, 2) + // we should have unique start/end IPs for each node for each pool + g.Expect(uniqStartEndIPs).To(HaveLen(len(cfg1pools) * 3 * 2)) + // node1 should have restored ranges + g.Expect(node1Ranges).To(Equal(node1InitialRanges)) + + }, 15, 2) + + By("Set invalid config for controller") + updateConfigMap(invalidConfig) + time.Sleep(time.Second) + + By("Set valid cfg2, which ignores all nodes") + + updateConfigMap(validConfig2) + + By("Wait for controller to remove annotations") + WaitAndCheckForStability(func(g Gomega) { + g.Expect(getRangeFromNode(testNode1)).To(BeNil()) + g.Expect(getRangeFromNode(testNode2)).To(BeNil()) + g.Expect(getRangeFromNode(testNode3)).To(BeNil()) + }, 15, 2) + + By("Update nodes to match selector in cfg2") + + // node1 should have range + node1 = getNode(testNode1) + node1.Labels = map[string]string{"foo": "bar"} + updateNode(node1) + WaitAndCheckForStability(func(g Gomega) { + g.Expect(getRangeFromNode(testNode1)).NotTo(BeNil()) + }, 15, 2) + + // node2 should have range + node2 = getNode(testNode2) + node2.Labels = map[string]string{"foo": "bar"} + var node2Ranges map[string]*pool.IPPool + updateNode(node2) + WaitAndCheckForStability(func(g Gomega) { + node2Ranges = getRangeFromNode(testNode2) + g.Expect(node2Ranges).NotTo(BeNil()) + }, 15, 2) + + // node3 should have no range, because no free ranges available + node3 := getNode(testNode3) + node3.Labels = map[string]string{"foo": "bar"} + updateNode(node3) + WaitAndCheckForStability(func(g Gomega) { + g.Expect(getRangeFromNode(testNode3)).To(BeNil()) + }, 15, 5) + + // remove label from node2, node3 should have a range now + node2 = getNode(testNode2) + node2.Labels = nil + updateNode(node2) + WaitAndCheckForStability(func(g Gomega) { + node3Ranges := getRangeFromNode(testNode3) + g.Expect(node3Ranges).NotTo(BeNil()) + // should reuse ranges from node2 + g.Expect(node3Ranges).To(Equal(node2Ranges)) + }, 15, 2) + + By("Stop controller") + ctrlStopFunc() + Eventually(controllerStopped, 30*time.Second).Should(BeClosed()) + + close(done) + }() + Eventually(done, 5*time.Minute).Should(BeClosed()) }) }) diff --git a/pkg/ipam-controller/allocator/allocator.go b/pkg/ipam-controller/allocator/allocator.go index c631513..bbf17b4 100644 --- a/pkg/ipam-controller/allocator/allocator.go +++ b/pkg/ipam-controller/allocator/allocator.go @@ -136,7 +136,7 @@ func (pa *poolAllocator) Deallocate(ctx context.Context, node string) { // Load loads range to the pool allocator with validation for conflicts func (pa *poolAllocator) Load(ctx context.Context, allocData nodeAllocationInfo) error { - log := pa.getLog(ctx, pa.cfg) + log := pa.getLog(ctx, pa.cfg).WithValues("node", allocData.Node) if err := pa.checkAllocation(allocData); err != nil { log.Info("range check failed", "reason", err.Error()) return err @@ -151,6 +151,7 @@ func (pa *poolAllocator) Load(ctx context.Context, allocData nodeAllocationInfo) return err } } + log.Info("data loaded", "startIP", allocData.StartIP, "endIP", allocData.EndIP) pa.allocations[allocData.Node] = allocData.allocatedRange return nil } @@ -238,21 +239,27 @@ func (a *Allocator) ConfigureAndLoadAllocations(ctx context.Context, configs []A a.configure(ctx, configs) for i := range nodes { node := nodes[i] + nodeLog := log.WithValues("node", node.Name) nodePoolMgr, err := pool.NewManagerImpl(&node) if err != nil { - log.Info("skip loading data from the node", "reason", err.Error()) + nodeLog.Info("skip loading data from the node", "reason", err.Error()) continue } // load allocators only for know pools (pools which are defined in the config) for poolName, poolData := range a.allocators { nodeIPPoolConfig := nodePoolMgr.GetPoolByName(poolName) allocInfo, err := ipPoolConfigToNodeAllocationInfo(node.Name, nodeIPPoolConfig) + logErr := func(err error) { + nodeLog.Info("ignore allocation info from node", + "pool", poolName, "reason", err.Error()) + } if err != nil { - log.Info("ignore allocation info from node", "reason", err.Error()) + logErr(err) continue } if err := poolData.Load(ctx, allocInfo); err != nil { + logErr(err) continue } a.allocators[poolName] = poolData