Merge pull request #51266 from resouer/not-ready

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Refactor node taint conditions

**What this PR does / why we need it**:
We should use `not-ready` etc as node condition taint key.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: 
fixes #51246 

**Special notes for your reviewer**:

**Release note**:

```release-note
Use `not-ready` to replace `notReady` in node condition taint keys.
```

pkg/api/validation/validation_test.go

@@ -5036,7 +5036,7 @@ func TestValidatePod(t *testing.T) {
 				Name:      "pod-forgiveness-invalid",
 				Namespace: "ns",
 			},
-			Spec: extendPodSpecwithTolerations(validPodSpec(nil), []api.Toleration{{Key: "node.alpha.kubernetes.io/notReady", Operator: "Exists", Effect: "NoExecute", TolerationSeconds: &[]int64{-2}[0]}}),
+			Spec: extendPodSpecwithTolerations(validPodSpec(nil), []api.Toleration{{Key: "node.kubernetes.io/not-ready", Operator: "Exists", Effect: "NoExecute", TolerationSeconds: &[]int64{-2}[0]}}),
 		},
 		{ // docker default seccomp profile
 			ObjectMeta: metav1.ObjectMeta{
@@ -5594,7 +5594,7 @@ func TestValidatePod(t *testing.T) {
 					Name:      "pod-forgiveness-invalid",
 					Namespace: "ns",
 				},
-				Spec: extendPodSpecwithTolerations(validPodSpec(nil), []api.Toleration{{Key: "node.alpha.kubernetes.io/notReady", Operator: "Exists", Effect: "NoSchedule", TolerationSeconds: &[]int64{20}[0]}}),
+				Spec: extendPodSpecwithTolerations(validPodSpec(nil), []api.Toleration{{Key: "node.kubernetes.io/not-ready", Operator: "Exists", Effect: "NoSchedule", TolerationSeconds: &[]int64{20}[0]}}),
 			},
 		},
 		"must be a valid pod seccomp profile": {

pkg/controller/node/node_controller.go

@@ -406,6 +406,17 @@ func NewNodeController(
 		})
 	}
 
+	// NOTE(resouer): nodeInformer to substitute deprecated taint key (notReady -> not-ready).
+	// Remove this logic when we don't need this backwards compatibility
+	nodeInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc: util.CreateAddNodeHandler(func(node *v1.Node) error {
+			return nc.doFixDeprecatedTaintKeyPass(node)
+		}),
+		UpdateFunc: util.CreateUpdateNodeHandler(func(_, newNode *v1.Node) error {
+			return nc.doFixDeprecatedTaintKeyPass(newNode)
+		}),
+	})
+
 	nc.nodeLister = nodeInformer.Lister()
 	nc.nodeInformerSynced = nodeInformer.Informer().HasSynced
 
@@ -444,6 +455,38 @@ func (nc *Controller) doEvictionPass() {
 	}
 }
 
+// doFixDeprecatedTaintKeyPass checks and replaces deprecated taint key with proper key name if needed.
+func (nc *Controller) doFixDeprecatedTaintKeyPass(node *v1.Node) error {
+	taintsToAdd := []*v1.Taint{}
+	taintsToDel := []*v1.Taint{}
+
+	for _, taint := range node.Spec.Taints {
+		if taint.Key == algorithm.DeprecatedTaintNodeNotReady {
+			// delete old taint
+			tDel := taint
+			taintsToDel = append(taintsToDel, &tDel)
+
+			// add right taint
+			tAdd := taint
+			tAdd.Key = algorithm.TaintNodeNotReady
+			taintsToAdd = append(taintsToAdd, &tAdd)
+
+			glog.Warningf("Detected deprecated taint key: %v on node: %v, will substitute it with %v",
+				algorithm.DeprecatedTaintNodeNotReady, node.GetName(), algorithm.TaintNodeNotReady)
+
+			break
+		}
+	}
+
+	if len(taintsToAdd) == 0 && len(taintsToDel) == 0 {
+		return nil
+	}
+	if !util.SwapNodeControllerTaint(nc.kubeClient, taintsToAdd, taintsToDel, node) {
+		return fmt.Errorf("failed to swap taints of node %+v", node)
+	}
+	return nil
+}
+
 func (nc *Controller) doNoScheduleTaintingPass(node *v1.Node) error {
 	// Map node's condition to Taints.
 	taints := []v1.Taint{}

pkg/controller/node/nodecontroller_test.go

@@ -2445,3 +2445,109 @@ func TestCheckNodeKubeletVersionParsing(t *testing.T) {
 		}
 	}
 }
