From a687fdc1c56dcb5c8a0c5b6bbfb5c1bb4800532f Mon Sep 17 00:00:00 2001 From: Karl Isenberg Date: Tue, 2 Jul 2024 12:24:24 -0700 Subject: [PATCH] Add watch filtering by package-id label go/config-sync-watch-filter --- e2e/testcases/custom_resources_test.go | 14 +- e2e/testcases/managed_resources_test.go | 46 +++++-- e2e/testcases/multi_sync_test.go | 18 +-- e2e/testcases/preserve_fields_test.go | 26 ++-- pkg/applier/applier_test.go | 9 +- pkg/applier/clientset.go | 16 ++- pkg/metadata/labels.go | 6 + pkg/metadata/metadata_test.go | 6 + pkg/metadata/package_id.go | 44 ++++++ pkg/metadata/package_id_test.go | 125 ++++++++++++++++++ pkg/parse/annotations.go | 2 + pkg/parse/annotations_test.go | 12 +- pkg/parse/root_test.go | 43 +++++- pkg/reconciler/reconciler.go | 4 +- .../controllers/reconciler_base.go | 25 ++++ .../controllers/reposync_controller.go | 6 +- .../controllers/rootsync_controller.go | 6 +- pkg/remediator/watch/filteredwatcher_test.go | 2 + pkg/remediator/watch/manager.go | 13 ++ pkg/remediator/watch/manager_test.go | 2 + pkg/remediator/watch/watcher.go | 5 + 21 files changed, 376 insertions(+), 54 deletions(-) create mode 100644 pkg/metadata/package_id.go create mode 100644 pkg/metadata/package_id_test.go diff --git a/e2e/testcases/custom_resources_test.go b/e2e/testcases/custom_resources_test.go index 66444db2b9..02c9f5fee4 100644 --- a/e2e/testcases/custom_resources_test.go +++ b/e2e/testcases/custom_resources_test.go @@ -109,16 +109,12 @@ func TestCRDDeleteBeforeRemoveCustomResourceV1(t *testing.T) { nt.T.Fatal(err) } - // Wait for remediator to detect the drift (managed Anvil CR was deleted) - // and record it as a conflict. - // TODO: distinguish between management conflict (spec/generation drift) and concurrent status update conflict (resource version change) - err = nomostest.ValidateMetrics(nt, + // Reverting a manual change is NOT considered a "conflict", unless the new + // owner is a different reconciler. + nt.Must(nomostest.ValidateMetrics(nt, nomostest.ReconcilerErrorMetrics(nt, rootSyncLabels, firstCommitHash, metrics.ErrorSummary{ - Conflicts: 1, - })) - if err != nil { - nt.T.Fatal(err) - } + Conflicts: 0, // from the remediator + }))) // Reset discovery client to invalidate the cached Anvil CRD nt.RenewClient() diff --git a/e2e/testcases/managed_resources_test.go b/e2e/testcases/managed_resources_test.go index f8a18d9a83..c5ba2629b5 100644 --- a/e2e/testcases/managed_resources_test.go +++ b/e2e/testcases/managed_resources_test.go @@ -37,19 +37,21 @@ import ( // This file includes tests for drift correction and drift prevention. // -// The drift correction in the mono-repo mode utilizes the following two annotations: -// * configmanagement.gke.io/managed -// * configsync.gke.io/resource-id +// The drift correction in the mono-repo mode utilizes the following metadata: +// * the configmanagement.gke.io/managed annotation +// * the configsync.gke.io/resource-id annotation // -// The drift correction in the multi-repo mode utilizes the following three annotations: -// * configmanagement.gke.io/managed -// * configsync.gke.io/resource-id -// * configsync.gke.io/manager +// The drift correction in the multi-repo mode utilizes the following metadata: +// * the configmanagement.gke.io/managed annotation +// * the configsync.gke.io/resource-id annotation +// * the configsync.gke.io/manager annotation +// * the config.k8s.io/owning-inventory label // // The drift prevention is only supported in the multi-repo mode, and utilizes the following Config Sync metadata: // * the configmanagement.gke.io/managed annotation // * the configsync.gke.io/resource-id annotation // * the configsync.gke.io/declared-version label +// * the config.k8s.io/owning-inventory label // The reason we have both TestKubectlCreatesManagedNamespaceResourceMonoRepo and // TestKubectlCreatesManagedNamespaceResourceMultiRepo is that the mono-repo mode and @@ -79,6 +81,8 @@ metadata: configmanagement.gke.io/managed: enabled configsync.gke.io/resource-id: _namespace_test-ns1 configsync.gke.io/manager: :root + labels: + configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync `) if err := os.WriteFile(filepath.Join(nt.TmpDir, "test-ns1.yaml"), ns, 0644); err != nil { @@ -106,7 +110,9 @@ metadata: annotations: configmanagement.gke.io/managed: enabled configsync.gke.io/resource-id: _namespace_test-ns2 - configsync.gke.io/manager: :abcdef + configsync.gke.io/manager: config-management-system_abcdef + labels: + configsync.gke.io/parent-package-id: abcdef.config-management-system.RootSync `) if err := os.WriteFile(filepath.Join(nt.TmpDir, "test-ns2.yaml"), ns, 0644); err != nil { @@ -121,8 +127,10 @@ metadata: // Wait 5 seconds so that the remediator can process the event. time.Sleep(5 * time.Second) - // The `configsync.gke.io/manager` annotation of `test-ns2` suggests that its manager is ':abcdef'. - // The root reconciler does not manage `test-ns2`, therefore should not remove `test-ns2`. + // The `configsync.gke.io/manager` annotation of `test-ns2` suggests that + // its manager is 'config-management-system_abcdef' (RootSync named 'abcdef'). + // The root reconciler (RootSync named 'root-sync') does not manage + // `test-ns2`, therefore should not remove `test-ns2`. err = nt.Validate("test-ns2", "", &corev1.Namespace{}, testpredicates.HasExactlyAnnotationKeys( metadata.ResourceManagementKey, @@ -233,6 +241,8 @@ metadata: configmanagement.gke.io/managed: enabled configsync.gke.io/resource-id: _configmap_bookstore_test-cm1 configsync.gke.io/manager: :root + labels: + configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync data: weekday: "monday" `) @@ -263,6 +273,8 @@ metadata: configmanagement.gke.io/managed: enabled configsync.gke.io/resource-id: _configmap_bookstore_wrong-cm2 configsync.gke.io/manager: :root + labels: + configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync data: weekday: "monday" `) @@ -301,7 +313,9 @@ metadata: annotations: configmanagement.gke.io/managed: enabled configsync.gke.io/resource-id: _configmap_bookstore_test-cm3 - configsync.gke.io/manager: :abcdef + configsync.gke.io/manager: config-management-system_abcdef + labels: + configsync.gke.io/parent-package-id: abcdef.config-management-system.RootSync data: weekday: "monday" `) @@ -318,8 +332,10 @@ data: // Wait 5 seconds so that the reconciler can process the event. time.Sleep(5 * time.Second) - // The `configsync.gke.io/manager` annotation of `test-ns3` suggests that its manager is ':abcdef'. - // The root reconciler does not manage `test-ns3`, therefore should not remove `test-ns3`. + // The `configsync.gke.io/manager` annotation of `test-ns3` suggests that + // its manager is 'config-management-system_abcdef' (RootSync named 'abcdef'). + // The root reconciler (RootSync named 'root-sync') does not manage `test-ns3`, + // therefore should not remove `test-ns3`. err = nt.Validate("test-cm3", "bookstore", &corev1.ConfigMap{}, testpredicates.HasExactlyAnnotationKeys( metadata.ResourceManagementKey, @@ -340,6 +356,8 @@ metadata: annotations: configmanagement.gke.io/managed: enabled configsync.gke.io/resource-id: _configmap_bookstore_test-cm4 + labels: + configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync data: weekday: "monday" `) @@ -378,6 +396,8 @@ metadata: configmanagement.gke.io/managed: enabled configsync.gke.io/resource-id: _configmap_bookstore_test-secret configsync.gke.io/manager: :root + labels: + configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync `) if err := os.WriteFile(filepath.Join(nt.TmpDir, "test-secret.yaml"), cm, 0644); err != nil { diff --git a/e2e/testcases/multi_sync_test.go b/e2e/testcases/multi_sync_test.go index 362700ad73..5cb8b7a797 100644 --- a/e2e/testcases/multi_sync_test.go +++ b/e2e/testcases/multi_sync_test.go @@ -453,20 +453,20 @@ func TestConflictingDefinitions_NamespaceToRoot(t *testing.T) { } // Validate reconciler error metric is emitted from namespace reconciler. - rootSyncLabels, err := nomostest.MetricLabelsForRepoSync(nt, repoSyncNN) + repoSyncLabels, err := nomostest.MetricLabelsForRepoSync(nt, repoSyncNN) if err != nil { nt.T.Fatal(err) } commitHash := nt.NonRootRepos[repoSyncNN].MustHash(nt.T) - err = nomostest.ValidateMetrics(nt, - nomostest.ReconcilerSyncError(nt, rootSyncLabels, commitHash), - nomostest.ReconcilerErrorMetrics(nt, rootSyncLabels, commitHash, metrics.ErrorSummary{ - Sync: 1, - })) - if err != nil { - nt.T.Fatal(err) - } + // Reverting a manual change IS considered a "conflict" when the new + // owner is a different reconciler. + nt.Must(nomostest.ValidateMetrics(nt, + nomostest.ReconcilerSyncError(nt, repoSyncLabels, commitHash), + nomostest.ReconcilerErrorMetrics(nt, repoSyncLabels, commitHash, metrics.ErrorSummary{ + Conflicts: 1, // from the remediator + Sync: 1, // remediator conflict error replaces applier conflict error + }))) nt.T.Logf("Ensure the Role matches the one in the Root repo %s", configsync.RootSyncName) err = nt.Validate("pods", testNs, &rbacv1.Role{}, diff --git a/e2e/testcases/preserve_fields_test.go b/e2e/testcases/preserve_fields_test.go index 30c8ca13c7..48ff5ba92f 100644 --- a/e2e/testcases/preserve_fields_test.go +++ b/e2e/testcases/preserve_fields_test.go @@ -333,7 +333,11 @@ func TestAddUpdateDeleteLabels(t *testing.T) { nt.T.Fatal(err) } - var defaultLabels = []string{metadata.ManagedByKey, metadata.DeclaredVersionLabel} + var defaultLabels = []string{ + metadata.ManagedByKey, + metadata.ParentPackageIDLabel, + metadata.DeclaredVersionLabel, + } // Checking that the configmap with no labels appears on cluster, and // that no user labels are specified @@ -350,12 +354,13 @@ func TestAddUpdateDeleteLabels(t *testing.T) { nt.T.Fatal(err) } - // Checking that label is updated after syncing an update. - err = nt.Validate(cmName, ns, &corev1.ConfigMap{}, - testpredicates.HasExactlyLabelKeys(append(defaultLabels, "baz")...)) - if err != nil { - nt.T.Fatal(err) - } + var updatedLabels []string + updatedLabels = append(updatedLabels, defaultLabels...) + updatedLabels = append(updatedLabels, "baz") + + // Checking that label was added after syncing. + nt.Must(nt.Validate(cmName, ns, &corev1.ConfigMap{}, + testpredicates.HasExactlyLabelKeys(updatedLabels...))) delete(cm.Labels, "baz") nt.Must(nt.RootRepos[configsync.RootSyncName].Add(cmPath, cm)) @@ -365,11 +370,8 @@ func TestAddUpdateDeleteLabels(t *testing.T) { } // Check that the label is deleted after syncing. - err = nt.Validate(cmName, ns, &corev1.ConfigMap{}, - testpredicates.HasExactlyLabelKeys(metadata.ManagedByKey, metadata.DeclaredVersionLabel)) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cmName, ns, &corev1.ConfigMap{}, + testpredicates.HasExactlyLabelKeys(defaultLabels...))) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, nsObj) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, cm) diff --git a/pkg/applier/applier_test.go b/pkg/applier/applier_test.go index 209cc1a465..0e59343fc3 100644 --- a/pkg/applier/applier_test.go +++ b/pkg/applier/applier_test.go @@ -98,10 +98,11 @@ func TestApply(t *testing.T) { "example-to-not-delete": "anything", }) abandonObj.SetLabels(map[string]string{ - metadata.ManagedByKey: metadata.ManagedByValue, - metadata.SystemLabel: "anything", - metadata.ArchLabel: "anything", - "example-to-not-delete": "anything", + metadata.ManagedByKey: metadata.ManagedByValue, + metadata.ParentPackageIDLabel: "anything", + metadata.SystemLabel: "anything", + metadata.ArchLabel: "anything", + "example-to-not-delete": "anything", }) abandonObjID := core.IDOf(abandonObj) diff --git a/pkg/applier/clientset.go b/pkg/applier/clientset.go index f46194c055..d69336bdbc 100644 --- a/pkg/applier/clientset.go +++ b/pkg/applier/clientset.go @@ -19,12 +19,15 @@ import ( "github.com/GoogleContainerTools/kpt/pkg/live" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/labels" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/klog/v2" "k8s.io/kubectl/pkg/cmd/util" + "kpt.dev/configsync/pkg/metadata" "sigs.k8s.io/cli-utils/pkg/apply" "sigs.k8s.io/cli-utils/pkg/apply/event" "sigs.k8s.io/cli-utils/pkg/inventory" + "sigs.k8s.io/cli-utils/pkg/kstatus/watcher" "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -53,7 +56,7 @@ type ClientSet struct { } // NewClientSet constructs a new ClientSet. -func NewClientSet(c client.Client, configFlags *genericclioptions.ConfigFlags, statusMode string) (*ClientSet, error) { +func NewClientSet(c client.Client, configFlags *genericclioptions.ConfigFlags, statusMode, packageID string) (*ClientSet, error) { matchVersionKubeConfigFlags := util.NewMatchVersionFlags(configFlags) f := util.NewFactory(matchVersionKubeConfigFlags) @@ -71,9 +74,19 @@ func NewClientSet(c client.Client, configFlags *genericclioptions.ConfigFlags, s return nil, err } + // Only watch objects applied by this reconciler for status updates. + // This reduces both the number of events processed and the memory used by + // the informer cache. + watchFilters := &watcher.Filters{ + Labels: labels.Set{ + metadata.ParentPackageIDLabel: packageID, + }.AsSelector(), + } + applier, err := apply.NewApplierBuilder(). WithInventoryClient(invClient). WithFactory(f). + WithStatusWatcherFilters(watchFilters). Build() if err != nil { return nil, err @@ -82,6 +95,7 @@ func NewClientSet(c client.Client, configFlags *genericclioptions.ConfigFlags, s destroyer, err := apply.NewDestroyerBuilder(). WithInventoryClient(invClient). WithFactory(f). + WithStatusWatcherFilters(watchFilters). Build() if err != nil { return nil, err diff --git a/pkg/metadata/labels.go b/pkg/metadata/labels.go index b2c3a2144a..1418f81b0b 100644 --- a/pkg/metadata/labels.go +++ b/pkg/metadata/labels.go @@ -61,6 +61,12 @@ const ( // the resource. Similar to the well known app.kubernetes.io/managed-by label, // but scoped to Config Sync. ConfigSyncManagedByLabel = configsync.ConfigSyncPrefix + "managed-by" + + // ParentPackageIDLabel indicates the parent package a managed object was sourced from. + ParentPackageIDLabel = configsync.ConfigSyncPrefix + "parent-package-id" + + // PackageIDLabel indicates the ID of the package represented by a RootSync or RepoSync. + PackageIDLabel = configsync.ConfigSyncPrefix + "package-id" ) // DepthSuffix is a label suffix for hierarchical namespace depth. diff --git a/pkg/metadata/metadata_test.go b/pkg/metadata/metadata_test.go index 667f730de1..dbf7448188 100644 --- a/pkg/metadata/metadata_test.go +++ b/pkg/metadata/metadata_test.go @@ -105,6 +105,12 @@ func TestHasConfigSyncMetadata(t *testing.T) { core.Label(metadata.ManagedByKey, "random-value")), want: false, }, + { + name: "An object with the `parent-package-id` label", + obj: fake.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Label(metadata.ParentPackageIDLabel, "random-value")), + want: true, + }, } for _, tc := range testcases { diff --git a/pkg/metadata/package_id.go b/pkg/metadata/package_id.go new file mode 100644 index 0000000000..c018eabc57 --- /dev/null +++ b/pkg/metadata/package_id.go @@ -0,0 +1,44 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metadata + +import ( + "fmt" + "hash/fnv" + + "k8s.io/apimachinery/pkg/util/validation" +) + +const maxLabelLength = validation.LabelValueMaxLength // 63 + +// PackageID is a label value that uniquely identifies a RootSync or RepoSync. +// PACKAGE_ID = - +// PACKAGE_ID_FULL = .. +// Design: go/config-sync-watch-filter +func PackageID(syncName, syncNamespace, syncKind string) string { + packageID := fmt.Sprintf("%s.%s.%s", syncName, syncNamespace, syncKind) + if len(packageID) <= maxLabelLength { + return packageID + } + // fnv32a has slightly better avalanche characteristics than fnv32 + hasher := fnv.New32a() + hasher.Write([]byte(fmt.Sprintf("%s.%s.%s", syncName, syncNamespace, syncKind))) + // Converting 32-bit fnv to hex results in at most 8 characters. + // Rarely it's fewer, so pad the prefix with zeros to make it consistent. + suffix := fmt.Sprintf("%08x", hasher.Sum32()) + packageIDLen := maxLabelLength - len(suffix) - 1 + packageIDShort := packageID[0 : packageIDLen-1] + return fmt.Sprintf("%s-%s", packageIDShort, suffix) +} diff --git a/pkg/metadata/package_id_test.go b/pkg/metadata/package_id_test.go new file mode 100644 index 0000000000..d5d2c676df --- /dev/null +++ b/pkg/metadata/package_id_test.go @@ -0,0 +1,125 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metadata + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/validation" + "kpt.dev/configsync/pkg/api/configsync" +) + +const ( + // maxLabelLength = validation.LabelValueMaxLength // 63 + maxNameLength = validation.DNS1123SubdomainMaxLength // 253 +) +const loremIpsum = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum." + +func TestPackageIDLabelValue(t *testing.T) { + loremIpsumValid := strings.ReplaceAll(loremIpsum, " ", "") + loremIpsumValid = strings.ReplaceAll(loremIpsumValid, ",", "") + loremIpsumValid = strings.ReplaceAll(loremIpsumValid, ".", "") + loremIpsumValid = strings.ToLower(loremIpsumValid) + loremIpsumReverse := reverse(loremIpsumValid) + + type args struct { + syncName string + syncNamespace string + syncKind string + } + tests := []struct { + name string + in args + out string + }{ + { + // As long as possible + name: "253 char name & 63 char namespace", + in: args{ + syncName: loremIpsumValid[0 : maxNameLength-1], + syncNamespace: loremIpsumReverse[0 : maxLabelLength-1], + syncKind: configsync.RootSyncKind, + }, + out: "loremipsumdolorsitametconsecteturadipiscingelitseddoe-9758dfab", + }, + { + // Padded checksum + name: "63 char name & 63 char namespace", + in: args{ + syncName: loremIpsumValid[0 : maxLabelLength-1], + syncNamespace: loremIpsumReverse[0 : maxLabelLength-1], + syncKind: configsync.RootSyncKind, + }, + out: "loremipsumdolorsitametconsecteturadipiscingelitseddoe-0e8e8264", + }, + { + // Trailing dot + name: "53 char name & 63 char namespace", + in: args{ + syncName: loremIpsumValid[0:52], + syncNamespace: loremIpsumReverse[0 : maxLabelLength-1], + syncKind: configsync.RootSyncKind, + }, + out: "loremipsumdolorsitametconsecteturadipiscingelitseddo.-ea053540", + }, + { + // Just a little too long + name: "30 char name & 30 char namespace", + in: args{ + syncName: loremIpsumValid[0:29], + syncNamespace: loremIpsumReverse[0:29], + syncKind: configsync.RepoSyncKind, + }, + out: "loremipsumdolorsitametconsect.murobaltsediminatillomt-ff884db0", + }, + { + // Short enough to not hash + name: "10 char name & 10 char namespace", + in: args{ + syncName: loremIpsumValid[0:9], + syncNamespace: loremIpsumReverse[0:9], + syncKind: configsync.RepoSyncKind, + }, + out: "loremipsu.murobalts.RepoSync", + }, + { + // As short as possible + name: "1 char name & 1 char namespace", + in: args{ + syncName: loremIpsumValid[0:1], + syncNamespace: loremIpsumReverse[0:1], + syncKind: configsync.RepoSyncKind, + }, + out: "l.m.RepoSync", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := PackageID(tt.in.syncName, tt.in.syncNamespace, tt.in.syncKind) + assert.Equal(t, tt.out, out) + assert.LessOrEqual(t, len(out), 63) + }) + } +} + +func reverse(s string) string { + runes := []rune(s) + for i, j := 0, len(runes)-1; i < j; i, j = i+1, j-1 { + runes[i], runes[j] = runes[j], runes[i] + } + return string(runes) +} diff --git a/pkg/parse/annotations.go b/pkg/parse/annotations.go index a0865011f8..ceab1e6c71 100644 --- a/pkg/parse/annotations.go +++ b/pkg/parse/annotations.go @@ -37,10 +37,12 @@ func addAnnotationsAndLabels(objs []ast.FileObject, scope declared.Scope, syncNa if err != nil { return fmt.Errorf("marshaling sourceContext: %w", err) } + packageID := metadata.PackageID(syncName, scope.SyncNamespace(), scope.SyncKind()) inventoryID := applier.InventoryID(syncName, scope.SyncNamespace()) manager := declared.ResourceManager(scope, syncName) for _, obj := range objs { core.SetLabel(obj, metadata.ManagedByKey, metadata.ManagedByValue) + core.SetLabel(obj, metadata.ParentPackageIDLabel, packageID) core.SetAnnotation(obj, metadata.GitContextKey, string(gcVal)) core.SetAnnotation(obj, metadata.ResourceManagerKey, manager) core.SetAnnotation(obj, metadata.SyncTokenAnnotationKey, commitHash) diff --git a/pkg/parse/annotations_test.go b/pkg/parse/annotations_test.go index fc92985267..c21cd621b7 100644 --- a/pkg/parse/annotations_test.go +++ b/pkg/parse/annotations_test.go @@ -18,14 +18,21 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/applier" "kpt.dev/configsync/pkg/core" + "kpt.dev/configsync/pkg/declared" "kpt.dev/configsync/pkg/importer/analyzer/ast" "kpt.dev/configsync/pkg/metadata" "kpt.dev/configsync/pkg/testing/fake" ) func TestAddAnnotationsAndLabels(t *testing.T) { + syncKind := configsync.RepoSyncKind + syncName := "rs" + syncNamespace := "some-namespace" + syncScope := declared.ScopeFromSyncNamespace(syncNamespace) + testcases := []struct { name string actual []ast.FileObject @@ -50,11 +57,12 @@ func TestAddAnnotationsAndLabels(t *testing.T) { expected: []ast.FileObject{fake.Role( core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(syncName, syncNamespace, syncKind)), core.Annotation(metadata.ResourceManagementKey, "enabled"), core.Annotation(metadata.ResourceManagerKey, "some-namespace_rs"), core.Annotation(metadata.SyncTokenAnnotationKey, "1234567"), core.Annotation(metadata.GitContextKey, `{"repo":"git@github.com/foo","branch":"main","rev":"HEAD"}`), - core.Annotation(metadata.OwningInventoryKey, applier.InventoryID("rs", "some-namespace")), + core.Annotation(metadata.OwningInventoryKey, applier.InventoryID(syncName, syncNamespace)), core.Annotation(metadata.ResourceIDKey, "rbac.authorization.k8s.io_role_foo_default-name"), )}, }, @@ -62,7 +70,7 @@ func TestAddAnnotationsAndLabels(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - if err := addAnnotationsAndLabels(tc.actual, "some-namespace", "rs", tc.gc, tc.commitHash); err != nil { + if err := addAnnotationsAndLabels(tc.actual, syncScope, syncName, tc.gc, tc.commitHash); err != nil { t.Fatalf("Failed to add annotations and labels: %v", err) } if diff := cmp.Diff(tc.expected, tc.actual, ast.CompareFileObject); diff != "" { diff --git a/pkg/parse/root_test.go b/pkg/parse/root_test.go index 95b54a51b9..c8d6fa86fa 100644 --- a/pkg/parse/root_test.go +++ b/pkg/parse/root_test.go @@ -87,6 +87,8 @@ func TestRoot_Parse(t *testing.T) { fakeTime := fakeMetaTime.Time fakeClock := fakeclock.NewFakeClock(fakeTime) + packageID := metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind) + // TODO: Test different source types and inputs fileSource := FileSource{ SourceType: configsync.GitSource, @@ -262,6 +264,7 @@ func TestRoot_Parse(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -274,6 +277,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -303,6 +307,7 @@ func TestRoot_Parse(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -321,6 +326,7 @@ func TestRoot_Parse(t *testing.T) { existingObjects: []client.Object{ fake.NamespaceObject("foo", core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -341,6 +347,7 @@ func TestRoot_Parse(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -353,6 +360,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -370,11 +378,12 @@ func TestRoot_Parse(t *testing.T) { namespaceStrategy: configsync.NamespaceStrategyImplicit, existingObjects: []client.Object{fake.NamespaceObject("foo", core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID("other-root-sync", configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), core.Annotation(metadata.SyncTokenAnnotationKey, testGitCommit), - core.Annotation(metadata.OwningInventoryKey, applier.InventoryID(rootSyncName, configmanagement.ControllerNamespace)), + core.Annotation(metadata.OwningInventoryKey, applier.InventoryID("other-root-sync", configmanagement.ControllerNamespace)), core.Annotation(metadata.ResourceIDKey, "_namespace_foo"), difftest.ManagedBy(declared.RootScope, "other-root-sync")), rootSyncInput.DeepCopy(), @@ -390,6 +399,7 @@ func TestRoot_Parse(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -420,6 +430,7 @@ func TestRoot_Parse(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -450,6 +461,7 @@ func TestRoot_Parse(t *testing.T) { fake.WithRootSyncSourceType(configsync.GitSource), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1beta1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, fmt.Sprintf("namespaces/%s/test.yaml", configsync.ControllerNamespace)), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -480,6 +492,7 @@ func TestRoot_Parse(t *testing.T) { fake.Role(core.Namespace("bar"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -491,6 +504,7 @@ func TestRoot_Parse(t *testing.T) { fake.ConfigMap(core.Namespace("bar"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/configmap.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -503,6 +517,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("bar"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -523,6 +538,7 @@ func TestRoot_Parse(t *testing.T) { // bar not exists, should be added as an implicit namespace fake.NamespaceObject("baz", // baz exists and self-managed, should be added as an implicit namespace core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -547,6 +563,7 @@ func TestRoot_Parse(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -558,6 +575,7 @@ func TestRoot_Parse(t *testing.T) { fake.Role(core.Namespace("bar"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -569,6 +587,7 @@ func TestRoot_Parse(t *testing.T) { fake.ConfigMap(core.Namespace("bar"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/configmap.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -580,6 +599,7 @@ func TestRoot_Parse(t *testing.T) { fake.Role(core.Namespace("baz"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -591,6 +611,7 @@ func TestRoot_Parse(t *testing.T) { fake.ConfigMap(core.Namespace("baz"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/configmap.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -603,6 +624,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("bar"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -615,6 +637,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("baz"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, gitContextOutput), @@ -729,6 +752,8 @@ func TestRoot_Parse(t *testing.T) { } func TestRoot_DeclaredFields(t *testing.T) { + packageID := metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind) + testCases := []struct { name string webhookEnabled bool @@ -741,6 +766,7 @@ func TestRoot_DeclaredFields(t *testing.T) { webhookEnabled: false, existingObjects: []client.Object{fake.NamespaceObject("foo", core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.DeclaredFieldsKey, `{"f:metadata":{"f:annotations":{},"f:labels":{}},"f:rules":{}}`), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -755,6 +781,7 @@ func TestRoot_DeclaredFields(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -770,6 +797,7 @@ func TestRoot_DeclaredFields(t *testing.T) { webhookEnabled: true, existingObjects: []client.Object{fake.NamespaceObject("foo", core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.DeclaredFieldsKey, `{"f:metadata":{"f:annotations":{},"f:labels":{}},"f:rules":{}}`), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -785,6 +813,7 @@ func TestRoot_DeclaredFields(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.DeclaredFieldsKey, `{"f:metadata":{"f:annotations":{"f:configmanagement.gke.io/source-path":{}},"f:labels":{"f:configsync.gke.io/declared-version":{}}},"f:rules":{}}`), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), @@ -802,6 +831,7 @@ func TestRoot_DeclaredFields(t *testing.T) { existingObjects: []client.Object{ fake.NamespaceObject("foo", core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), core.Annotation(metadata.SyncTokenAnnotationKey, ""), @@ -817,6 +847,7 @@ func TestRoot_DeclaredFields(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.DeclaredFieldsKey, `{"f:metadata":{"f:annotations":{"f:configmanagement.gke.io/source-path":{}},"f:labels":{"f:configsync.gke.io/declared-version":{}}},"f:rules":{}}`), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), @@ -834,6 +865,7 @@ func TestRoot_DeclaredFields(t *testing.T) { existingObjects: []client.Object{ fake.NamespaceObject("foo", core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), core.Annotation(metadata.SyncTokenAnnotationKey, ""), @@ -848,6 +880,7 @@ func TestRoot_DeclaredFields(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -982,6 +1015,8 @@ func fakeParseError(err error, gvks ...schema.GroupVersionKind) status.MultiErro } func TestRoot_Parse_Discovery(t *testing.T) { + packageID := metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind) + testCases := []struct { name string parsed []ast.FileObject @@ -1038,6 +1073,7 @@ func TestRoot_Parse_Discovery(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -1050,6 +1086,7 @@ func TestRoot_Parse_Discovery(t *testing.T) { "", core.Name("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -1074,6 +1111,7 @@ func TestRoot_Parse_Discovery(t *testing.T) { fakeCRD(core.Name("anvils.acme.com"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "cluster/crd.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -1084,6 +1122,7 @@ func TestRoot_Parse_Discovery(t *testing.T) { fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -1097,6 +1136,7 @@ func TestRoot_Parse_Discovery(t *testing.T) { core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1"), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(metadata.ResourceManagerKey, ":root_my-rs"), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/obj.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), @@ -1109,6 +1149,7 @@ func TestRoot_Parse_Discovery(t *testing.T) { "", core.Name("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, packageID), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 6e6d55c0b5..1c6084219e 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -32,6 +32,7 @@ import ( "kpt.dev/configsync/pkg/importer/filesystem" "kpt.dev/configsync/pkg/importer/filesystem/cmpath" "kpt.dev/configsync/pkg/importer/reader" + "kpt.dev/configsync/pkg/metadata" "kpt.dev/configsync/pkg/parse" "kpt.dev/configsync/pkg/reconciler/finalizer" "kpt.dev/configsync/pkg/reconciler/namespacecontroller" @@ -195,7 +196,8 @@ func Run(opts Options) { if reconcileTimeout < 0 { klog.Fatalf("Invalid reconcileTimeout: %v, timeout should not be negative", reconcileTimeout) } - clientSet, err := applier.NewClientSet(cl, configFlags, opts.StatusMode) + packageID := metadata.PackageID(opts.SyncName, opts.ReconcilerScope.SyncNamespace(), opts.ReconcilerScope.SyncKind()) + clientSet, err := applier.NewClientSet(cl, configFlags, opts.StatusMode, packageID) if err != nil { klog.Fatalf("Error creating clients: %v", err) } diff --git a/pkg/reconcilermanager/controllers/reconciler_base.go b/pkg/reconcilermanager/controllers/reconciler_base.go index b117bf4d54..16e5a88e10 100644 --- a/pkg/reconcilermanager/controllers/reconciler_base.go +++ b/pkg/reconcilermanager/controllers/reconciler_base.go @@ -711,3 +711,28 @@ func (r *reconcilerBase) isWebhookEnabled(ctx context.Context) (bool, error) { } return err == nil, err } + +func (r *reconcilerBase) patchSyncMetadata(ctx context.Context, rs client.Object) (bool, error) { + packageID := metadata.PackageID(rs.GetName(), rs.GetNamespace(), r.syncKind) + currentPackageID := core.GetLabel(rs, metadata.PackageIDLabel) + if currentPackageID == packageID { + r.logger(ctx).V(5).Info("Sync metadata update skipped: no change") + return false, nil + } + + if r.logger(ctx).V(5).Enabled() { + r.logger(ctx).Info("Updating sync metadata: package ID label", + logFieldResourceVersion, rs.GetResourceVersion(), + "labelKey", metadata.PackageIDLabel, + "labelValueOld", currentPackageID, "labelValue", packageID) + } + + patch := fmt.Sprintf(`{"metadata": {"labels": {%q: %q}}}`, metadata.PackageIDLabel, packageID) + err := r.client.Patch(ctx, rs, client.RawPatch(types.MergePatchType, []byte(patch)), + client.FieldOwner(reconcilermanager.FieldManager)) + if err != nil { + return true, fmt.Errorf("Sync metadata update failed: %w", err) + } + r.logger(ctx).Info("Sync metadata update successful") + return true, nil +} diff --git a/pkg/reconcilermanager/controllers/reposync_controller.go b/pkg/reconcilermanager/controllers/reposync_controller.go index 03151565f4..bf2f7da405 100644 --- a/pkg/reconcilermanager/controllers/reposync_controller.go +++ b/pkg/reconcilermanager/controllers/reposync_controller.go @@ -320,11 +320,15 @@ func (r *RepoSyncReconciler) upsertManagedObjects(ctx context.Context, reconcile } // setup performs the following steps: +// - Patch RepoSync to upsert package-id label // - Create or update managed objects // - Convert any error into RepoSync status conditions // - Update the RepoSync status func (r *RepoSyncReconciler) setup(ctx context.Context, reconcilerRef types.NamespacedName, rs *v1beta1.RepoSync) error { - err := r.upsertManagedObjects(ctx, reconcilerRef, rs) + _, err := r.patchSyncMetadata(ctx, rs) + if err == nil { + err = r.upsertManagedObjects(ctx, reconcilerRef, rs) + } updated, updateErr := r.updateSyncStatus(ctx, rs, reconcilerRef, func(syncObj *v1beta1.RepoSync) error { // Modify the sync status, // but keep the upsert error separate from the status update error. diff --git a/pkg/reconcilermanager/controllers/rootsync_controller.go b/pkg/reconcilermanager/controllers/rootsync_controller.go index 769a0c40b0..10d3a7b700 100644 --- a/pkg/reconcilermanager/controllers/rootsync_controller.go +++ b/pkg/reconcilermanager/controllers/rootsync_controller.go @@ -269,11 +269,15 @@ func (r *RootSyncReconciler) upsertManagedObjects(ctx context.Context, reconcile } // setup performs the following steps: +// - Patch RootSync to upsert package-id label // - Create or update managed objects // - Convert any error into RootSync status conditions // - Update the RootSync status func (r *RootSyncReconciler) setup(ctx context.Context, reconcilerRef types.NamespacedName, rs *v1beta1.RootSync) error { - err := r.upsertManagedObjects(ctx, reconcilerRef, rs) + _, err := r.patchSyncMetadata(ctx, rs) + if err == nil { + err = r.upsertManagedObjects(ctx, reconcilerRef, rs) + } updated, updateErr := r.updateSyncStatus(ctx, rs, reconcilerRef, func(syncObj *v1beta1.RootSync) error { // Modify the sync status, // but keep the upsert error separate from the status update error. diff --git a/pkg/remediator/watch/filteredwatcher_test.go b/pkg/remediator/watch/filteredwatcher_test.go index 4a36927237..f821ed87cd 100644 --- a/pkg/remediator/watch/filteredwatcher_test.go +++ b/pkg/remediator/watch/filteredwatcher_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/require" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" "k8s.io/utils/ptr" @@ -346,6 +347,7 @@ func TestFilteredWatcher(t *testing.T) { return <-watches, nil }, conflictHandler: testfake.NewConflictHandler(), + labelSelector: labels.Everything(), } w := NewFiltered(cfg) diff --git a/pkg/remediator/watch/manager.go b/pkg/remediator/watch/manager.go index 6c126672f0..e976464350 100644 --- a/pkg/remediator/watch/manager.go +++ b/pkg/remediator/watch/manager.go @@ -19,10 +19,12 @@ import ( "errors" "sync" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" "k8s.io/klog/v2" "kpt.dev/configsync/pkg/declared" + "kpt.dev/configsync/pkg/metadata" "kpt.dev/configsync/pkg/remediator/conflict" "kpt.dev/configsync/pkg/remediator/queue" "kpt.dev/configsync/pkg/status" @@ -49,6 +51,9 @@ type Manager struct { // watcherFactory is the function to create a watcher. watcherFactory watcherFactory + // labelSelector filters watches + labelSelector labels.Selector + // The following fields are guarded by the mutex. mux sync.RWMutex // watching is true if UpdateWatches has been called @@ -89,6 +94,12 @@ func NewManager(scope declared.Scope, syncName string, cfg *rest.Config, } } + // Only watch & remediate objects applied by this reconciler + packageID := metadata.PackageID(syncName, scope.SyncNamespace(), scope.SyncKind()) + labelSelector := labels.Set{ + metadata.ParentPackageIDLabel: packageID, + }.AsSelector() + return &Manager{ scope: scope, syncName: syncName, @@ -96,6 +107,7 @@ func NewManager(scope declared.Scope, syncName string, cfg *rest.Config, resources: decls, watcherMap: make(map[schema.GroupVersionKind]Runnable), watcherFactory: options.watcherFactory, + labelSelector: labelSelector, queue: q, conflictHandler: ch, }, nil @@ -206,6 +218,7 @@ func (m *Manager) startWatcher(ctx context.Context, gvk schema.GroupVersionKind) scope: m.scope, syncName: m.syncName, conflictHandler: m.conflictHandler, + labelSelector: m.labelSelector, } w, err := m.watcherFactory(cfg) if err != nil { diff --git a/pkg/remediator/watch/manager_test.go b/pkg/remediator/watch/manager_test.go index b3a5366311..4c9741cdc0 100644 --- a/pkg/remediator/watch/manager_test.go +++ b/pkg/remediator/watch/manager_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/watch" "kpt.dev/configsync/pkg/declared" @@ -38,6 +39,7 @@ func fakeRunnable() Runnable { return watch.NewFake(), nil }, conflictHandler: fake.NewConflictHandler(), + labelSelector: labels.Everything(), } return NewFiltered(cfg) } diff --git a/pkg/remediator/watch/watcher.go b/pkg/remediator/watch/watcher.go index 310c7d8fb6..754f5572a4 100644 --- a/pkg/remediator/watch/watcher.go +++ b/pkg/remediator/watch/watcher.go @@ -18,6 +18,7 @@ import ( "context" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/rest" @@ -38,6 +39,7 @@ type watcherConfig struct { syncName string startWatch WatchFunc conflictHandler conflict.Handler + labelSelector labels.Selector } // watcherFactory knows how to build watch.Runnables. @@ -56,6 +58,9 @@ func watcherFactoryFromListerWatcherFactory(factory ListerWatcherFactory) watche namespace = string(cfg.scope) } lw := factoryPtr(cfg.gvk, namespace) + if cfg.labelSelector != nil { + options.LabelSelector = cfg.labelSelector.String() + } return ListAndWatch(ctx, lw, options) } }