Merge pull request #505 from prashanth26/regression/issue-448

Changes missed out by PR #492

Author: prashanth26

pkg/controller/controller_utils.go

@@ -24,7 +24,6 @@ package controller
 
 import (
 	"encoding/binary"
-	"encoding/json"
 	"fmt"
 	"hash/fnv"
 	"math/rand"
@@ -33,32 +32,27 @@ import (
 	"sync/atomic"
 	"time"
 
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/api/validation"
-
 	"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
 	machineapi "github.com/gardener/machine-controller-manager/pkg/client/clientset/versioned/typed/machine/v1alpha1"
+	fakemachineapi "github.com/gardener/machine-controller-manager/pkg/client/clientset/versioned/typed/machine/v1alpha1/fake"
 	annotationsutils "github.com/gardener/machine-controller-manager/pkg/util/annotations"
-	conditionutils "github.com/gardener/machine-controller-manager/pkg/util/conditions"
 	hashutil "github.com/gardener/machine-controller-manager/pkg/util/hash"
-	taintutils "github.com/gardener/machine-controller-manager/pkg/util/taints"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/api/validation"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/clock"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	clientretry "k8s.io/client-go/util/retry"
-
-	fakemachineapi "github.com/gardener/machine-controller-manager/pkg/client/clientset/versioned/typed/machine/v1alpha1/fake"
 	"k8s.io/klog"
 )
 
@@ -904,193 +898,6 @@ func FilterMachineSets(ISes []*v1alpha1.MachineSet, filterFn filterIS) []*v1alph
 	return filtered
 }
 
-// AddOrUpdateTaintOnNode add taints to the node. If taint was added into node, it'll issue API calls
-// to update nodes; otherwise, no API calls. Return error if any.
-func AddOrUpdateTaintOnNode(c clientset.Interface, nodeName string, taints ...*v1.Taint) error {
-	if len(taints) == 0 {
-		return nil
-	}
-	firstTry := true
-	return clientretry.RetryOnConflict(Backoff, func() error {
-		var err error
-		var oldNode *v1.Node
-		// First we try getting node from the API server cache, as it's cheaper. If it fails
-		// we get it from etcd to be sure to have fresh data.
-		if firstTry {
-			oldNode, err = c.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{ResourceVersion: "0"})
-			firstTry = false
-		} else {
-			oldNode, err = c.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{})
-		}
-		if err != nil {
-			return err
-		}
-
-		var newNode *v1.Node
-		oldNodeCopy := oldNode
-		updated := false
-		for _, taint := range taints {
-			curNewNode, ok, err := taintutils.AddOrUpdateTaint(oldNodeCopy, taint)
-			if err != nil {
-				return fmt.Errorf("Failed to update taint of node")
-			}
-			updated = updated || ok
-			newNode = curNewNode
-			oldNodeCopy = curNewNode
-		}
-		if !updated {
-			return nil
-		}
-		return UpdateNodeTaints(c, nodeName, oldNode, newNode)
-	})
-}
-
-// RemoveTaintOffNode is for cleaning up taints temporarily added to node,
-// won't fail if target taint doesn't exist or has been removed.
-// If passed a node it'll check if there's anything to be done, if taint is not present it won't issue
-// any API calls.
-func RemoveTaintOffNode(c clientset.Interface, nodeName string, node *v1.Node, taints ...*v1.Taint) error {
-	if len(taints) == 0 {
-		return nil
-	}
-	// Short circuit for limiting amount of API calls.
-	if node != nil {
-		match := false
-		for _, taint := range taints {
-			if taintutils.TaintExists(node.Spec.Taints, taint) {
-				match = true
-				break
-			}
-		}
-		if !match {
-			return nil
-		}
-	}
-
-	firstTry := true
-	return clientretry.RetryOnConflict(Backoff, func() error {
-		var err error
-		var oldNode *v1.Node
-		// First we try getting node from the API server cache, as it's cheaper. If it fails
-		// we get it from etcd to be sure to have fresh data.
-		if firstTry {
-			oldNode, err = c.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{ResourceVersion: "0"})
-			firstTry = false
-		} else {
-			oldNode, err = c.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{})
-		}
-		if err != nil {
-			return err
-		}
-
-		var newNode *v1.Node
-		oldNodeCopy := oldNode
-		updated := false
-		for _, taint := range taints {
-			curNewNode, ok, err := taintutils.RemoveTaint(oldNodeCopy, taint)
-			if err != nil {
-				return fmt.Errorf("Failed to remove taint of node")
-			}
-			updated = updated || ok
-			newNode = curNewNode
-			oldNodeCopy = curNewNode
-		}
-		if !updated {
-			return nil
-		}
-		return UpdateNodeTaints(c, nodeName, oldNode, newNode)
-	})
-}
-
-// PatchNodeTaints is for updating the node taints from oldNode to the newNode
-// It makes a TwoWayMergePatch by comparing the two objects
-// It calls the Patch() method to do the final patch
-func PatchNodeTaints(c clientset.Interface, nodeName string, oldNode *v1.Node, newNode *v1.Node) error {
-	oldData, err := json.Marshal(oldNode)
-	if err != nil {
-		return fmt.Errorf("failed to marshal old node %#v for node %q: %v", oldNode, nodeName, err)
-	}
-
-	newTaints := newNode.Spec.Taints
-	newNodeClone := oldNode.DeepCopy()
-	newNodeClone.Spec.Taints = newTaints
-	newData, err := json.Marshal(newNodeClone)
-	if err != nil {
-		return fmt.Errorf("failed to marshal new node %#v for node %q: %v", newNodeClone, nodeName, err)
-	}
-
-	patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, v1.Node{})
-	if err != nil {
-		return fmt.Errorf("failed to create patch for node %q: %v", nodeName, err)
-	}
-
-	_, err = c.CoreV1().Nodes().Patch(string(nodeName), types.StrategicMergePatchType, patchBytes)
-	return err
-}
-
-// UpdateNodeTaints is for updating the node taints from oldNode to the newNode
-// using the nodes Update() method
-func UpdateNodeTaints(c clientset.Interface, nodeName string, oldNode *v1.Node, newNode *v1.Node) error {
-	newNodeClone := oldNode.DeepCopy()
-	newNodeClone.Spec.Taints = newNode.Spec.Taints
-
-	_, err := c.CoreV1().Nodes().Update(newNodeClone)
-	if err != nil {
-		return fmt.Errorf("failed to create update taints for node %q: %v", nodeName, err)
-	}
-
-	return nil
-}
-
-// GetNodeCondition get the nodes condition matching the specified type
-func GetNodeCondition(c clientset.Interface, nodeName string, conditionType v1.NodeConditionType) (*v1.NodeCondition, error) {
-	node, err := c.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{})
-	if err != nil {
-		return nil, err
-	}
-	return conditionutils.GetNodeCondition(node, conditionType), nil
-}
-
-// AddOrUpdateConditionsOnNode adds a condition to the node's status
-func AddOrUpdateConditionsOnNode(c clientset.Interface, nodeName string, condition v1.NodeCondition) error {
-	firstTry := true
-	return clientretry.RetryOnConflict(Backoff, func() error {
-		var err error
-		var oldNode *v1.Node
-		// First we try getting node from the API server cache, as it's cheaper. If it fails
-		// we get it from etcd to be sure to have fresh data.
-		if firstTry {
-			oldNode, err = c.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{ResourceVersion: "0"})
-			firstTry = false
-		} else {
-			oldNode, err = c.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{})
-		}
-
-		if err != nil {
-			return err
-		}
-
-		var newNode *v1.Node
-		oldNodeCopy := oldNode
-		newNode = conditionutils.AddOrUpdateCondition(oldNodeCopy, condition)
-		return UpdateNodeConditions(c, nodeName, oldNode, newNode)
-	})
-}
-
-// UpdateNodeConditions is for updating the node conditions from oldNode to the newNode
-// using the nodes Update() method
-func UpdateNodeConditions(c clientset.Interface, nodeName string, oldNode *v1.Node, newNode *v1.Node) error {
-	newNodeClone := oldNode.DeepCopy()
-	newNodeClone.Status.Conditions = newNode.Status.Conditions
-
-	_, err := c.CoreV1().Nodes().UpdateStatus(newNodeClone)
-	if err != nil {
-		return fmt.Errorf("failed to create update conditions for node %q: %v", nodeName, err)
-	}
-
-	return nil
-}
-
 // WaitForCacheSync is a wrapper around cache.WaitForCacheSync that generates log messages
 // indicating that the controller identified by controllerName is waiting for syncs, followed by
 // either a successful or failed sync.

