From 1b93e5de2995d7bd35bb9a418b22b67b9b044e3a 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 Signed-off-by: Shereen Haj --- controllers/kubeletconfig_controller_test.go | 9 +- .../numaresourcesoperator_controller_test.go | 154 ++++++++++++++++-- internal/objects/objects.go | 9 +- internal/objects/objects_test.go | 11 +- pkg/status/status_test.go | 4 +- 5 files changed, 151 insertions(+), 36 deletions(-) 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..52a73cab5 100644 --- a/controllers/numaresourcesoperator_controller_test.go +++ b/controllers/numaresourcesoperator_controller_test.go @@ -106,29 +106,78 @@ 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) + }) + }) + + 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() { var nro *nropv1.NUMAResourcesOperator var mcp1 *machineconfigv1.MachineConfigPool @@ -136,6 +185,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 +195,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 +355,56 @@ 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() { + It("should create RTE daemonset and reflect status in NodeGroupStatus", func(ctx context.Context) { + mcpName := "test1" + label := map[string]string{ + "test1": "test1", + } + ng := nropv1.NodeGroup{ + PoolName: &mcpName, + } + + nro := testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, ng) + mcp := testobjs.NewMachineConfigPool(mcpName, label, &metav1.LabelSelector{MatchLabels: label}, &metav1.LabelSelector{MatchLabels: label}) + + reconciler := reconcileObjects(nro, mcp) + + mcpDSKey := client.ObjectKey{ + Name: objectnames.GetComponentName(nro.Name, mcp.Name), + Namespace: testNamespace, + } + ds := &appsv1.DaemonSet{} + Expect(reconciler.Client.Get(context.TODO(), mcpDSKey, ds)).ToNot(HaveOccurred()) + + nroUpdated := &nropv1.NUMAResourcesOperator{} + Expect(reconciler.Client.Get(context.TODO(), client.ObjectKeyFromObject(nro), nroUpdated)).ToNot(HaveOccurred()) + + Expect(len(nroUpdated.Status.MachineConfigPools)).To(Equal(1)) + Expect(nroUpdated.Status.MachineConfigPools[0].Name).To(Equal(mcp.Name)) + Expect(len(nroUpdated.Status.NodeGroups)).To(Equal(1)) + Expect(nroUpdated.Status.NodeGroups[0].PoolName).To(Equal(mcp.Name)) + // TODO check the actual returned config and the daemonset in status, we need to force the NRO condition as Available for this. + }) }) + Context("with correct NRO CR", func() { var nro *nropv1.NUMAResourcesOperator var mcp1 *machineconfigv1.MachineConfigPool @@ -315,10 +421,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}) @@ -1126,7 +1239,14 @@ func reconcileObjects(nro *nropv1.NUMAResourcesOperator, mcp *machineconfigv1.Ma Status: corev1.ConditionTrue, }, } + mcName := objectnames.GetMachineConfigName(nro.Name, mcp.Name) + mcp.Status.Configuration.Source[0].Name = mcName + Expect(reconciler.Client.Update(context.TODO(), mcp)).Should(Succeed()) + Expect(reconciler.Client.Get(context.TODO(), client.ObjectKeyFromObject(mcp), mcp)).ToNot(HaveOccurred()) + Expect(mcp.Status.Conditions[0].Type).To(Equal(machineconfigv1.MachineConfigPoolUpdated)) + Expect(mcp.Status.Conditions[0].Status).To(Equal(corev1.ConditionTrue)) + Expect(mcp.Status.Configuration.Source[0].Name).To(Equal(mcName)) var secondLoopResult reconcile.Result secondLoopResult, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) 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, "", "")