diff --git a/staging/src/k8s.io/kubectl/pkg/drain/cordon.go b/staging/src/k8s.io/kubectl/pkg/drain/cordon.go index 006eef76259..2820cf85c02 100644 --- a/staging/src/k8s.io/kubectl/pkg/drain/cordon.go +++ b/staging/src/k8s.io/kubectl/pkg/drain/cordon.go @@ -86,9 +86,10 @@ func (c *CordonHelper) PatchOrReplaceWithContext(clientCtx context.Context, clie return err, nil } - c.node.Spec.Unschedulable = c.desired + desiredNode := c.node.DeepCopy() + desiredNode.Spec.Unschedulable = c.desired - newData, err := json.Marshal(c.node) + newData, err := json.Marshal(desiredNode) if err != nil { return err, nil } @@ -99,13 +100,21 @@ func (c *CordonHelper) PatchOrReplaceWithContext(clientCtx context.Context, clie if serverDryRun { patchOptions.DryRun = []string{metav1.DryRunAll} } - _, err = client.Patch(clientCtx, c.node.Name, types.StrategicMergePatchType, patchBytes, patchOptions) + var updatedNode *corev1.Node + updatedNode, err = client.Patch(clientCtx, c.node.Name, types.StrategicMergePatchType, patchBytes, patchOptions) + if err == nil { + c.node.Spec.Unschedulable = updatedNode.Spec.Unschedulable + } } else { updateOptions := metav1.UpdateOptions{} if serverDryRun { updateOptions.DryRun = []string{metav1.DryRunAll} } - _, err = client.Update(clientCtx, c.node, updateOptions) + var updatedNode *corev1.Node + updatedNode, err = client.Update(clientCtx, desiredNode, updateOptions) + if err == nil { + c.node.Spec.Unschedulable = updatedNode.Spec.Unschedulable + } } return err, patchErr } diff --git a/staging/src/k8s.io/kubectl/pkg/drain/cordon_test.go b/staging/src/k8s.io/kubectl/pkg/drain/cordon_test.go index f584f564f39..c7281f6bd55 100644 --- a/staging/src/k8s.io/kubectl/pkg/drain/cordon_test.go +++ b/staging/src/k8s.io/kubectl/pkg/drain/cordon_test.go @@ -17,6 +17,8 @@ limitations under the License. package drain import ( + "context" + "errors" "testing" "github.com/google/go-cmp/cmp" @@ -24,6 +26,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" ) type newCordonHelperFromRuntimeObjectTestCase struct { @@ -145,3 +149,95 @@ func TestUpdateIfRequired(t *testing.T) { }) } } + +func TestCordonHelper_PatchOrReplaceWithContext(t *testing.T) { + tests := []struct { + name string + node *corev1.Node + clientset *fake.Clientset + serverDryRun bool + shouldFail bool + }{ + { + name: "update success: local object should be updated", + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-node", + }, + Spec: corev1.NodeSpec{ + Unschedulable: false, + }, + }, + clientset: fake.NewClientset(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-node", + }, + Spec: corev1.NodeSpec{ + Unschedulable: false, + }, + }), + serverDryRun: false, + shouldFail: false, + }, + { + name: "update faild: local object should not be updated", + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-node", + }, + Spec: corev1.NodeSpec{ + Unschedulable: false, + }, + }, + clientset: fake.NewClientset(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-node", + }, + Spec: corev1.NodeSpec{ + Unschedulable: false, + }, + }), + serverDryRun: false, + shouldFail: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewCordonHelper(tt.node) + c.desired = true + if tt.shouldFail { + tt.clientset.Fake.PrependReactor("update", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake update error") + }) + tt.clientset.Fake.PrependReactor("patch", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake update error") + }) + } + + err, _ := c.PatchOrReplaceWithContext(context.Background(), tt.clientset, tt.serverDryRun) + if tt.shouldFail { + if err == nil { + t.Errorf("PatchOrReplaceWithContext() failed: update should failed but no error returned") + return + } + } else { + if err != nil { + t.Errorf("PatchOrReplaceWithContext() failed: update should succeed but got error: %v", err) + return + } + } + + if err != nil { + t.Logf("patch/update failed, error: %v, c.node.Spec.Unschedulable shouldn't be updated", err) + if c.node.Spec.Unschedulable == c.desired { + t.Errorf("PatchOrReplaceWithContext() failed: update failed but c.node's unschedulable was updated") + } + } else { + t.Log("patch/update successed, c.node.Spec.Unschedulable should be updated") + if c.node.Spec.Unschedulable != c.desired { + t.Errorf("PatchOrReplaceWithContext() failed: update succeeded but c.node's unschedulable was not updated") + } + } + }) + } +}