From e635e9b3901b9f0c507ff6674e9c17afa81a965e Mon Sep 17 00:00:00 2001 From: Ryota Arai Date: Tue, 15 Nov 2022 23:19:18 +0900 Subject: [PATCH 1/3] Delete NodeOperation if the Node becomes remediated. --- controllers/noderemediation_controller.go | 40 ++++++++++++++++------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/controllers/noderemediation_controller.go b/controllers/noderemediation_controller.go index 15af282..3b90162 100644 --- a/controllers/noderemediation_controller.go +++ b/controllers/noderemediation_controller.go @@ -22,7 +22,7 @@ import ( nodeopsv1alpha1 "github.com/pfnet-research/node-operation-controller/api/v1alpha1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -65,11 +65,7 @@ func (r *NodeRemediationReconciler) Reconcile(ctx context.Context, req ctrl.Requ var remediation nodeopsv1alpha1.NodeRemediation if err := r.Get(ctx, req.NamespacedName, &remediation); err != nil { - sterr, ok := err.(*errors.StatusError) - if ok && sterr.Status().Code == 404 { - return ctrl.Result{}, nil - } - return ctrl.Result{}, err + return ctrl.Result{}, client.IgnoreNotFound(err) } var node corev1.Node @@ -115,11 +111,6 @@ func (r *NodeRemediationReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, err } - if remediation.Status.ActiveNodeOperation.Name != "" { - // active operation exists - return ctrl.Result{}, nil - } - // Check node condition switch remediation.Status.NodeStatus { case nodeopsv1alpha1.NodeStatusUnknown: @@ -131,6 +122,33 @@ func (r *NodeRemediationReconciler) Reconcile(ctx context.Context, req ctrl.Requ if err := r.Status().Update(ctx, &remediation); err != nil { return ctrl.Result{}, err } + + if ref := remediation.Status.ActiveNodeOperation; ref.Name != "" { + // active operation exists + var nodeOp nodeopsv1alpha1.NodeOperation + if err := r.Get(ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, &nodeOp); apierrors.IsNotFound(err) { + // Do nothing + } else if err != nil { + return ctrl.Result{}, err + } else { + if err := r.Delete(ctx, &nodeOp); err != nil { + return ctrl.Result{}, err + } + + r.eventRecorder.Eventf(&remediation, corev1.EventTypeNormal, "DeleteNodeOperation", `Deleted NodeOperation %s because the Node is remediated`, nodeOp.Name) + } + + remediation.Status.ActiveNodeOperation = corev1.ObjectReference{} + if err := r.Status().Update(ctx, &remediation); err != nil { + return ctrl.Result{}, err + } + } + + return ctrl.Result{}, nil + } + + if remediation.Status.ActiveNodeOperation.Name != "" { + // active operation exists return ctrl.Result{}, nil } From 77fbf31d0eb9e7b7157a5aa1bcf936ff290c327c Mon Sep 17 00:00:00 2001 From: Ryota Arai Date: Thu, 12 Jan 2023 00:38:22 +0900 Subject: [PATCH 2/3] test: Remediation deletes a NodeOperation when the Node becomes remediated --- controllers/suite_test.go | 99 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 104f2ca..ea38582 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -35,6 +35,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -725,4 +726,102 @@ var _ = Describe("NodeRemediation", func() { return false }, eventuallyTimeout).Should(BeTrue()) }) + + It("deletes NodeOperation when the Node becomes remediated", func() { + ctx := context.Background() + nodeName := nodeNames[1] + + template := nodeopsv1alpha1.NodeOperationTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-remediation-2", + }, + Spec: nodeopsv1alpha1.NodeOperationTemplateSpec{ + Template: nodeopsv1alpha1.NodeOperationTemplateTemplateSpec{ + Spec: nodeopsv1alpha1.NodeOperationSpecTemplate{ + JobTemplate: nodeopsv1alpha1.JobTemplateSpec{ + Metadata: metav1.ObjectMeta{ + Namespace: "default", + }, + Spec: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "c", + Image: "busybox", + Command: []string{"sleep", "infinity"}, + }}, + RestartPolicy: corev1.RestartPolicyNever, + Tolerations: []corev1.Toleration{ + {Key: controllerTaint.Key, Operator: corev1.TolerationOpExists}, + }, + }, + }, + }, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, &template)).NotTo(HaveOccurred()) + + remediation := nodeopsv1alpha1.NodeRemediation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-remediation-2", + }, + Spec: nodeopsv1alpha1.NodeRemediationSpec{ + NodeRemediationSpecTemplate: nodeopsv1alpha1.NodeRemediationSpecTemplate{ + Rule: nodeopsv1alpha1.NodeRemediationRule{ + Conditions: []nodeopsv1alpha1.NodeConditionMatcher{ + {Type: "TestRemediation2", Status: corev1.ConditionTrue}, + }, + }, + NodeOperationTemplateName: template.Name, + }, + NodeName: nodeName, + }, + } + Expect(k8sClient.Create(ctx, &remediation)).NotTo(HaveOccurred()) + + node := corev1.Node{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: nodeName}, &node)).NotTo(HaveOccurred()) + + node.Status.Conditions = append(node.Status.Conditions, corev1.NodeCondition{ + Type: "TestRemediation2", + Status: corev1.ConditionTrue, + Reason: "testing", + Message: "testing", + LastHeartbeatTime: metav1.NewTime(time.Now()), + LastTransitionTime: metav1.NewTime(time.Now()), + }) + Expect(k8sClient.Status().Update(ctx, &node)).NotTo(HaveOccurred()) + + var nodeOp *nodeopsv1alpha1.NodeOperation + Eventually(func() bool { + nodeOpList := nodeopsv1alpha1.NodeOperationList{} + Expect(k8sClient.List(ctx, &nodeOpList)).NotTo(HaveOccurred()) + + for _, op := range nodeOpList.Items { + for _, owner := range op.OwnerReferences { + if owner.Kind == "NodeRemediation" && owner.Name == remediation.Name { + nodeOp = &op + return true + } + } + } + + return false + }, eventuallyTimeout).Should(BeTrue()) + + node.Status.Conditions[len(node.Status.Conditions)-1].Status = corev1.ConditionFalse + Expect(k8sClient.Status().Update(ctx, &node)).NotTo(HaveOccurred()) + + Eventually(func() bool { + err := k8sClient.Get(ctx, client.ObjectKey{ + Namespace: nodeOp.Namespace, + Name: nodeOp.Name, + }, &nodeopsv1alpha1.NodeOperation{}) + + return apierrors.IsNotFound(err) + }, eventuallyTimeout).Should(BeTrue()) + }) }) From e44f86515d3217c801f178a1428694c6f8a146fd Mon Sep 17 00:00:00 2001 From: Ryota Arai Date: Thu, 12 Jan 2023 08:58:04 +0900 Subject: [PATCH 3/3] Get Node before updating. --- controllers/suite_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index ea38582..b71bf7d 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -812,6 +812,7 @@ var _ = Describe("NodeRemediation", func() { return false }, eventuallyTimeout).Should(BeTrue()) + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: nodeName}, &node)).NotTo(HaveOccurred()) node.Status.Conditions[len(node.Status.Conditions)-1].Status = corev1.ConditionFalse Expect(k8sClient.Status().Update(ctx, &node)).NotTo(HaveOccurred())