+
+func TestFixDeprecatedTaintKey(t *testing.T) {
+	fakeNow := metav1.Date(2017, 1, 1, 12, 0, 0, 0, time.UTC)
+	evictionTimeout := 10 * time.Minute
+
+	fakeNodeHandler := &testutil.FakeNodeHandler{
+		Existing: []*v1.Node{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "node0",
+					CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
+					Labels: map[string]string{
+						kubeletapis.LabelZoneRegion:        "region1",
+						kubeletapis.LabelZoneFailureDomain: "zone1",
+					},
+				},
+			},
+		},
+		Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+	}
+
+	nodeController, _ := newNodeControllerFromClient(nil, fakeNodeHandler, evictionTimeout,
+		testRateLimiterQPS, testRateLimiterQPS, testLargeClusterThreshold, testUnhealthyThreshold, testNodeMonitorGracePeriod,
+		testNodeStartupGracePeriod, testNodeMonitorPeriod, nil, nil, 0, false, true)
+	nodeController.now = func() metav1.Time { return fakeNow }
+	nodeController.recorder = testutil.NewFakeRecorder()
+
+	deprecatedTaint := &v1.Taint{
+		Key:    algorithm.DeprecatedTaintNodeNotReady,
+		Effect: v1.TaintEffectNoExecute,
+	}
+	nodeNotReadyTaint := &v1.Taint{
+		Key:    algorithm.TaintNodeNotReady,
+		Effect: v1.TaintEffectNoExecute,
+	}
+
+	tests := []struct {
+		Name           string
+		Node           *v1.Node
+		ExpectedTaints []*v1.Taint
+	}{
+		{
+			Name: "Node with deprecated taint key",
+			Node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "node0",
+					CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
+					Labels: map[string]string{
+						kubeletapis.LabelZoneRegion:        "region1",
+						kubeletapis.LabelZoneFailureDomain: "zone1",
+					},
+				},
+				Spec: v1.NodeSpec{
+					Taints: []v1.Taint{
+						*deprecatedTaint,
+					},
+				},
+			},
+			ExpectedTaints: []*v1.Taint{nodeNotReadyTaint},
+		},
+		{
+			Name: "Node with not-ready taint key",
+			Node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "node0",
+					CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
+					Labels: map[string]string{
+						kubeletapis.LabelZoneRegion:        "region1",
+						kubeletapis.LabelZoneFailureDomain: "zone1",
+					},
+				},
+				Spec: v1.NodeSpec{
+					Taints: []v1.Taint{
+						*nodeNotReadyTaint,
+					},
+				},
+			},
+			ExpectedTaints: []*v1.Taint{nodeNotReadyTaint},
+		},
+	}
+
+	for _, test := range tests {
+		fakeNodeHandler.Update(test.Node)
+		if err := syncNodeStore(nodeController, fakeNodeHandler); err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+		nodeController.doFixDeprecatedTaintKeyPass(test.Node)
+		if err := syncNodeStore(nodeController, fakeNodeHandler); err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+		node0, err := nodeController.nodeLister.Get("node0")
+		if err != nil {
+			t.Errorf("Can't get current node0...")
+			return
+		}
+		if len(node0.Spec.Taints) != len(test.ExpectedTaints) {
+			t.Errorf("%s: Unexpected number of taints: expected %d, got %d",
+				test.Name, len(test.ExpectedTaints), len(node0.Spec.Taints))
+		}
+		for _, taint := range test.ExpectedTaints {
+			if !taintutils.TaintExists(node0.Spec.Taints, taint) {
+				t.Errorf("%s: Can't find taint %v in %v", test.Name, taint, node0.Spec.Taints)
+			}
+		}
+	}
+}

plugin/pkg/scheduler/algorithm/well_known_labels.go

@@ -20,7 +20,11 @@ const (
 	// When feature-gate for TaintBasedEvictions=true flag is enabled,
 	// TaintNodeNotReady would be automatically added by node controller
 	// when node is not ready, and removed when node becomes ready.
-	TaintNodeNotReady = "node.alpha.kubernetes.io/notReady"
+	TaintNodeNotReady = "node.kubernetes.io/not-ready"
+
+	// DeprecatedTaintNodeNotReady is the deprecated version of TaintNodeNotReady.
+	// It is deprecated since 1.9
+	DeprecatedTaintNodeNotReady = "node.alpha.kubernetes.io/notReady"
 
 	// When feature-gate for TaintBasedEvictions=true flag is enabled,
 	// TaintNodeUnreachable would be automatically added by node controller