Skip to content

Commit

Permalink
Use Update rather than patch for updating node taints
Browse files Browse the repository at this point in the history
The API server is updating taints as well while nodes get healthy/unhealthy, so patch can overwrite changes made by API server
  • Loading branch information
razo7 committed Jun 15, 2023
1 parent 25d21be commit 2c50034
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ spec:
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- ""
Expand Down
2 changes: 1 addition & 1 deletion config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ rules:
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- ""
Expand Down
13 changes: 6 additions & 7 deletions controllers/fenceagentsremediation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (r *FenceAgentsRemediationReconciler) SetupWithManager(mgr ctrl.Manager) er

//+kubebuilder:rbac:groups=core,resources=pods/exec,verbs=create
//+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch;update;delete;deletecollection
//+kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch;patch;delete
//+kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch;update;delete
//+kubebuilder:rbac:groups=fence-agents-remediation.medik8s.io,resources=fenceagentsremediations,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=fence-agents-remediation.medik8s.io,resources=fenceagentsremediations/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=fence-agents-remediation.medik8s.io,resources=fenceagentsremediations/finalizers,verbs=update
Expand All @@ -78,23 +78,22 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
far := &v1alpha1.FenceAgentsRemediation{}
if err := r.Get(ctx, req.NamespacedName, far); err != nil {
if apiErrors.IsNotFound(err) {
// FAR is deleted, stop reconciling
// FAR's CR is deleted, stop reconciling
r.Log.Info("FAR CR is deleted - nothing to do", "CR Name", req.Name, "CR Namespace", req.Namespace)
return emptyResult, nil
}
r.Log.Error(err, "failed to get FAR CR")
return emptyResult, err
}
// Add finalizer when object is created
if far.ObjectMeta.DeletionTimestamp.IsZero() {
// Add finalizer when the CR is created
if !controllerutil.ContainsFinalizer(far, v1alpha1.FARFinalizer) && far.ObjectMeta.DeletionTimestamp.IsZero() {
controllerutil.AddFinalizer(far, v1alpha1.FARFinalizer)
if err := r.Client.Update(context.Background(), far); err != nil {
return emptyResult, fmt.Errorf("failed to add finalizer to the CR - %w", err)
}
} else if controllerutil.ContainsFinalizer(far, v1alpha1.FARFinalizer) {
} else if controllerutil.ContainsFinalizer(far, v1alpha1.FARFinalizer) && !far.ObjectMeta.DeletionTimestamp.IsZero() {
// Delete CR only when a finalizer and DeletionTimestamp are set
r.Log.Info("CR's deletion timestamp is not zero, and FAR finalizer exists", "CR Name", req.Name)
// The object is being deleted

// remove node's taints
if err := utils.RemoveTaint(r.Client, far.Name); err != nil && !apiErrors.IsNotFound(err) {
return emptyResult, err
Expand Down
6 changes: 2 additions & 4 deletions pkg/utils/taints.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,12 @@ func AppendTaint(r client.Client, nodeName string) error {
return nil
}
// add the taint to the taint list
patch := client.MergeFrom(node.DeepCopy())
now := metav1.Now()
taint.TimeAdded = &now
node.Spec.Taints = append(node.Spec.Taints, taint)

// update with new taint list
if err := r.Patch(context.Background(), node, patch); err != nil {
if err := r.Update(context.Background(), node); err != nil {
loggerTaint.Error(err, "Failed to append taint on node", "node name", node.Name, "taint key", taint.Key, "taint effect", taint.Effect)
return err
}
Expand All @@ -94,7 +93,6 @@ func RemoveTaint(r client.Client, nodeName string) error {
}

// delete the taint from the taint list
patch := client.MergeFrom(node.DeepCopy())
if taints, deleted := deleteTaint(node.Spec.Taints, &taint); !deleted {
loggerTaint.Info("Failed to remove taint from node - taint was not found", "node name", node.Name, "taint key", taint.Key, "taint effect", taint.Effect)
return nil
Expand All @@ -103,7 +101,7 @@ func RemoveTaint(r client.Client, nodeName string) error {
}

// update with new taint list
if err := r.Patch(context.Background(), node, patch); err != nil {
if err := r.Update(context.Background(), node); err != nil {
loggerTaint.Error(err, "Failed to remove taint from node,", "node name", node.Name, "taint key", taint.Key, "taint effect", taint.Effect)
return err
}
Expand Down

0 comments on commit 2c50034

Please sign in to comment.