pkg/controller/deployment_rollback.go

@@ -27,6 +27,7 @@ import (
 	"time"
 
 	"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
+	"github.com/gardener/machine-controller-manager/pkg/util/nodeops"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -176,7 +177,7 @@ func (dc *controller) removeTaintNodesBackingMachineSet(machineSet *v1alpha1.Mac
 				continue
 			}
 
-			err = RemoveTaintOffNode(
+			err = nodeops.RemoveTaintOffNode(
 				dc.targetCoreClient,
 				machine.Status.Node,
 				node,

pkg/controller/deployment_rolling.go

@@ -27,14 +27,14 @@ import (
 	"sort"
 	"time"
 
+	"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
+	"github.com/gardener/machine-controller-manager/pkg/util/nodeops"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/utils/integer"
-
-	"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
 	"k8s.io/klog"
+	"k8s.io/utils/integer"
 )
 
 var (
@@ -322,7 +322,7 @@ func (dc *controller) taintNodesBackingMachineSets(MachineSets []*v1alpha1.Machi
 		// to avoid scheduling on older machines
 		for _, machine := range filteredMachines {
 			if machine.Status.Node != "" {
-				err = AddOrUpdateTaintOnNode(
+				err = nodeops.AddOrUpdateTaintOnNode(
 					dc.targetCoreClient,
 					machine.Status.Node,
 					taint,

pkg/controller/machine_util.go

@@ -26,6 +26,7 @@ import (
 	"encoding/json"
 
 	"github.com/gardener/machine-controller-manager/pkg/apis/machine/validation"
+	"github.com/gardener/machine-controller-manager/pkg/util/nodeops"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog"
@@ -520,7 +521,7 @@ func (c *controller) UpdateNodeTerminationCondition(machine *v1alpha1.Machine) e
 	}
 
 	// check if condition already exists
-	cond, err := GetNodeCondition(c.targetCoreClient, nodeName, NodeTerminationCondition)
+	cond, err := nodeops.GetNodeCondition(c.targetCoreClient, nodeName, NodeTerminationCondition)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			return nil
@@ -536,7 +537,7 @@ func (c *controller) UpdateNodeTerminationCondition(machine *v1alpha1.Machine) e
 		setTerminationReasonByPhase(machine.Status.CurrentStatus.Phase, &terminationCondition)
 	}
 
-	err = AddOrUpdateConditionsOnNode(c.targetCoreClient, nodeName, terminationCondition)
+	err = nodeops.AddOrUpdateConditionsOnNode(c.targetCoreClient, nodeName, terminationCondition)
 	if apierrors.IsNotFound(err) {
 		return nil
 	}