Merge pull request #86215 from whalecold/history

Fix(kubectl): the fields of history controllerrevision will be covered with daemonset

staging/src/k8s.io/kubectl/pkg/polymorphichelpers/history.go

@@ -348,20 +348,20 @@ func statefulSetHistory(
 
 // applyDaemonSetHistory returns a specific revision of DaemonSet by applying the given history to a copy of the given DaemonSet
 func applyDaemonSetHistory(ds *appsv1.DaemonSet, history *appsv1.ControllerRevision) (*appsv1.DaemonSet, error) {
-	clone := ds.DeepCopy()
+	dsBytes, err := json.Marshal(ds)
-	cloneBytes, err := json.Marshal(clone)
 	if err != nil {
 		return nil, err
 	}
-	patched, err := strategicpatch.StrategicMergePatch(cloneBytes, history.Data.Raw, clone)
+	patched, err := strategicpatch.StrategicMergePatch(dsBytes, history.Data.Raw, ds)
 	if err != nil {
 		return nil, err
 	}
-	err = json.Unmarshal(patched, clone)
+	result := &appsv1.DaemonSet{}
+	err = json.Unmarshal(patched, result)
 	if err != nil {
 		return nil, err
 	}
-	return clone, nil
+	return result, nil
 }
 
 // TODO: copied here until this becomes a describer

staging/src/k8s.io/kubectl/pkg/polymorphichelpers/history_test.go

@@ -17,12 +17,14 @@ limitations under the License.
 package polymorphichelpers
 
 import (
-	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"reflect"
 	"testing"
 
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	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"
 )
@@ -105,3 +107,123 @@ func TestViewHistory(t *testing.T) {
 	}
 
 }
+
+func TestApplyDaemonSetHistory(t *testing.T) {
+	tests := []struct {
+		name     string
+		source   *appsv1.DaemonSet
+		expected *appsv1.DaemonSet
+	}{
+		{
+			"test_less",
+			&appsv1.DaemonSet{
+				Spec: appsv1.DaemonSetSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{"version": "v3"},
+						},
+						Spec: corev1.PodSpec{
+							InitContainers:   []corev1.Container{{Name: "i0"}},
+							Containers:       []corev1.Container{{Name: "c0"}},
+							Volumes:          []corev1.Volume{{Name: "v0"}},
+							NodeSelector:     map[string]string{"1dsa": "n0"},
+							ImagePullSecrets: []corev1.LocalObjectReference{{Name: "ips0"}},
+							Tolerations:      []corev1.Toleration{{Key: "t0"}},
+							HostAliases:      []corev1.HostAlias{{IP: "h0"}},
+							ReadinessGates:   []corev1.PodReadinessGate{{ConditionType: corev1.PodScheduled}},
+						},
+					},
+				},
+			},
+			&appsv1.DaemonSet{
+				Spec: appsv1.DaemonSetSpec{
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{{Name: "c1"}},
+							// keep diversity field, eg: nil or empty slice
+							InitContainers: []corev1.Container{},
+						},
+					},
+				},
+			},
+		},
+		{
+			"test_more",
+			&appsv1.DaemonSet{
+				Spec: appsv1.DaemonSetSpec{
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							InitContainers: []corev1.Container{{Name: "i0"}},
+						},
+					},
+				},
+			},
+			&appsv1.DaemonSet{
+				Spec: appsv1.DaemonSetSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{"version": "v3"},
+						},
+						Spec: corev1.PodSpec{
+							InitContainers: []corev1.Container{{Name: "i1"}},
+							Containers:     []corev1.Container{{Name: "c1"}},
+							Volumes:        []corev1.Volume{{Name: "v1"}},
+						},
+					},
+				},
+			},
+		},
+		{
+			"test_equal",
+			&appsv1.DaemonSet{
+				Spec: appsv1.DaemonSetSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{"version": "v0"},
+						},
+						Spec: corev1.PodSpec{
+							Containers:     []corev1.Container{{Name: "c1"}},
+							InitContainers: []corev1.Container{{Name: "i0"}},
+							Volumes:        []corev1.Volume{{Name: "v0"}},
+						},
+					},
+				},
+			},
+			&appsv1.DaemonSet{
+				Spec: appsv1.DaemonSetSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{"version": "v1"},
+						},
+						Spec: corev1.PodSpec{
+							InitContainers: []corev1.Container{{Name: "i1"}},
+							Containers:     []corev1.Container{{Name: "c1"}},
+							Volumes:        []corev1.Volume{{Name: "v1"}},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			patch, err := getDaemonSetPatch(tt.expected)
+			if err != nil {
+				t.Errorf("getDaemonSetPatch failed : %v", err)
+			}
+			cr := &appsv1.ControllerRevision{
+				Data: runtime.RawExtension{
+					Raw: patch,
+				},
+			}
+			tt.source, err = applyDaemonSetHistory(tt.source, cr)
+			if err != nil {
+				t.Errorf("applyDaemonSetHistory failed : %v", err)
+			}
+			if !apiequality.Semantic.DeepEqual(tt.source, tt.expected) {
+				t.Errorf("expected out [%v]  but get [%v]", tt.expected, tt.source)
+			}
+		})
+	}
+}

staging/src/k8s.io/kubectl/pkg/polymorphichelpers/rollback.go

