remove the creationTimestamp from metadata when using ssa apply manifests (#611)

Signed-off-by: Wei Liu <liuweixa@redhat.com>

pkg/work/spoke/apply/server_side_apply.go

@@ -12,6 +12,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/klog/v2"
 
 	workapiv1 "open-cluster-management.io/api/work/v1"
 
@@ -52,6 +53,13 @@ func (c *ServerSideApply) Apply(
 		}
 	}
 
+	// Currently, if the required object has zero creationTime in metadata, it will cause
+	// kube-apiserver to increment generation even if nothing else changes. more details see:
+	// https://github.com/kubernetes/kubernetes/issues/67610
+	//
+	// TODO Remove this after the above issue fixed in Kubernetes
+	removeCreationTimeFromMetadata(required.Object)
+
 	obj, err := c.client.
 		Resource(gvr).
 		Namespace(required.GetNamespace()).
@@ -72,3 +80,30 @@ func (c *ServerSideApply) Apply(
 
 	return obj, err
 }
+
+func removeCreationTimeFromMetadata(obj map[string]interface{}) {
+	if metadata, found := obj["metadata"]; found {
+		if metaObj, ok := metadata.(map[string]interface{}); ok {
+			klog.V(4).Infof("remove `metadata.creationTimestamp`")
+			creationTimestamp, ok := metaObj["creationTimestamp"]
+			if ok && creationTimestamp == nil {
+				unstructured.RemoveNestedField(metaObj, "creationTimestamp")
+			}
+		}
+	}
+
+	for k, v := range obj {
+		switch val := v.(type) {
+		case map[string]interface{}:
+			klog.V(4).Infof("remove `metadata.creationTimestamp` from %s", k)
+			removeCreationTimeFromMetadata(val)
+		case []interface{}:
+			for index, item := range val {
+				klog.V(4).Infof("remove `metadata.creationTimestamp` from %s[%d]", k, index)
+				if itemObj, ok := item.(map[string]interface{}); ok {
+					removeCreationTimeFromMetadata(itemObj)
+				}
+			}
+		}
+	}
+}

pkg/work/spoke/apply/server_side_apply_test.go

@@ -6,6 +6,8 @@ import (
 	"testing"
 
 	"github.com/pkg/errors"
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -139,3 +141,141 @@ func (r *reactor) React(action clienttesting.Action) (handled bool, ret runtime.
 
 	return true, nil, fmt.Errorf("PatchType is not supported")
 }
+
+func TestRemoveCreationTime(t *testing.T) {
+	cases := []struct {
+		name         string
+		required     *unstructured.Unstructured
+		validateFunc func(t *testing.T, obj *unstructured.Unstructured)
+	}{
+		{
+			name:     "remove creationTimestamp from a kube object",
+			required: newDeployment(),
+			validateFunc: func(t *testing.T, obj *unstructured.Unstructured) {
+				_, existing, err := unstructured.NestedFieldCopy(obj.Object, "metadata", "creationTimestamp")
+				if err != nil {
+					t.Fatal(err)
+				}
+				if existing {
+					t.Errorf("unexpected creationTimestamp in `metadata.creationTimestamp`")
+				}
+				_, existing, err = unstructured.NestedFieldCopy(obj.Object, "spec", "template", "metadata", "creationTimestamp")
+				if err != nil {
+					t.Fatal(err)
+				}
+				if existing {
+					t.Errorf("unexpected creationTimestamp in `spec.template.metadata.creationTimestamp`")
+				}
+			},
+		},
+		{
+			name:     "remove creationTimestamp from a manifestwork",
+			required: newManifestWork(),
+			validateFunc: func(t *testing.T, obj *unstructured.Unstructured) {
+				_, existing, err := unstructured.NestedFieldCopy(obj.Object, "metadata", "creationTimestamp")
+				if err != nil {
+					t.Fatal(err)
+				}
+				if existing {
+					t.Errorf("unexpected creationTimestamp in `metadata.creationTimestamp`")
+				}
+
+				manifests, existing, err := unstructured.NestedSlice(obj.Object, "spec", "workload", "manifests")
+				if err != nil {
+					t.Fatal(err)
+				}
+				if !existing {
+					t.Fatalf("no manifests")
+				}
+
+				_, existing, err = unstructured.NestedFieldCopy(manifests[0].(map[string]interface{}), "metadata", "creationTimestamp")
+				if err != nil {
+					t.Fatal(err)
+				}
+				if existing {
+					t.Errorf("unexpected creationTimestamp in `spec.workload.manifests[0].metadata.creationTimestamp`")
+				}
+			},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			removeCreationTimeFromMetadata(c.required.Object)
+			c.validateFunc(t, c.required)
+		})
+	}
+}
+
+func newDeployment() *unstructured.Unstructured {
+	var replicas int32 = 3
+	deploy := &appsv1.Deployment{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "apps/v1",
+			Kind:       "Deployment",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "test",
+		},
+		Spec: appsv1.DeploymentSpec{
+			Replicas: &replicas,
+			Template: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{"test": "test"},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name:  "test",
+							Image: "test",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	obj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(deploy)
+	return &unstructured.Unstructured{Object: obj}
+}
+
+func newManifestWork() *unstructured.Unstructured {
+	cm := &corev1.ConfigMap{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "ConfigMap",
+			APIVersion: "v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "cm-test",
+			Namespace: "default",
+		},
+		Data: map[string]string{
+			"some": "data",
+		},
+	}
+
+	cmObj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(cm)
+	raw, _ := (&unstructured.Unstructured{Object: cmObj}).MarshalJSON()
+	manifest := workapiv1.Manifest{}
+	manifest.Raw = raw
+
+	work := &workapiv1.ManifestWork{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "work.open-cluster-management.io/v1",
+			Kind:       "ManifestWork",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "test",
+		},
+		Spec: workapiv1.ManifestWorkSpec{
+			Workload: workapiv1.ManifestsTemplate{
+				Manifests: []workapiv1.Manifest{manifest},
+			},
+		},
+	}
+
+	obj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(work)
+	return &unstructured.Unstructured{Object: obj}
+}

