From 218320c1f64d53eb375e0929ae771afc32c8c97d Mon Sep 17 00:00:00 2001 From: Shereen Haj Date: Wed, 2 Oct 2024 18:12:38 +0300 Subject: [PATCH] controller: add tests related to PoolName Extend the controller tests coverage to verify PoolName scenarios. Signed-off-by: Shereen Haj l --- .../v1/helper/nodegroup/nodegroup.go | 5 +- controllers/kubeletconfig_controller_test.go | 9 +- .../numaresourcesoperator_controller_test.go | 126 +++++++++++++++--- internal/objects/objects.go | 9 +- internal/objects/objects_test.go | 11 +- pkg/status/status_test.go | 4 +- 6 files changed, 127 insertions(+), 37 deletions(-) diff --git a/api/numaresourcesoperator/v1/helper/nodegroup/nodegroup.go b/api/numaresourcesoperator/v1/helper/nodegroup/nodegroup.go index 148ecc51c..f00d3cb29 100644 --- a/api/numaresourcesoperator/v1/helper/nodegroup/nodegroup.go +++ b/api/numaresourcesoperator/v1/helper/nodegroup/nodegroup.go @@ -75,7 +75,10 @@ func FindTrees(mcps *mcov1.MachineConfigPoolList, nodeGroups []nropv1.NodeGroup) break } } - // it is valid if no match was found + // is it valid if no match was found? + //if len(treeMCPs) == 0 { + // return nil, fmt.Errorf("failed to find MachineConfigPool for the node group with the PoolName %q", nodeGroup.PoolName) + //} } if nodeGroup.MachineConfigPoolSelector != nil { diff --git a/controllers/kubeletconfig_controller_test.go b/controllers/kubeletconfig_controller_test.go index 019842a5b..7c6f07c76 100644 --- a/controllers/kubeletconfig_controller_test.go +++ b/controllers/kubeletconfig_controller_test.go @@ -65,9 +65,12 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { "test1": "test1", } mcp1 = testobjs.NewMachineConfigPool("test1", label1, &metav1.LabelSelector{MatchLabels: label1}, &metav1.LabelSelector{MatchLabels: label1}) - nro = testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, []*metav1.LabelSelector{ - {MatchLabels: label1}, - }) + ng := nropv1.NodeGroup{ + MachineConfigPoolSelector: &metav1.LabelSelector{ + MatchLabels: label1, + }, + } + nro = testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng) kubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{} mcoKc1 = testobjs.NewKubeletConfig("test1", label1, mcp1.Spec.MachineConfigSelector, kubeletConfig) }) diff --git a/controllers/numaresourcesoperator_controller_test.go b/controllers/numaresourcesoperator_controller_test.go index c07ac6534..d750235df 100644 --- a/controllers/numaresourcesoperator_controller_test.go +++ b/controllers/numaresourcesoperator_controller_test.go @@ -106,27 +106,84 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { Context("with unexpected NRO CR name", func() { It("should updated the CR condition to degraded", func() { - nro := testobjs.NewNUMAResourcesOperator("test", nil) + nro := testobjs.NewNUMAResourcesOperator("test") verifyDegradedCondition(nro, status.ConditionTypeIncorrectNUMAResourcesOperatorResourceName) }) }) - Context("with NRO empty machine config pool selector node group", func() { - It("should updated the CR condition to degraded", func() { - nro := testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, []*metav1.LabelSelector{nil}) + Context("with NRO empty selectors node group", func() { + It("should update the CR condition to degraded", func() { + ng := nropv1.NodeGroup{} + nro := testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng) verifyDegradedCondition(nro, validation.NodeGroupsError) }) }) - Context("without available machine config pools", func() { - It("should updated the CR condition to degraded", func() { - nro := testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, []*metav1.LabelSelector{ - { + Context("with NRO mutiple pool specifiers set on same node group", func() { + It("should update the CR condition to degraded", func() { + pn := "pn-1" + ng := nropv1.NodeGroup{ + MachineConfigPoolSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{"test": "test"}, }, - }) + PoolName: &pn, + } + nro := testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng) + verifyDegradedCondition(nro, validation.NodeGroupsError) + }) + }) + + Context("without available machine config pools", func() { + It("should update the CR condition to degraded when MachineConfigPoolSelector is set", func() { + ng := nropv1.NodeGroup{ + MachineConfigPoolSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"test": "test"}}, + } + nro := testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng) verifyDegradedCondition(nro, validation.NodeGroupsError) }) + //It("should update the CR condition to degraded when PoolName set", func() { + // pn := "pn-1" + // ng := nropv1.NodeGroup{ + // PoolName: &pn, + // } + // nro := testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng) + // verifyDegradedCondition(nro, validation.NodeGroupsError) + //}) + }) + + Context("with two node groups each with different pool specifier type and both point to same MCP", func() { + It("should update the CR condition to degraded", func() { + mcpName := "test1" + label1 := map[string]string{ + "test1": "test1", + } + + ng1 := nropv1.NodeGroup{ + MachineConfigPoolSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + } + ng2 := nropv1.NodeGroup{ + PoolName: &mcpName, + } + nro := testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng1, ng2) + + mcp1 := testobjs.NewMachineConfigPool(mcpName, label1, &metav1.LabelSelector{MatchLabels: label1}, &metav1.LabelSelector{MatchLabels: label1}) + + var err error + reconciler, err := NewFakeNUMAResourcesOperatorReconciler(platform.OpenShift, defaultOCPVersion, nro, mcp1) + Expect(err).ToNot(HaveOccurred()) + + key := client.ObjectKeyFromObject(nro) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + + Expect(reconciler.Client.Get(context.TODO(), key, nro)).ToNot(HaveOccurred()) + degradedCondition := getConditionByType(nro.Status.Conditions, status.ConditionDegraded) + Expect(degradedCondition.Status).To(Equal(metav1.ConditionTrue)) + Expect(degradedCondition.Reason).To(Equal(validation.NodeGroupsError)) + }) }) Context("with correct NRO and more than one NodeGroup", func() { @@ -136,6 +193,7 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { var reconciler *NUMAResourcesOperatorReconciler var label1, label2 map[string]string + var ng1, ng2 nropv1.NodeGroup BeforeEach(func() { label1 = map[string]string{ @@ -145,10 +203,17 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { "test2": "test2", } - nro = testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, []*metav1.LabelSelector{ - {MatchLabels: label1}, - {MatchLabels: label2}, - }) + ng1 = nropv1.NodeGroup{ + MachineConfigPoolSelector: &metav1.LabelSelector{ + MatchLabels: label1, + }, + } + ng2 = nropv1.NodeGroup{ + MachineConfigPoolSelector: &metav1.LabelSelector{ + MatchLabels: label2, + }, + } + nro = testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng1, ng2) mcp1 = testobjs.NewMachineConfigPool("test1", label1, &metav1.LabelSelector{MatchLabels: label1}, &metav1.LabelSelector{MatchLabels: label1}) mcp2 = testobjs.NewMachineConfigPool("test2", label2, &metav1.LabelSelector{MatchLabels: label2}, &metav1.LabelSelector{MatchLabels: label2}) @@ -298,7 +363,27 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { }) }) }) + When("add PoolName on existing node group with another specifier already exist", func() { + It("should update the CR condition to degraded", func(ctx context.Context) { + pn := "pool-1" + ng1WithNodeSelector := ng1.DeepCopy() + ng1WithNodeSelector.PoolName = &pn + key := client.ObjectKeyFromObject(nro) + Eventually(func() error { + nroUpdated := &nropv1.NUMAResourcesOperator{} + Expect(reconciler.Client.Get(ctx, key, nroUpdated)) + nroUpdated.Spec.NodeGroups[0] = *ng1WithNodeSelector + return reconciler.Client.Update(context.TODO(), nroUpdated) + }).WithPolling(1 * time.Second).WithTimeout(30 * time.Second).ShouldNot(HaveOccurred()) + verifyDegradedCondition(nro, validation.NodeGroupsError) + }) + }) }) + + Context("with correct NRO and more than one NodeGroup each with different pool specifier", func() { + //TODO + }) + Context("with correct NRO CR", func() { var nro *nropv1.NUMAResourcesOperator var mcp1 *machineconfigv1.MachineConfigPool @@ -315,10 +400,17 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { "test2": "test2", } - nro = testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, []*metav1.LabelSelector{ - {MatchLabels: label1}, - {MatchLabels: label2}, - }) + ng1 := nropv1.NodeGroup{ + MachineConfigPoolSelector: &metav1.LabelSelector{ + MatchLabels: label1, + }, + } + ng2 := nropv1.NodeGroup{ + MachineConfigPoolSelector: &metav1.LabelSelector{ + MatchLabels: label2, + }, + } + nro = testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng1, ng2) mcp1 = testobjs.NewMachineConfigPool("test1", label1, &metav1.LabelSelector{MatchLabels: label1}, &metav1.LabelSelector{MatchLabels: label1}) mcp2 = testobjs.NewMachineConfigPool("test2", label2, &metav1.LabelSelector{MatchLabels: label2}, &metav1.LabelSelector{MatchLabels: label2}) diff --git a/internal/objects/objects.go b/internal/objects/objects.go index b7b4acee5..cc0c5aff7 100644 --- a/internal/objects/objects.go +++ b/internal/objects/objects.go @@ -29,14 +29,7 @@ import ( nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" ) -func NewNUMAResourcesOperator(name string, labelSelectors []*metav1.LabelSelector) *nropv1.NUMAResourcesOperator { - var nodeGroups []nropv1.NodeGroup - for _, selector := range labelSelectors { - nodeGroups = append(nodeGroups, nropv1.NodeGroup{ - MachineConfigPoolSelector: selector, - }) - } - +func NewNUMAResourcesOperator(name string, nodeGroups ...nropv1.NodeGroup) *nropv1.NUMAResourcesOperator { return &nropv1.NUMAResourcesOperator{ TypeMeta: metav1.TypeMeta{ Kind: "NUMAResourcesOperator", diff --git a/internal/objects/objects_test.go b/internal/objects/objects_test.go index 061988593..e260bb129 100644 --- a/internal/objects/objects_test.go +++ b/internal/objects/objects_test.go @@ -28,15 +28,14 @@ import ( func TestNewNUMAResourcesOperator(t *testing.T) { name := "test-nrop" - labelSelectors := []*metav1.LabelSelector{ - { - MatchLabels: map[string]string{ - "unit-test-nrop-obj": "foobar", - }, + ng := nropv1.NodeGroup{ + MachineConfigPoolSelector: &metav1.LabelSelector{MatchLabels: map[string]string{ + "unit-test-nrop-obj": "foobar", + }, }, } - obj := NewNUMAResourcesOperator(name, labelSelectors) + obj := NewNUMAResourcesOperator(name, ng) if obj == nil { t.Fatalf("null object") diff --git a/pkg/status/status_test.go b/pkg/status/status_test.go index 42e947bac..3c8a0c6a2 100644 --- a/pkg/status/status_test.go +++ b/pkg/status/status_test.go @@ -36,7 +36,7 @@ func TestUpdate(t *testing.T) { t.Errorf("nropv1.AddToScheme() failed with: %v", err) } - nro := testobjs.NewNUMAResourcesOperator("test-nro", nil) + nro := testobjs.NewNUMAResourcesOperator("test-nro") fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(nro).Build() nro.Status.Conditions, _ = GetUpdatedConditions(nro.Status.Conditions, ConditionProgressing, "testReason", "test message") @@ -64,7 +64,7 @@ func TestUpdateIfNeeded(t *testing.T) { t.Errorf("nropv1.AddToScheme() failed with: %v", err) } - nro := testobjs.NewNUMAResourcesOperator("test-nro", nil) + nro := testobjs.NewNUMAResourcesOperator("test-nro") var ok bool nro.Status.Conditions, ok = GetUpdatedConditions(nro.Status.Conditions, ConditionAvailable, "", "")