diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/history.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/history.go index f228d8b3887..2efc828131f 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/history.go +++ b/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() - cloneBytes, err := json.Marshal(clone) + dsBytes, err := json.Marshal(ds) 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 diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/history_test.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/history_test.go index bbcd0d72dab..3bef0bc3678 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/history_test.go +++ b/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) + } + }) + } +} diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/rollback.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/rollback.go index 8127ceeca37..e3e64be8fa1 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/rollback.go +++ b/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, clone)), revision.Data.Raw, clone) + patched, err := strategicpatch.StrategicMergePatch([]byte(runtime.EncodeOrDie(appsCodec, set)), revision.Data.Raw, set) 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. diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/rollback_test.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/rollback_test.go index 673e8847b52..eaeb0513417 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/rollback_test.go +++ b/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) + } + }) + } +}