Skip to content

Commit

Permalink
Simplify taint handeling and function signatures
Browse files Browse the repository at this point in the history
Shorter and more readable code
  • Loading branch information
razo7 committed Jun 12, 2023
1 parent 0303d6b commit 2286224
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 40 deletions.
2 changes: 0 additions & 2 deletions api/v1alpha1/fenceagentsremediation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ const (
FARFinalizer string = "fence-agents-remediation.medik8s.io/far-finalizer"

// Taint actions and names
AppendTaintAction = "append"
RemoveTaintAction = "remove"
FARNoExecuteTaint = "FAR-NoExecute"
Medik8sNoExecuteTaintKey = "medik8s.io/remediation"
FARNoExecuteTaintKeyValue = "fence-agents-remediation"
Expand Down
4 changes: 2 additions & 2 deletions controllers/fenceagentsremediation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
// The object is being deleted

// remove node's taints
if err := utils.HandleTaint(r.Client, farV1alpha1.FARNoExecuteTaint, farV1alpha1.RemoveTaintAction, far); err != nil {
if err := utils.RemoveTaint(r.Client, farV1alpha1.FARNoExecuteTaint, far.Name); err != nil {
return emptyResult, err
}
// remove finalizer
Expand Down Expand Up @@ -134,7 +134,7 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
}
// Add medik8s remediation taint
r.Log.Info("Add Medik8s remediation taint", "Node Name", req.Name)
if err := utils.HandleTaint(r.Client, farV1alpha1.FARNoExecuteTaint, farV1alpha1.AppendTaintAction, far); err != nil {
if err := utils.AppendTaint(r.Client, farV1alpha1.FARNoExecuteTaint, far.Name); err != nil {
return emptyResult, err
}

Expand Down
11 changes: 3 additions & 8 deletions pkg/utils/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
apiErrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

farV1alpha1 "github.com/medik8s/fence-agents-remediation/api/v1alpha1"
)

// IsNodeNameValid returns an error if nodeName doesn't match any node name int the cluster, otherwise a nil
Expand All @@ -27,13 +25,10 @@ func IsNodeNameValid(r client.Reader, nodeName string) (bool, error) {
return true, nil
}

// getNodeFromFAR returns the unhealthy node reported in the given far CR (by it's name)
func getNodeFromFAR(r client.Client, far *farV1alpha1.FenceAgentsRemediation) (*corev1.Node, error) {
// getNodeWithName returns a node with a name nodeName, or an error if it can't be found
func getNodeWithName(r client.Client, nodeName string) (*corev1.Node, error) {
node := &corev1.Node{}
key := client.ObjectKey{
Name: far.Name,
Namespace: "",
}
key := client.ObjectKey{Name: nodeName}
if err := r.Get(context.TODO(), key, node); err != nil {
return nil, err
}
Expand Down
57 changes: 29 additions & 28 deletions pkg/utils/taints.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,36 +46,32 @@ func deleteTaint(taints []corev1.Taint, taintToDelete *corev1.Taint) ([]corev1.T
return newTaints, deleted
}

// HandleTaint calls other taint-functions basesd on the taint name and acion.
// If there is no associated node for FAR or taint name is invalid or taint action is invalid then it returns an error
func HandleTaint(r client.Client, taintName, actionOnTaint string, far *farV1alpha1.FenceAgentsRemediation) error {
var taintObj *corev1.Taint
// check if taint exist
switch taintName {
case farV1alpha1.FARNoExecuteTaint:
func getTaintAndNode(r client.Client, taintName, nodeName string) (*corev1.Taint, *corev1.Node, error) {
var (
taintObj *corev1.Taint
node *corev1.Node
)
// select taint
if taintName == farV1alpha1.FARNoExecuteTaint {
taintObj = FARNodeNoExecuteTaint
default:
return fmt.Errorf("invalid taintName - %s", taintName)
} else {
return nil, nil, fmt.Errorf("invalid taintName - %s", taintName)
}
// find node
node, err := getNodeFromFAR(r, far)
// find node by name
node, err := getNodeWithName(r, nodeName)
if err != nil {
return err
}

switch actionOnTaint {
case farV1alpha1.AppendTaintAction:
return appendTaint(r, taintObj, node)
case farV1alpha1.RemoveTaintAction:
return removeTaint(r, taintObj, node)
default:
return fmt.Errorf("invalid action for taint - %s", actionOnTaint)
return nil, nil, err
}
return taintObj, node, nil
}

// appendTaint appends new taint to the taint list when it is not present, and returns error if it fails in the process
func appendTaint(r client.Client, taintObj *corev1.Taint, node *corev1.Node) error {

func AppendTaint(r client.Client, taintName, nodeName string) error {
taintObj, node, err := getTaintAndNode(r, taintName, nodeName)
if err != nil {
return err
}
// check if taint doesn't exist
if taintExists(node.Spec.Taints, taintObj) {
return nil
}
Expand All @@ -89,15 +85,20 @@ func appendTaint(r client.Client, taintObj *corev1.Taint, node *corev1.Node) err

// update with new taint list
if err := r.Patch(context.Background(), node, patch); err != nil {
loggerTaint.Error(err, "Failed to add taint on node", "node name", node.Name, "taint key", taintObj.Key, "taint effect", taintObj.Effect)
loggerTaint.Error(err, "Failed to append taint on node", "node name", node.Name, "taint key", taintObj.Key, "taint effect", taintObj.Effect)
return err
}
loggerTaint.Info("Taint was added", "old taint effect", taintObj.Effect, "new taints", node.Spec.Taints)
loggerTaint.Info("Taint was added", "taint effect", taintObj.Effect, "new taints", node.Spec.Taints)
return nil
}

// removeTaint removes taint from the taint list when it is existed, and returns error if it fails in the process
func removeTaint(r client.Client, taintObj *corev1.Taint, node *corev1.Node) error {
// RemoveTaint removes taint from the taint list when it is existed, and returns error if it fails in the process
func RemoveTaint(r client.Client, taintName, nodeName string) error {
taintObj, node, err := getTaintAndNode(r, taintName, nodeName)
if err != nil {
return err
}
// check if taint exist
if !taintExists(node.Spec.Taints, taintObj) {
return nil
}
Expand All @@ -116,6 +117,6 @@ func removeTaint(r client.Client, taintObj *corev1.Taint, node *corev1.Node) err
loggerTaint.Error(err, "Failed to remove taint from node,", "node name", node.Name, "taint key", taintObj.Key, "taint effect", taintObj.Effect)
return err
}
loggerTaint.Info("Taint was removed", "old taint effect", taintObj.Effect, "new taints", node.Spec.Taints)
loggerTaint.Info("Taint was removed", "taint effect", taintObj.Effect, "new taints", node.Spec.Taints)
return nil
}

0 comments on commit 2286224

Please sign in to comment.