diff --git a/controllers/suite_test.go b/controllers/controllers_suite_test.go similarity index 66% rename from controllers/suite_test.go rename to controllers/controllers_suite_test.go index 09474d2d..e333227e 100644 --- a/controllers/suite_test.go +++ b/controllers/controllers_suite_test.go @@ -21,6 +21,7 @@ import ( "path/filepath" "testing" + "github.com/medik8s/common/pkg/lease" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -31,7 +32,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" - metricsServer "sigs.k8s.io/controller-runtime/pkg/metrics/server" nodemaintenancev1beta1 "github.com/medik8s/node-maintenance-operator/api/v1beta1" //+kubebuilder:scaffold:imports @@ -40,10 +40,14 @@ import ( // These tests use Ginkgo (BDD-style Go testing framework). Refer to // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. -var cfg *rest.Config -var k8sClient client.Client -var testEnv *envtest.Environment -var ctxFromSignalHandler context.Context +var ( + cfg *rest.Config + k8sClient client.Client + testEnv *envtest.Environment + ctx context.Context + cancel context.CancelFunc + r *NodeMaintenanceReconciler +) func TestControllers(t *testing.T) { RegisterFailHandler(Fail) @@ -55,9 +59,6 @@ var _ = BeforeSuite(func() { // call start or refactor when moving to "normal" testEnv test -}) - -func startTestEnv() { By("bootstrapping test environment") testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")}, @@ -74,40 +75,38 @@ func startTestEnv() { Expect(err).NotTo(HaveOccurred()) //+kubebuilder:scaffold:scheme + k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{Scheme: scheme.Scheme}) + Expect(err).NotTo(HaveOccurred()) + Expect(k8sManager).NotTo(BeNil()) k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) - k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ - Scheme: scheme.Scheme, - Metrics: metricsServer.Options{BindAddress: "0"}, - }) - Expect(err).ToNot(HaveOccurred()) + mockManager, _ := lease.NewManager(k8sClient, "") + // Create a ReconcileNodeMaintenance object with the scheme and fake client + r = &NodeMaintenanceReconciler{ + Client: k8sClient, + Scheme: scheme.Scheme, + LeaseManager: &mockLeaseManager{mockManager}, + logger: ctrl.Log.WithName("unit test"), + } + Expect(initDrainer(r, cfg)).To(Succeed()) + // in test pods are not evicted, so don't wait forever for them + r.drainer.SkipWaitForDeleteTimeoutSeconds = 0 - // comment in when moving to "normal" testEnv test - // this isn't needed atm, because the controller tests call relevant funcs of the controller themself - //err = (&NodeMaintenanceReconciler{ - // Client: k8sManager.GetClient(), - // Scheme: k8sManager.GetScheme(), - //}).SetupWithManager(k8sManager) - //Expect(err).ToNot(HaveOccurred()) + err = (r).SetupWithManager(k8sManager) + Expect(err).NotTo(HaveOccurred()) go func() { - if ctxFromSignalHandler == nil { - ctxFromSignalHandler = ctrl.SetupSignalHandler() - } - err = k8sManager.Start(ctxFromSignalHandler) - Expect(err).ToNot(HaveOccurred()) + // https://github.com/kubernetes-sigs/controller-runtime/issues/1571 + ctx, cancel = context.WithCancel(ctrl.SetupSignalHandler()) + Expect(k8sManager.Start(ctx)).To(Succeed()) }() -} - -var _ = AfterSuite(func() { - // call stop or refactor when moving to "normal" testEnv test }) -func stopTestEnv() { +var _ = AfterSuite(func() { By("tearing down the test environment") - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) -} + cancel() + Expect(testEnv.Stop()).To(Succeed()) +}) diff --git a/controllers/nodemaintenance_controller.go b/controllers/nodemaintenance_controller.go index 8bf003fe..3530fa43 100644 --- a/controllers/nodemaintenance_controller.go +++ b/controllers/nodemaintenance_controller.go @@ -138,7 +138,7 @@ func (r *NodeMaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ return reconcile.Result{}, nil } - err = r.initMaintenanceStatus(instance) + err = initMaintenanceStatus(instance, r.drainer, r.Client) if err != nil { r.logger.Error(err, "Failed to update NodeMaintenance with \"Running\" status") return r.onReconcileError(instance, err) @@ -152,7 +152,7 @@ func (r *NodeMaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ return r.onReconcileError(instance, err) } - r.setOwnerRefToNode(instance, node) + setOwnerRefToNode(instance, node, r.logger) updateOwnedLeaseFailed, err := r.obtainLease(node) if err != nil && updateOwnedLeaseFailed { @@ -180,7 +180,7 @@ func (r *NodeMaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ } } //Add exclude from remediation label - if err := r.addExcludeRemediationLabel(ctx, node); err != nil { + if err := addExcludeRemediationLabel(ctx, node, r.Client, r.logger); err != nil { return r.onReconcileError(instance, err) } @@ -285,15 +285,15 @@ func initDrainer(r *NodeMaintenanceReconciler, config *rest.Config) error { return nil } -func (r *NodeMaintenanceReconciler) setOwnerRefToNode(instance *nodemaintenancev1beta1.NodeMaintenance, node *corev1.Node) { +func setOwnerRefToNode(nm *nodemaintenancev1beta1.NodeMaintenance, node *corev1.Node, log logr.Logger) { - for _, ref := range instance.ObjectMeta.GetOwnerReferences() { + for _, ref := range nm.ObjectMeta.GetOwnerReferences() { if ref.APIVersion == node.TypeMeta.APIVersion && ref.Kind == node.TypeMeta.Kind && ref.Name == node.ObjectMeta.GetName() && ref.UID == node.ObjectMeta.GetUID() { return } } - r.logger.Info("setting owner ref to node") + log.Info("setting owner ref to node") nodeMeta := node.TypeMeta ref := metav1.OwnerReference{ @@ -305,7 +305,7 @@ func (r *NodeMaintenanceReconciler) setOwnerRefToNode(instance *nodemaintenancev Controller: pointer.Bool(false), } - instance.ObjectMeta.SetOwnerReferences(append(instance.ObjectMeta.GetOwnerReferences(), ref)) + nm.ObjectMeta.SetOwnerReferences(append(nm.ObjectMeta.GetOwnerReferences(), ref)) } func (r *NodeMaintenanceReconciler) obtainLease(node *corev1.Node) (bool, error) { @@ -320,7 +320,7 @@ func (r *NodeMaintenanceReconciler) obtainLease(node *corev1.Node) (bool, error) return false, nil } -func (r *NodeMaintenanceReconciler) addExcludeRemediationLabel(ctx context.Context, node *corev1.Node) error { +func addExcludeRemediationLabel(ctx context.Context, node *corev1.Node, r client.Client, log logr.Logger) error { if node.Labels[labels.ExcludeFromRemediation] != "true" { patch := client.MergeFrom(node.DeepCopy()) if node.Labels == nil { @@ -328,20 +328,20 @@ func (r *NodeMaintenanceReconciler) addExcludeRemediationLabel(ctx context.Conte } else if node.Labels[labels.ExcludeFromRemediation] != "true" { node.Labels[labels.ExcludeFromRemediation] = "true" } - if err := r.Client.Patch(ctx, node, patch); err != nil { - r.logger.Error(err, "Failed to add exclude from remediation label from the node", "node name", node.Name) + if err := r.Patch(ctx, node, patch); err != nil { + log.Error(err, "Failed to add exclude from remediation label from the node", "node name", node.Name) return err } } return nil } -func (r *NodeMaintenanceReconciler) removeExcludeRemediationLabel(ctx context.Context, node *corev1.Node) error { +func removeExcludeRemediationLabel(ctx context.Context, node *corev1.Node, r client.Client, log logr.Logger) error { if node.Labels[labels.ExcludeFromRemediation] == "true" { patch := client.MergeFrom(node.DeepCopy()) delete(node.Labels, labels.ExcludeFromRemediation) - if err := r.Client.Patch(ctx, node, patch); err != nil { - r.logger.Error(err, "Failed to remove exclude from remediation label from the node", "node name", node.Name) + if err := r.Patch(ctx, node, patch); err != nil { + log.Error(err, "Failed to remove exclude from remediation label from the node", "node name", node.Name) return err } } @@ -362,7 +362,7 @@ func (r *NodeMaintenanceReconciler) stopNodeMaintenanceImp(ctx context.Context, if err := r.LeaseManager.InvalidateLease(ctx, node); err != nil { return err } - return r.removeExcludeRemediationLabel(ctx, node) + return removeExcludeRemediationLabel(ctx, node, r.Client, r.logger) } func (r *NodeMaintenanceReconciler) stopNodeMaintenanceOnDeletion(ctx context.Context, nodeName string) error { @@ -392,11 +392,11 @@ func (r *NodeMaintenanceReconciler) fetchNode(nodeName string) (*corev1.Node, er return node, nil } -func (r *NodeMaintenanceReconciler) initMaintenanceStatus(nm *nodemaintenancev1beta1.NodeMaintenance) error { +func initMaintenanceStatus(nm *nodemaintenancev1beta1.NodeMaintenance, drainer *drain.Helper, r client.Client) error { if nm.Status.Phase == "" { nm.Status.Phase = nodemaintenancev1beta1.MaintenanceRunning setLastUpdate(nm) - pendingList, errlist := r.drainer.GetPodsForDeletion(nm.Spec.NodeName) + pendingList, errlist := drainer.GetPodsForDeletion(nm.Spec.NodeName) if errlist != nil { return fmt.Errorf("failed to get pods for eviction while initializing status") } @@ -405,7 +405,7 @@ func (r *NodeMaintenanceReconciler) initMaintenanceStatus(nm *nodemaintenancev1b } nm.Status.EvictionPods = len(nm.Status.PendingPods) - podlist, err := r.drainer.Client.CoreV1().Pods(metav1.NamespaceAll).List( + podlist, err := drainer.Client.CoreV1().Pods(metav1.NamespaceAll).List( context.Background(), metav1.ListOptions{ FieldSelector: fields.SelectorFromSet(fields.Set{"spec.nodeName": nm.Spec.NodeName}).String(), @@ -414,7 +414,7 @@ func (r *NodeMaintenanceReconciler) initMaintenanceStatus(nm *nodemaintenancev1b return err } nm.Status.TotalPods = len(podlist.Items) - err = r.Client.Status().Update(context.TODO(), nm) + err = r.Status().Update(context.TODO(), nm) return err } return nil diff --git a/controllers/nodemaintenance_controller_test.go b/controllers/nodemaintenance_controller_test.go index 06b8ddea..93230620 100644 --- a/controllers/nodemaintenance_controller_test.go +++ b/controllers/nodemaintenance_controller_test.go @@ -2,338 +2,346 @@ package controllers import ( "context" + "fmt" "reflect" "time" - "github.com/medik8s/common/pkg/labels" + commonLabels "github.com/medik8s/common/pkg/labels" "github.com/medik8s/common/pkg/lease" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" 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/client-go/kubernetes/scheme" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/reconcile" nodemaintenanceapi "github.com/medik8s/node-maintenance-operator/api/v1beta1" ) -var _ = Describe("Node Maintenance", func() { - - var r *NodeMaintenanceReconciler - var nm *nodemaintenanceapi.NodeMaintenance - var req reconcile.Request - var clObjs []client.Object - - checkSuccesfulReconcile := func() { - maintenance := &nodemaintenanceapi.NodeMaintenance{} - err := k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(nm), maintenance) - Expect(err).NotTo(HaveOccurred()) - Expect(maintenance.Status.Phase).To(Equal(nodemaintenanceapi.MaintenanceSucceeded)) - Expect(maintenance.Status.DrainProgress).To(Equal(100)) - } +const ( + taintedNodeName = "node01" + invalidNodeName = "non-existing" + dummyTaintKey = "dummy-key" + dummyLabelKey = "dummyLabel" + dummyLabelValue = "true" + testNamespace = "test-namespace" +) - checkFailedReconcile := func() { - maintenance := &nodemaintenanceapi.NodeMaintenance{} - err := k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(nm), maintenance) - Expect(err).NotTo(HaveOccurred()) - Expect(len(maintenance.Status.LastError)).NotTo(Equal(0)) - } +var testLog = ctrl.Log.WithName("nmo-controllers-unit-test") +var _ = Describe("Node Maintenance", func() { - reconcileMaintenance := func(nm *nodemaintenanceapi.NodeMaintenance) { - _, _ = r.Reconcile(context.Background(), req) - } - - taintExist := func(node *corev1.Node, key string, effect corev1.TaintEffect) bool { - checkTaint := corev1.Taint{ - Key: key, - Effect: effect, - } - taints := node.Spec.Taints - for _, taint := range taints { - if reflect.DeepEqual(taint, checkTaint) { - return true - } - } - return false - } + var ( + nodeOne *corev1.Node + podOne, podTwo *corev1.Pod + ) BeforeEach(func() { - - startTestEnv() - - // Create a ReconcileNodeMaintenance object with the scheme and fake client - // TODO add reconciler to manager in suite_test.go and don't call reconcile funcs manually - manager, _ := lease.NewManager(k8sClient, "") - r = &NodeMaintenanceReconciler{ - Client: k8sClient, - Scheme: scheme.Scheme, - LeaseManager: &mockLeaseManager{manager}, - logger: ctrl.Log.WithName("unit test"), - } - _ = initDrainer(r, cfg) - - // in test pods are not evicted, so don't wait forever for them - r.drainer.SkipWaitForDeleteTimeoutSeconds = 0 - - var objs []client.Object - nm, objs = getTestObjects() - clObjs = append(objs, nm) - - // create test ns on 1st run + // create testNamespace ns testNs := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: "test", + Name: testNamespace, }, } - if err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(testNs), &corev1.Namespace{}); err != nil { - err := k8sClient.Create(context.Background(), testNs) - Expect(err).ToNot(HaveOccurred()) - } - - for _, o := range clObjs { - err := k8sClient.Create(context.Background(), o) - Expect(err).ToNot(HaveOccurred()) - } - - // Mock request to simulate Reconcile() being called on an event for a watched resource . - req = reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: nm.ObjectMeta.Name, - }, + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(testNs), &corev1.Namespace{}); err != nil { + Expect(k8sClient.Create(ctx, testNs)).To(Succeed()) } + nodeOne = getNode(taintedNodeName) }) - - AfterEach(func() { - stopTestEnv() - }) - - Context("Initialization test", func() { - - It("Node maintenance should be initialized properly", func() { - _ = r.initMaintenanceStatus(nm) - maintenance := &nodemaintenanceapi.NodeMaintenance{} - err := k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(nm), maintenance) - Expect(err).NotTo(HaveOccurred()) - Expect(maintenance.Status.Phase).To(Equal(nodemaintenanceapi.MaintenanceRunning)) - Expect(len(maintenance.Status.PendingPods)).To(Equal(2)) - Expect(maintenance.Status.EvictionPods).To(Equal(2)) - Expect(maintenance.Status.TotalPods).To(Equal(2)) - Expect(maintenance.Status.DrainProgress).To(Equal(0)) - Expect(maintenance.Status.LastUpdate.IsZero()).To(BeFalse()) - }) - - It("owner ref should be set properly", func() { - _ = r.initMaintenanceStatus(nm) - maintenance := &nodemaintenanceapi.NodeMaintenance{} - err := k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(nm), maintenance) - node := &corev1.Node{} - err = k8sClient.Get(context.TODO(), client.ObjectKey{Name: "node01"}, node) - Expect(err).ToNot(HaveOccurred()) - r.setOwnerRefToNode(maintenance, node) - Expect(len(maintenance.ObjectMeta.GetOwnerReferences())).To(Equal(1)) - ref := maintenance.ObjectMeta.GetOwnerReferences()[0] - Expect(ref.Name).To(Equal(node.ObjectMeta.Name)) - Expect(ref.UID).To(Equal(node.ObjectMeta.UID)) - Expect(ref.APIVersion).To(Equal(node.TypeMeta.APIVersion)) - Expect(ref.Kind).To(Equal(node.TypeMeta.Kind)) - r.setOwnerRefToNode(maintenance, node) - Expect(len(maintenance.ObjectMeta.GetOwnerReferences())).To(Equal(1)) + Context("Functionality test", func() { + Context("Testing initMaintenanceStatus", func() { + var nm *nodemaintenanceapi.NodeMaintenance + BeforeEach(func() { + Expect(k8sClient.Create(ctx, nodeOne)).To(Succeed()) + DeferCleanup(k8sClient.Delete, ctx, nodeOne) + + podOne, podTwo = getTestPod("test-pod-1", taintedNodeName), getTestPod("test-pod-2", taintedNodeName) + Expect(k8sClient.Create(ctx, podOne)).To(Succeed()) + Expect(k8sClient.Create(ctx, podTwo)).To(Succeed()) + DeferCleanup(cleanupPod, ctx, podOne) + DeferCleanup(cleanupPod, ctx, podTwo) + nm = getTestNM("node-maintenance-cr-initMaintenanceStatus", taintedNodeName) + }) + When("Status was initalized", func() { + It("should be set for running with 2 pods to drain", func() { + Expect(initMaintenanceStatus(nm, r.drainer, r.Client)).To(HaveOccurred()) + // status was initialized but the function will fail on updating the CR status, since we don't create a nm CR here + Expect(nm.Status.Phase).To(Equal(nodemaintenanceapi.MaintenanceRunning)) + Expect(len(nm.Status.PendingPods)).To(Equal(2)) + Expect(nm.Status.EvictionPods).To(Equal(2)) + Expect(nm.Status.TotalPods).To(Equal(2)) + Expect(nm.Status.DrainProgress).To(Equal(0)) + Expect(nm.Status.LastUpdate).NotTo(BeZero()) + }) + }) + When("Owner ref was set", func() { + It("should be set properly", func() { + Expect(initMaintenanceStatus(nm, r.drainer, r.Client)).To(HaveOccurred()) + // status was initialized but the function will fail on updating the CR status, since we don't create a nm CR here + By("Setting owner ref for a modified nm CR") + node := &corev1.Node{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: taintedNodeName}, node)).To(Succeed()) + setOwnerRefToNode(nm, node, r.logger) + Expect(nm.ObjectMeta.GetOwnerReferences()).To(HaveLen(1)) + ref := nm.ObjectMeta.GetOwnerReferences()[0] + Expect(ref.Name).To(Equal(node.ObjectMeta.Name)) + Expect(ref.UID).To(Equal(node.ObjectMeta.UID)) + Expect(ref.APIVersion).To(Equal(node.TypeMeta.APIVersion)) + Expect(ref.Kind).To(Equal(node.TypeMeta.Kind)) + + By("Setting owner ref for an empty nm CR") + maintenance := &nodemaintenanceapi.NodeMaintenance{} + setOwnerRefToNode(maintenance, node, r.logger) + Expect(maintenance.ObjectMeta.GetOwnerReferences()).To(HaveLen(1)) + }) + }) + When("Node Maintenance CR was initalized", func() { + It("Should not modify the CR after initalization", func() { + nmCopy := nm.DeepCopy() + nmCopy.Status.Phase = nodemaintenanceapi.MaintenanceFailed + Expect(initMaintenanceStatus(nmCopy, r.drainer, r.Client)).To(Succeed()) + // status was not initialized thus the function succeeds + Expect(nmCopy.Status.Phase).To(Equal(nodemaintenanceapi.MaintenanceFailed)) + Expect(len(nmCopy.Status.PendingPods)).To(Equal(0)) + Expect(nmCopy.Status.EvictionPods).To(Equal(0)) + Expect(nmCopy.Status.TotalPods).To(Equal(0)) + Expect(nmCopy.Status.DrainProgress).To(Equal(0)) + Expect(nmCopy.Status.LastUpdate).To(BeZero()) + }) + }) }) - It("Should not init Node maintenance if already set", func() { - nmCopy := nm.DeepCopy() - nmCopy.Status.Phase = nodemaintenanceapi.MaintenanceRunning - _ = r.initMaintenanceStatus(nmCopy) - maintenance := &nodemaintenanceapi.NodeMaintenance{} - err := k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(nm), maintenance) - Expect(err).NotTo(HaveOccurred()) - Expect(maintenance.Status.Phase).NotTo(Equal(nodemaintenanceapi.MaintenanceRunning)) - Expect(len(maintenance.Status.PendingPods)).NotTo(Equal(2)) - Expect(maintenance.Status.EvictionPods).NotTo(Equal(2)) - Expect(maintenance.Status.TotalPods).NotTo(Equal(2)) - Expect(maintenance.Status.DrainProgress).To(Equal(0)) - Expect(maintenance.Status.LastUpdate.IsZero()).To(BeTrue()) + Context("Testing exclude remediation label", func() { + BeforeEach(func() { + Expect(k8sClient.Create(ctx, nodeOne)).To(Succeed()) + DeferCleanup(k8sClient.Delete, ctx, nodeOne) + }) + When("Adding and removing exclude remediation label", func() { + It("should keep the dummy label", func() { + node := &corev1.Node{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: taintedNodeName}, node)).To(Succeed()) + Expect(len(node.Labels)).To(Equal(1)) + By("Adding exclude remediation label") + Expect(addExcludeRemediationLabel(ctx, node, r.Client, testLog)).To(Succeed()) + labeledNode := &corev1.Node{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: taintedNodeName}, labeledNode)).To(Succeed()) + Expect(isLabelExist(labeledNode, commonLabels.ExcludeFromRemediation)).To(BeTrue()) + Expect(len(labeledNode.Labels)).To(Equal(2)) + By("Removing exclude remediation label") + Expect(removeExcludeRemediationLabel(ctx, node, r.Client, testLog)).To(Succeed()) + unlabeledNode := &corev1.Node{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: taintedNodeName}, unlabeledNode)).To(Succeed()) + Expect(isLabelExist(unlabeledNode, commonLabels.ExcludeFromRemediation)).To(BeFalse()) + By("Finding dummy label") + Expect(isLabelExist(unlabeledNode, dummyLabelKey)).To(BeTrue()) + Expect(len(unlabeledNode.Labels)).To(Equal(1)) + }) + }) }) - - }) - - Context("Taint functioninality test", func() { - It("should add medik8s NoSchedule taint and keep other existing taints", func() { - node := &corev1.Node{} - err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "node01"}, node) - Expect(err).NotTo(HaveOccurred()) - _ = AddOrRemoveTaint(r.drainer.Client, node, true) - taintedNode := &corev1.Node{} - err = k8sClient.Get(context.TODO(), client.ObjectKey{Name: "node01"}, taintedNode) - Expect(err).NotTo(HaveOccurred()) - Expect(taintExist(taintedNode, "medik8s.io/drain", corev1.TaintEffectNoSchedule)).To(BeTrue()) - Expect(taintExist(taintedNode, "node.kubernetes.io/unschedulable", corev1.TaintEffectNoSchedule)).To(BeTrue()) - Expect(taintExist(taintedNode, "test", corev1.TaintEffectPreferNoSchedule)).To(BeTrue()) - - // there is a not-ready taint now as well... skip count tests - //Expect(len(taintedNode.Spec.Taints)).To(Equal(3)) - }) - - It("should remove medik8s NoSchedule taint and keep other existing taints", func() { - node := &corev1.Node{} - err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "node01"}, node) - Expect(err).NotTo(HaveOccurred()) - Expect(taintExist(node, "medik8s.io/drain", corev1.TaintEffectNoSchedule)).To(BeFalse()) - _ = AddOrRemoveTaint(r.drainer.Client, node, true) - taintedNode := &corev1.Node{} - err = k8sClient.Get(context.TODO(), client.ObjectKey{Name: "node01"}, taintedNode) - Expect(err).ToNot(HaveOccurred()) - Expect(taintExist(taintedNode, "medik8s.io/drain", corev1.TaintEffectNoSchedule)).To(BeTrue()) - _ = AddOrRemoveTaint(r.drainer.Client, taintedNode, false) - unTaintedNode := &corev1.Node{} - err = k8sClient.Get(context.TODO(), client.ObjectKey{Name: "node01"}, unTaintedNode) - Expect(err).NotTo(HaveOccurred()) - Expect(taintExist(unTaintedNode, "medik8s.io/drain", corev1.TaintEffectNoSchedule)).To(BeFalse()) - Expect(taintExist(unTaintedNode, "test", corev1.TaintEffectPreferNoSchedule)).To(BeTrue()) - - //Expect(len(unTaintedNode.Spec.Taints)).To(Equal(1)) + Context("Testing Taints", func() { + BeforeEach(func() { + Expect(k8sClient.Create(ctx, nodeOne)).To(Succeed()) + DeferCleanup(k8sClient.Delete, ctx, nodeOne) + }) + When("Adding and then removing a taint", func() { + It("should keep the dummy taint", func() { + node := &corev1.Node{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: taintedNodeName}, node)).To(Succeed()) + Expect(isTaintExist(node, medik8sDrainTaint.Key, medik8sDrainTaint.Effect)).To(BeFalse()) + By("Adding drain taint") + Expect(AddOrRemoveTaint(r.drainer.Client, node, true)).To(Succeed()) + taintedNode := &corev1.Node{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: taintedNodeName}, taintedNode)).To(Succeed()) + Expect(isTaintExist(taintedNode, medik8sDrainTaint.Key, medik8sDrainTaint.Effect)).To(BeTrue()) + By("Removing drain taint") + Expect(AddOrRemoveTaint(r.drainer.Client, taintedNode, false)).To(Succeed()) + unTaintedNode := &corev1.Node{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: taintedNodeName}, unTaintedNode)).To(Succeed()) + Expect(isTaintExist(unTaintedNode, medik8sDrainTaint.Key, medik8sDrainTaint.Effect)).To(BeFalse()) + By("Finding dummy taint") + Expect(isTaintExist(unTaintedNode, dummyTaintKey, corev1.TaintEffectPreferNoSchedule)).To(BeTrue()) + }) + }) }) }) Context("Reconciliation", func() { - - It("should reconcile once without failing", func() { - reconcileMaintenance(nm) - checkSuccesfulReconcile() + var nm *nodemaintenanceapi.NodeMaintenance + BeforeEach(func() { + Expect(k8sClient.Create(ctx, nodeOne)).To(Succeed()) + DeferCleanup(k8sClient.Delete, ctx, nodeOne) + + podOne, podTwo = getTestPod("test-pod-1", taintedNodeName), getTestPod("test-pod-2", taintedNodeName) + Expect(k8sClient.Create(ctx, podOne)).To(Succeed()) + Expect(k8sClient.Create(ctx, podTwo)).To(Succeed()) + DeferCleanup(cleanupPod, ctx, podOne) + DeferCleanup(cleanupPod, ctx, podTwo) }) - - It("should reconcile and cordon node", func() { - reconcileMaintenance(nm) - checkSuccesfulReconcile() - node := &corev1.Node{} - err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: nm.Spec.NodeName}, node) - Expect(err).NotTo(HaveOccurred()) - Expect(node.Spec.Unschedulable).To(Equal(true)) - }) - - It("should reconcile and taint node", func() { - reconcileMaintenance(nm) - checkSuccesfulReconcile() - node := &corev1.Node{} - err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: nm.Spec.NodeName}, node) - Expect(err).NotTo(HaveOccurred()) - Expect(taintExist(node, "medik8s.io/drain", corev1.TaintEffectNoSchedule)).To(BeTrue()) + JustBeforeEach(func() { + // Sleep for a second to ensure dummy reconciliation has begun running before the unit tests + time.Sleep(1 * time.Second) }) - It("should fail on non existing node", func() { - nmFail := getTestNM() - nmFail.Spec.NodeName = "non-existing" - err := k8sClient.Delete(context.TODO(), nm) - Expect(err).NotTo(HaveOccurred()) - err = k8sClient.Create(context.TODO(), nmFail) - Expect(err).NotTo(HaveOccurred()) - reconcileMaintenance(nm) - checkFailedReconcile() + When("nm CR is valid", func() { + BeforeEach(func() { + nm = getTestNM("node-maintenance-cr", taintedNodeName) + Expect(k8sClient.Create(ctx, nm)).To(Succeed()) + }) + It("should cordon node and add proper taints", func() { + By("check nm CR status was success") + maintenance := &nodemaintenanceapi.NodeMaintenance{} + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(nm), maintenance)).To(Succeed()) + + Expect(maintenance.Status.Phase).To(Equal(nodemaintenanceapi.MaintenanceSucceeded)) + Expect(len(maintenance.Status.PendingPods)).To(Equal(0)) + Expect(maintenance.Status.EvictionPods).To(Equal(2)) + Expect(maintenance.Status.TotalPods).To(Equal(2)) + Expect(maintenance.Status.DrainProgress).To(Equal(100)) + Expect(maintenance.Status.LastError).To(Equal("")) + Expect(maintenance.Status.LastUpdate).NotTo(BeZero()) + Expect(maintenance.Status.ErrorOnLeaseCount).To(Equal(0)) + + By("Check whether node was cordoned") + node := &corev1.Node{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: nm.Spec.NodeName}, node)).To(Succeed()) + Expect(node.Spec.Unschedulable).To(Equal(true)) + + By("Check node taints") + Expect(isTaintExist(node, medik8sDrainTaint.Key, corev1.TaintEffectNoSchedule)).To(BeTrue()) + Expect(isTaintExist(node, NodeUnschedulableTaint.Key, corev1.TaintEffectNoSchedule)).To(BeTrue()) + + By("Check add/remove Exclude remediation label") + // Label added on CR creation + Expect(node.Labels[commonLabels.ExcludeFromRemediation]).To(Equal("true")) + // Re-fetch node after nm CR deletion + Expect(k8sClient.Delete(ctx, nm)).To(Succeed()) + // Sleep for a second to ensure dummy reconciliation has begun running before the unit tests + time.Sleep(1 * time.Second) + + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: nm.Spec.NodeName}, node)).NotTo(HaveOccurred()) + _, exist := node.Labels[commonLabels.ExcludeFromRemediation] + Expect(exist).To(BeFalse()) + }) }) - - It("add/remove Exclude remediation label", func() { - reconcileMaintenance(nm) - checkSuccesfulReconcile() - node := &corev1.Node{} - err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: nm.Spec.NodeName}, node) - Expect(err).NotTo(HaveOccurred()) - //Label added on CR creation - Expect(node.Labels[labels.ExcludeFromRemediation]).To(Equal("true")) - - Expect(k8sClient.Delete(context.Background(), nm)).To(Succeed()) - reconcileMaintenance(nm) - //Re-fetch node after reconcile - Expect(k8sClient.Get(context.TODO(), client.ObjectKey{Name: nm.Spec.NodeName}, node)).NotTo(HaveOccurred()) - _, exist := node.Labels[labels.ExcludeFromRemediation] - Expect(exist).To(BeFalse()) + When("nm CR is invalid for a missing node", func() { + BeforeEach(func() { + nm = getTestNM("non-existing-node-cr", invalidNodeName) + Expect(k8sClient.Create(ctx, nm)).To(Succeed()) + DeferCleanup(k8sClient.Delete, ctx, nm) + }) + It("should fail on non existing node", func() { + By("check nm CR status and whether LastError was updated") + maintenance := &nodemaintenanceapi.NodeMaintenance{} + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(nm), maintenance)).To(Succeed()) + + Expect(maintenance.Status.Phase).To(Equal(nodemaintenanceapi.MaintenanceRunning)) + Expect(len(maintenance.Status.PendingPods)).To(Equal(0)) + Expect(maintenance.Status.EvictionPods).To(Equal(0)) + Expect(maintenance.Status.TotalPods).To(Equal(0)) + Expect(maintenance.Status.DrainProgress).To(Equal(0)) + Expect(maintenance.Status.LastError).To(Equal(fmt.Sprintf("nodes \"%s\" not found", invalidNodeName))) + Expect(maintenance.Status.LastUpdate).NotTo(BeZero()) + Expect(maintenance.Status.ErrorOnLeaseCount).To(Equal(0)) + }) }) - }) }) -func getTestObjects() (*nodemaintenanceapi.NodeMaintenance, []client.Object) { - nm := getTestNM() +// isLabelExist checks whether a node label has the value true +func isLabelExist(node *corev1.Node, label string) bool { + return node.Labels[label] == "true" +} - return nm, []client.Object{ - &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node01", - }, - Spec: corev1.NodeSpec{ - Taints: []corev1.Taint{{ - Key: "test", - Effect: corev1.TaintEffectPreferNoSchedule}, - }, - }, +// isTaintExist checks whether a node has a specific taint +func isTaintExist(node *corev1.Node, key string, effect corev1.TaintEffect) bool { + checkTaint := corev1.Taint{ + Key: key, + Effect: effect, + } + taints := node.Spec.Taints + for _, taint := range taints { + if reflect.DeepEqual(taint, checkTaint) { + return true + } + } + return false +} + +func getNode(nodeName string) *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + Labels: map[string]string{dummyLabelKey: dummyLabelValue}, }, - &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node02", + Spec: corev1.NodeSpec{ + Taints: []corev1.Taint{{ + Key: dummyTaintKey, + Effect: corev1.TaintEffectPreferNoSchedule}, }, }, - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "test-pod-1", - }, - Spec: corev1.PodSpec{ - NodeName: "node01", - Containers: []corev1.Container{ - { - Name: "c1", - Image: "i1", - }, - }, - TerminationGracePeriodSeconds: pointer.Int64(0), - }, - Status: corev1.PodStatus{ - Conditions: []corev1.PodCondition{ - { - Type: corev1.PodReady, - Status: corev1.ConditionTrue, - }, - }, - }, + } +} + +func getTestPod(podName, nodeName string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: podName, }, - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "test-pod-2", - }, - Spec: corev1.PodSpec{ - NodeName: "node01", - Containers: []corev1.Container{ - { - Name: "c1", - Image: "i1", - }, + Spec: corev1.PodSpec{ + NodeName: nodeName, + Containers: []corev1.Container{ + { + Name: "c1", + Image: "i1", }, - TerminationGracePeriodSeconds: pointer.Int64(0), }, - Status: corev1.PodStatus{ - Conditions: []corev1.PodCondition{ - { - Type: corev1.PodReady, - Status: corev1.ConditionTrue, - }, + TerminationGracePeriodSeconds: ptr.To[int64](0), + }, + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: corev1.ConditionTrue, }, }, }, } } -func getTestNM() *nodemaintenanceapi.NodeMaintenance { +// cleanupPod deletes the pod only if it exists +func cleanupPod(ctx context.Context, pod *corev1.Pod) { + + podErr := k8sClient.Get(ctx, client.ObjectKeyFromObject(pod), &corev1.Pod{}) + if podErr != nil { + // There is no to pod to delete/clean + testLog.Info("Cleanup: pods is missing", "pod", pod.Name) + return + } + var force client.GracePeriodSeconds = 0 + if err := k8sClient.Delete(ctx, pod, force); err != nil { + if !apierrors.IsNotFound(err) { + ConsistentlyWithOffset(1, func() error { + podErr := k8sClient.Get(ctx, client.ObjectKeyFromObject(pod), &corev1.Pod{}) + if apierrors.IsNotFound(podErr) { + testLog.Info("Cleanup: Got error 404", "name", pod.Name) + return nil + } + return podErr + }, "4s", "100ms").Should(BeNil(), "pod should be deleted") + } + } +} + +func getTestNM(crName, nodeName string) *nodemaintenanceapi.NodeMaintenance { return &nodemaintenanceapi.NodeMaintenance{ ObjectMeta: metav1.ObjectMeta{ - Name: "node-maintenance", + Name: crName, }, Spec: nodemaintenanceapi.NodeMaintenanceSpec{ - NodeName: "node01", + NodeName: nodeName, Reason: "test reason", }, }