test/integration/work/updatestrategy_test.go

@@ -557,4 +557,60 @@ var _ = ginkgo.Describe("ManifestWork Update Strategy", func() {
 			}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())
 		})
 	})
+
+	ginkgo.It("should not increase the workload generation when nothing changes", func() {
+		nestedWorkNamespace := "default"
+		nestedWorkName := fmt.Sprintf("nested-work-%s", utilrand.String(5))
+
+		cm := util.NewConfigmap(nestedWorkNamespace, "cm-test", map[string]string{"a": "b"}, []string{})
+		nestedWork := util.NewManifestWork(nestedWorkNamespace, nestedWorkName, []workapiv1.Manifest{util.ToManifest(cm)})
+		nestedWork.TypeMeta = metav1.TypeMeta{
+			APIVersion: "work.open-cluster-management.io/v1",
+			Kind:       "ManifestWork",
+		}
+
+		work := util.NewManifestWork(commOptions.SpokeClusterName, "", []workapiv1.Manifest{util.ToManifest(nestedWork)})
+		work.Spec.ManifestConfigs = []workapiv1.ManifestConfigOption{
+			{
+				ResourceIdentifier: workapiv1.ResourceIdentifier{
+					Group:     "work.open-cluster-management.io",
+					Resource:  "manifestworks",
+					Namespace: nestedWorkNamespace,
+					Name:      nestedWorkName,
+				},
+				UpdateStrategy: &workapiv1.UpdateStrategy{
+					Type: workapiv1.UpdateStrategyTypeServerSideApply,
+					ServerSideApply: &workapiv1.ServerSideApplyConfig{
+						Force: true,
+					},
+				},
+			},
+		}
+		_, err = hubWorkClient.WorkV1().ManifestWorks(commOptions.SpokeClusterName).Create(context.Background(), work, metav1.CreateOptions{})
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+		// make sure the nested work is created
+		gomega.Eventually(func() error {
+			_, err := spokeWorkClient.WorkV1().ManifestWorks(nestedWorkNamespace).Get(context.Background(), nestedWorkName, metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+
+			return nil
+		}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())
+
+		// make sure the nested work is not updated
+		gomega.Consistently(func() error {
+			nestedWork, err := spokeWorkClient.WorkV1().ManifestWorks(nestedWorkNamespace).Get(context.Background(), nestedWorkName, metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+
+			if nestedWork.Generation != 1 {
+				return fmt.Errorf("nested work generation is changed to %d", nestedWork.Generation)
+			}
+
+			return nil
+		}, eventuallyTimeout*3, eventuallyInterval*3).Should(gomega.BeNil())
+	})
 })