diff --git a/pkg/work/spoke/apply/server_side_apply.go b/pkg/work/spoke/apply/server_side_apply.go index 6f6c1b140..7904bdecc 100644 --- a/pkg/work/spoke/apply/server_side_apply.go +++ b/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) + } + } + } + } +} diff --git a/pkg/work/spoke/apply/server_side_apply_test.go b/pkg/work/spoke/apply/server_side_apply_test.go index c60959d01..1637849b9 100644 --- a/pkg/work/spoke/apply/server_side_apply_test.go +++ b/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} +} diff --git a/test/integration/work/updatestrategy_test.go b/test/integration/work/updatestrategy_test.go index da9cc0c07..fc0920dc4 100644 --- a/test/integration/work/updatestrategy_test.go +++ b/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()) + }) })