@@ -392,16 +392,16 @@ var appsCodec = scheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)
 // applyRevision returns a new StatefulSet constructed by restoring the state in revision to set. If the returned error
 // is nil, the returned StatefulSet is valid.
 func applyRevision(set *appsv1.StatefulSet, revision *appsv1.ControllerRevision) (*appsv1.StatefulSet, error) {
-	clone := set.DeepCopy()
+	patched, err := strategicpatch.StrategicMergePatch([]byte(runtime.EncodeOrDie(appsCodec, set)), revision.Data.Raw, set)
-	patched, err := strategicpatch.StrategicMergePatch([]byte(runtime.EncodeOrDie(appsCodec, clone)), revision.Data.Raw, clone)
 	if err != nil {
 		return nil, err
 	}
-	err = json.Unmarshal(patched, clone)
+	result := &appsv1.StatefulSet{}
+	err = json.Unmarshal(patched, result)
 	if err != nil {
 		return nil, err
 	}
-	return clone, nil
+	return result, nil
 }
 
 // statefulsetMatch check if the given StatefulSet's template matches the template stored in the given history.

staging/src/k8s.io/kubectl/pkg/polymorphichelpers/rollback_test.go

@@ -20,13 +20,17 @@ import (
 	"reflect"
 	"testing"
 
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
 )
 
-var rollbacktests = map[schema.GroupKind]reflect.Type{
+var rollbackTests = map[schema.GroupKind]reflect.Type{
 	{Group: "apps", Kind: "DaemonSet"}:   reflect.TypeOf(&DaemonSetRollbacker{}),
 	{Group: "apps", Kind: "StatefulSet"}: reflect.TypeOf(&StatefulSetRollbacker{}),
 	{Group: "apps", Kind: "Deployment"}:  reflect.TypeOf(&DeploymentRollbacker{}),
@@ -35,7 +39,7 @@ var rollbacktests = map[schema.GroupKind]reflect.Type{
 func TestRollbackerFor(t *testing.T) {
 	fakeClientset := &fake.Clientset{}
 
-	for kind, expectedType := range rollbacktests {
+	for kind, expectedType := range rollbackTests {
 		result, err := RollbackerFor(kind, fakeClientset)
 		if err != nil {
 			t.Fatalf("error getting Rollbacker for a %v: %v", kind.String(), err)
@@ -65,3 +69,120 @@ func TestGetDeploymentPatch(t *testing.T) {
 		t.Errorf("expected:\n%s\ngot\n%s", expectedPatch, string(patchBytes))
 	}
 }
+
+func TestStatefulSetApplyRevision(t *testing.T) {
+	tests := []struct {
+		name     string
+		source   *appsv1.StatefulSet
+		expected *appsv1.StatefulSet
+	}{
+		{
+			"test_less",
+			&appsv1.StatefulSet{
+				Spec: appsv1.StatefulSetSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{"version": "v3"},
+						},
+						Spec: corev1.PodSpec{
+							InitContainers: []corev1.Container{{Name: "i0"}},
+							Containers:     []corev1.Container{{Name: "c0"}},
+							Volumes:        []corev1.Volume{{Name: "v0"}},
+							NodeSelector:   map[string]string{"1dsa": "n0"},
+						},
+					},
+				},
+			},
+			&appsv1.StatefulSet{
+				Spec: appsv1.StatefulSetSpec{
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{{Name: "c1"}},
+							// keep diversity field, eg: nil or empty slice
+							InitContainers: []corev1.Container{},
+						},
+					},
+				},
+			},
+		},
+		{
+			"test_more",
+			&appsv1.StatefulSet{
+				Spec: appsv1.StatefulSetSpec{
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							InitContainers: []corev1.Container{{Name: "i0"}},
+						},
+					},
+				},
+			},
+			&appsv1.StatefulSet{
+				Spec: appsv1.StatefulSetSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{"version": "v3"},
+						},
+						Spec: corev1.PodSpec{
+							InitContainers: []corev1.Container{{Name: "i1"}},
+							Containers:     []corev1.Container{{Name: "c1"}},
+							Volumes:        []corev1.Volume{{Name: "v1"}},
+						},
+					},
+				},
+			},
+		},
+		{
+			"test_equal",
+			&appsv1.StatefulSet{
+				Spec: appsv1.StatefulSetSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{"version": "v3"},
+						},
+						Spec: corev1.PodSpec{
+							Containers:     []corev1.Container{{Name: "c1"}},
+							InitContainers: []corev1.Container{{Name: "i0"}},
+							Volumes:        []corev1.Volume{{Name: "v0"}},
+						},
+					},
+				},
+			},
+			&appsv1.StatefulSet{
+				Spec: appsv1.StatefulSetSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{"version": "v2"},
+						},
+						Spec: corev1.PodSpec{
+							InitContainers: []corev1.Container{{Name: "i1"}},
+							Containers:     []corev1.Container{{Name: "c1"}},
+							Volumes:        []corev1.Volume{{Name: "v1"}},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			patch, err := getStatefulSetPatch(tt.expected)
+			if err != nil {
+				t.Errorf("getStatefulSetPatch failed : %v", err)
+			}
+			cr := &appsv1.ControllerRevision{
+				Data: runtime.RawExtension{
+					Raw: patch,
+				},
+			}
+			tt.source, err = applyRevision(tt.source, cr)
+			if err != nil {
+				t.Errorf("apply revision failed : %v", err)
+			}
+			// applyRevision adds TypeMeta field to new the statefulset, so use spec to compare only.
+			if !apiequality.Semantic.DeepEqual(tt.source.Spec, tt.expected.Spec) {
+				t.Errorf("expected out [%v]  but get [%v]", tt.expected.Spec, tt.source.Spec)
+			}
+		})
+	}
+}