From 3cb9806d2b2a3c3b512caa94f6758e0c1c571d07 Mon Sep 17 00:00:00 2001 From: Michael Shitrit Date: Wed, 29 Nov 2023 12:39:45 +0200 Subject: [PATCH 1/2] add/remove exclude remediation label Signed-off-by: Michael Shitrit --- controllers/nodemaintenance_controller.go | 40 +++++++++++++++++-- .../nodemaintenance_controller_test.go | 18 +++++++++ go.mod | 2 +- go.sum | 4 +- .../medik8s/common/pkg/labels/labels.go | 4 ++ .../medik8s/common/pkg/lease/manager.go | 11 +++-- vendor/modules.txt | 2 +- 7 files changed, 70 insertions(+), 11 deletions(-) diff --git a/controllers/nodemaintenance_controller.go b/controllers/nodemaintenance_controller.go index 0b1898434..1a43440de 100644 --- a/controllers/nodemaintenance_controller.go +++ b/controllers/nodemaintenance_controller.go @@ -22,6 +22,7 @@ import ( "time" "github.com/go-logr/logr" + "github.com/medik8s/common/pkg/labels" "github.com/medik8s/common/pkg/lease" corev1 "k8s.io/api/core/v1" @@ -175,8 +176,13 @@ func (r *NodeMaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ if instance.Status.Phase != nodemaintenancev1beta1.MaintenanceRunning || instance.Status.ErrorOnLeaseCount != 0 { instance.Status.Phase = nodemaintenancev1beta1.MaintenanceRunning instance.Status.ErrorOnLeaseCount = 0 + } } + //Add exclude from remediation label + if err := r.addExcludeRemediationLabel(ctx, node); err != nil { + return r.onReconcileError(instance, err) + } // Cordon node err = AddOrRemoveTaint(r.drainer.Client, node, true) @@ -313,6 +319,35 @@ func (r *NodeMaintenanceReconciler) obtainLease(node *corev1.Node) (bool, error) return false, nil } + +func (r *NodeMaintenanceReconciler) addExcludeRemediationLabel(ctx context.Context, node *corev1.Node) error { + if node.Labels[labels.ExcludeFromRemediation] != "true" { + patch := client.MergeFrom(node.DeepCopy()) + if node.Labels == nil { + node.Labels = map[string]string{labels.ExcludeFromRemediation: "true"} + } 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) + return err + } + } + return nil +} + +func (r *NodeMaintenanceReconciler) removeExcludeRemediationLabel(ctx context.Context, node *corev1.Node) 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) + return err + } + } + return nil +} + func (r *NodeMaintenanceReconciler) stopNodeMaintenanceImp(ctx context.Context, node *corev1.Node) error { // Uncordon the node err := AddOrRemoveTaint(r.drainer.Client, node, false) @@ -327,8 +362,7 @@ func (r *NodeMaintenanceReconciler) stopNodeMaintenanceImp(ctx context.Context, if err := r.LeaseManager.InvalidateLease(ctx, node); err != nil { return err } - - return nil + return r.removeExcludeRemediationLabel(ctx, node) } func (r *NodeMaintenanceReconciler) stopNodeMaintenanceOnDeletion(ctx context.Context, nodeName string) error { @@ -339,7 +373,7 @@ func (r *NodeMaintenanceReconciler) stopNodeMaintenanceOnDeletion(ctx context.Co if err := r.LeaseManager.InvalidateLease(ctx, &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: nodeName}}); err != nil { return err } - return nil + return r.removeExcludeRemediationLabel(ctx, node) } return err } diff --git a/controllers/nodemaintenance_controller_test.go b/controllers/nodemaintenance_controller_test.go index 5a23e3e8b..06b8ddea5 100644 --- a/controllers/nodemaintenance_controller_test.go +++ b/controllers/nodemaintenance_controller_test.go @@ -5,6 +5,7 @@ import ( "reflect" "time" + "github.com/medik8s/common/pkg/labels" "github.com/medik8s/common/pkg/lease" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -235,6 +236,23 @@ var _ = Describe("Node Maintenance", func() { checkFailedReconcile() }) + 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()) + }) + }) }) diff --git a/go.mod b/go.mod index d67798695..d9e0243c8 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.20 require ( github.com/go-logr/logr v1.2.4 - github.com/medik8s/common v1.2.0 + github.com/medik8s/common v1.11.0 github.com/onsi/ginkgo/v2 v2.11.0 github.com/onsi/gomega v1.27.8 github.com/sirupsen/logrus v1.9.3 diff --git a/go.sum b/go.sum index 6e5607bc1..4f88be4ec 100644 --- a/go.sum +++ b/go.sum @@ -231,8 +231,8 @@ github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJ github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/matttproud/golang_protobuf_extensions v1.0.2 h1:hAHbPm5IJGijwng3PWk09JkG9WeqChjprR5s9bBZ+OM= github.com/matttproud/golang_protobuf_extensions v1.0.2/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= -github.com/medik8s/common v1.2.0 h1:xgQOijD3rEn+PfCd4lJuV3WFdt5QA6SIaqF01rRp2as= -github.com/medik8s/common v1.2.0/go.mod h1:ZT/3hfMXJLmZEcqmxRWB5LGC8Wl+qKGGQ4zM8hOE7PY= +github.com/medik8s/common v1.11.0 h1:uw00Ej2aeRXG81zjfw9V69KRTyF47vBlSiV6eu8JipM= +github.com/medik8s/common v1.11.0/go.mod h1:ZT/3hfMXJLmZEcqmxRWB5LGC8Wl+qKGGQ4zM8hOE7PY= github.com/mitchellh/go-wordwrap v1.0.0 h1:6GlHJ/LTGMrIJbwgdqdl2eEH8o+Exx/0m8ir9Gns0u4= github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo= github.com/moby/spdystream v0.2.0 h1:cjW1zVyyoiM0T7b6UoySUFqzXMoqRckQtXwGPiBhOM8= diff --git a/vendor/github.com/medik8s/common/pkg/labels/labels.go b/vendor/github.com/medik8s/common/pkg/labels/labels.go index de5038d49..87409558e 100644 --- a/vendor/github.com/medik8s/common/pkg/labels/labels.go +++ b/vendor/github.com/medik8s/common/pkg/labels/labels.go @@ -7,4 +7,8 @@ const ( MasterRole = "node-role.kubernetes.io/master" // ControlPlaneRole is the new role label of control plane nodes ControlPlaneRole = "node-role.kubernetes.io/control-plane" + // DefaultTemplate label indicates to third party tools (e.g. UI) the default remediation template in case several exists. + DefaultTemplate = "remediation.medik8s.io/default-template" + // ExcludeFromRemediation label would be put on a node with value "true" in order to indicate this node should not be remediated. + ExcludeFromRemediation = "remediation.medik8s.io/exclude-from-remediation" ) diff --git a/vendor/github.com/medik8s/common/pkg/lease/manager.go b/vendor/github.com/medik8s/common/pkg/lease/manager.go index 7e3df566a..d68b5c666 100644 --- a/vendor/github.com/medik8s/common/pkg/lease/manager.go +++ b/vendor/github.com/medik8s/common/pkg/lease/manager.go @@ -45,13 +45,13 @@ type manager struct { log logr.Logger } -// AlreadyHeldError is returned in case the lease that is requested is already held by a different holder +// AlreadyHeldError is returned in case the lease is already held by a different holder type AlreadyHeldError struct { holderIdentity string } -func (e *AlreadyHeldError) Error() string { - return fmt.Sprintf("can't update valid lease held by different owner: %s", e.holderIdentity) +func (e AlreadyHeldError) Error() string { + return fmt.Sprintf("can't update or invalidate the lease because it is held by different owner: %s", e.holderIdentity) } func (l *manager) RequestLease(ctx context.Context, obj client.Object, leaseDuration time.Duration) error { @@ -186,7 +186,7 @@ func (l *manager) requestLease(ctx context.Context, obj client.Object, leaseDura if lease.Spec.HolderIdentity != nil { identity = *lease.Spec.HolderIdentity } - return &AlreadyHeldError{holderIdentity: identity} + return AlreadyHeldError{holderIdentity: identity} } needUpdateLease = true @@ -227,6 +227,9 @@ func (l *manager) invalidateLease(ctx context.Context, obj client.Object) error } return err } + if lease.Spec.HolderIdentity != nil && l.holderIdentity != *lease.Spec.HolderIdentity { + return AlreadyHeldError{*lease.Spec.HolderIdentity} + } if err := l.Client.Delete(ctx, lease); err != nil { log.Error(err, "failed to delete lease to be invalidated") return err diff --git a/vendor/modules.txt b/vendor/modules.txt index ef34b70cd..47b39886a 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -128,7 +128,7 @@ github.com/mailru/easyjson/jwriter # github.com/matttproud/golang_protobuf_extensions v1.0.2 ## explicit; go 1.9 github.com/matttproud/golang_protobuf_extensions/pbutil -# github.com/medik8s/common v1.2.0 +# github.com/medik8s/common v1.11.0 ## explicit; go 1.20 github.com/medik8s/common/pkg/labels github.com/medik8s/common/pkg/lease From dc2d4367e4a7ffec27a914b514398b87a1dee68e Mon Sep 17 00:00:00 2001 From: Michael Shitrit Date: Tue, 5 Dec 2023 09:13:10 +0200 Subject: [PATCH 2/2] fix unnecessary label remove Signed-off-by: Michael Shitrit Signed-off-by: Michael Shitrit --- controllers/nodemaintenance_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/nodemaintenance_controller.go b/controllers/nodemaintenance_controller.go index 1a43440de..8bf003fe0 100644 --- a/controllers/nodemaintenance_controller.go +++ b/controllers/nodemaintenance_controller.go @@ -373,7 +373,7 @@ func (r *NodeMaintenanceReconciler) stopNodeMaintenanceOnDeletion(ctx context.Co if err := r.LeaseManager.InvalidateLease(ctx, &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: nodeName}}); err != nil { return err } - return r.removeExcludeRemediationLabel(ctx, node) + return nil } return err }