From 3b80a65caa2db5f49b00f710a6de6b65aeebab2d Mon Sep 17 00:00:00 2001 From: Karl Isenberg Date: Fri, 29 Mar 2024 12:31:51 -0700 Subject: [PATCH] [WIP] Prototype autoscaling by object count - Add .status.source.objectCount to track number of objects found after parsing from source (after hydration, if any). - Add .status.sync.objectCount to track number of objects in the inventory ResourceGroup's .spec.resources. - Add resource doubling for the reconciler container when the object count (greater of sync & source) is at least 1,000 (autopilot only). - Reduce the default resources for the reconciler on autopilot Notes: - Since the applier doesn't send the inventory size over events, we have to query the ResourceGroup after each inventory update event. This could be made more efficient with changes to the applier in cli-utils, but isn't really a big deal. - Autoscaling is currently very trivial and could be fine tuned, but this is just a proof of concept to show an MVP and test that it would work on our tests. Filter remediator watch by labels Add watch label selectors - Add config.k8s.io/owning-inventory as a label to all managed objects - Add label selectors to the watchers used by the remediator and applier - Modify cli-utils to allow optional label selector injection Add more complex autoscaling math Revert metrics expectation changes Fix e2e test with hard coded label expectations WIP resolve test issues - Update TestConflictingDefinitions_NamespaceToRoot to expect a management conflict from the remediator, and for the remediator's conflict to replace the applier's conflict. - De-dupe management conflict errors from remediator & applier. - Fix a bug in the remediator that was dropping management conflict errors when using a watch filter. With the filter, changing the filter labels causes a DELETE event, instead of an UPDATE event, like it did before. The only way to double-check is to query the cluster, and we don't want to do that in the watch handler for performance reasons. So instead, update the remediator queue worker to query the cluster to confirm object deletion. Then also update the worker to be able to add management conflict errors to the same conflictHandler used by the watcher. - Add HasConflictErrors() to the conflictHandler to replace the locked managementConflict bool in the watch.Manager. This will cause the retry run loop to re-apply if there's still a conflict. Is this desired behavior? Or do we only want to re-apply the first time? If so, we may need to cache the errors in the run loop so we can detect when new conflicts are added. Or maybe just expose the whole conflictHandler. - Clean up parse.Run a little. Add resetRetryBackoff method to avoid leaking implementation details to the rest of the parser. - Remove obsolete retry setting, since it wasn't exposed and was previously replaced with a backoff strategy. - Fix the reconciler container CPU calculation to use milli-cpu. - Add ConflictingObjectID to ManagementConflictError to allow de-duping by ID. - Fix KptManagementConflictError to return a ManagementConflictError, not just a ResourceError. The currentManager will be UNKNOWN, but it only seems to be used by reportRootSyncConflicts, which only reports conflicts from the remediator, not the applier. Reduce memory/cpu step size to 128Mi:125m Reduce memory step size to 32Mi This allows scaling down to 256Mi total reconciler Pod memory, not just 512Mi. However, on non-bursting autopilot clusters, the minimum is still 512Mi. Avoid invalid periodic sync status updates Skip sync status updates when the source cache is reset, otherwise the commit that was previously synced will be removed. Refactor Applier to expose an event handler - Use the new event handler to send errors and inventory updates to the caller (Updater) so it can track them without locks in the applier. - Update the Updater to cache the sync status with a shared lock. - Add initialization to the Updater to refresh the sync status using the RootSync/RepoSync status. This should avoid losing status fields after the reconciler is restarted or rescheduled. Unfortunately, this doesn't work for errors, because they can't be parsed back into typed errors. But this shouldn't be a real problem because the applier was invalidating them before each apply anyway. - Move periodic sync status updates into the Updater. - Add a baseFinalizer class to handle collecting destroyer errors. Double memory per object Even though the memory limit seems higher than usage, the reconciler is still being killed by the kernel because the node is out of memory on autopilot. So we need to be a little more defensive about requesting more memory than we need in order to avoid OOMKills on crowded nodes. Add TestStressProfileResourcesByObjectCount Fix e2e tests that revert manual changes In order for drift correction to work now, the object needs the correct owning-inventory label used by the watch filter. Without the label, drift will not be reverted. Add TestStressProfileResourcesByObjectCountWithMultiSync Fix waiting for deployments Use configsync.gke.io/parent-package-id as filter - Filter label key: configsync.gke.io/parent-package-id - PACKAGE_ID: - - PACKAGE_ID_FULL: .. - Pre-pad the hex of the hash to 8 characters for consistent length Add configsync.gke.io/package-id label to RSyncs - Use reconciler-manager field manager when updating objects, to distinguish from the reconciler. Otherwise SSA from a parent reconciler managing an RSync will delete any metadata updates made by the reconciler-manager. Fix rebase errors Fix labels in TestKubectl tests Use latest cli-utils watch filters --- cmd/reconciler/main.go | 4 +- e2e/nomostest/prometheus_metrics.go | 79 +++- e2e/testcases/custom_resources_test.go | 14 +- e2e/testcases/managed_resources_test.go | 48 ++- e2e/testcases/multi_sync_test.go | 111 +++-- e2e/testcases/preserve_fields_test.go | 16 +- e2e/testcases/stress_test.go | 380 +++++++++++++++++- go.mod | 3 +- go.sum | 9 +- manifests/reposync-crd.yaml | 16 + manifests/rootsync-crd.yaml | 16 + pkg/api/configsync/register.go | 7 +- pkg/api/configsync/v1alpha1/sync_types.go | 9 + .../v1alpha1/zz_generated.conversion.go | 4 + pkg/api/configsync/v1beta1/sync_types.go | 9 + pkg/applier/applier.go | 239 ++++++----- pkg/applier/applier_test.go | 25 +- pkg/applier/clientset.go | 23 +- pkg/applier/constants.go | 13 +- pkg/applier/destroyer_test.go | 14 +- pkg/applier/fake/applier.go | 31 +- pkg/applier/management_conflict_err.go | 9 +- pkg/applier/utils.go | 15 +- pkg/core/scheme.go | 2 + pkg/declared/scope.go | 26 ++ pkg/diff/precedence.go | 10 +- pkg/metadata/labels.go | 6 + pkg/metadata/metadata_test.go | 6 + pkg/metadata/package_id.go | 30 ++ pkg/metadata/package_id_test.go | 111 +++++ pkg/parse/annotations.go | 12 +- pkg/parse/annotations_test.go | 12 +- pkg/parse/namespace.go | 46 ++- pkg/parse/opts.go | 17 +- pkg/parse/root.go | 105 ++++- pkg/parse/root_test.go | 103 +++-- pkg/parse/run.go | 153 ++----- pkg/parse/run_test.go | 4 +- pkg/parse/source_test.go | 8 +- pkg/parse/state.go | 20 +- pkg/parse/updater.go | 307 ++++++++++++-- pkg/reconciler/finalizer/base_finalizer.go | 46 +++ pkg/reconciler/finalizer/finalizer.go | 8 +- .../finalizer/reposync_finalizer.go | 19 +- .../finalizer/reposync_finalizer_test.go | 4 +- .../finalizer/rootsync_finalizer.go | 19 +- .../finalizer/rootsync_finalizer_test.go | 45 ++- pkg/reconciler/reconciler.go | 20 +- pkg/reconcilermanager/constants.go | 6 + .../controllers/reconciler_base.go | 25 ++ .../reconciler_container_resources.go | 8 +- .../controllers/reposync_controller.go | 40 +- .../controllers/rootsync_controller.go | 40 +- pkg/reconcilermanager/controllers/util.go | 4 +- pkg/remediator/conflict/handler.go | 11 + pkg/remediator/reconcile/reconciler.go | 38 +- pkg/remediator/reconcile/reconciler_test.go | 6 +- pkg/remediator/reconcile/worker.go | 76 ++-- pkg/remediator/reconcile/worker_test.go | 15 +- pkg/remediator/remediator.go | 4 +- pkg/remediator/watch/filteredwatcher.go | 87 ++-- pkg/remediator/watch/filteredwatcher_test.go | 2 + pkg/remediator/watch/manager.go | 43 +- pkg/remediator/watch/manager_test.go | 2 + pkg/remediator/watch/watcher.go | 2 + pkg/status/management_conflict_error.go | 7 + .../syncertest/fake/conflict_handler.go | 5 + pkg/syncer/syncertest/fake/storage.go | 13 +- pkg/util/bytes/bytes.go | 30 ++ pkg/util/mutate/mutate.go | 30 +- .../github.com/wk8/go-ordered-map/.gitignore | 1 + .../wk8/go-ordered-map/.golangci.yml | 85 ++++ vendor/github.com/wk8/go-ordered-map/LICENSE | 201 +++++++++ vendor/github.com/wk8/go-ordered-map/Makefile | 13 + .../github.com/wk8/go-ordered-map/README.md | 104 +++++ .../wk8/go-ordered-map/orderedmap.go | 193 +++++++++ vendor/modules.txt | 5 +- .../cli-utils/pkg/apply/applier_builder.go | 5 + .../cli-utils/pkg/apply/builder.go | 11 +- .../cli-utils/pkg/apply/destroyer_builder.go | 5 + .../kstatus/watcher/default_status_watcher.go | 30 +- .../watcher/dynamic_informer_factory.go | 135 ++++++- 82 files changed, 2830 insertions(+), 685 deletions(-) create mode 100644 pkg/metadata/package_id.go create mode 100644 pkg/metadata/package_id_test.go create mode 100644 pkg/reconciler/finalizer/base_finalizer.go create mode 100644 pkg/util/bytes/bytes.go create mode 100644 vendor/github.com/wk8/go-ordered-map/.gitignore create mode 100644 vendor/github.com/wk8/go-ordered-map/.golangci.yml create mode 100644 vendor/github.com/wk8/go-ordered-map/LICENSE create mode 100644 vendor/github.com/wk8/go-ordered-map/Makefile create mode 100644 vendor/github.com/wk8/go-ordered-map/README.md create mode 100644 vendor/github.com/wk8/go-ordered-map/orderedmap.go diff --git a/cmd/reconciler/main.go b/cmd/reconciler/main.go index 23ffb84935..d50b5bfa41 100644 --- a/cmd/reconciler/main.go +++ b/cmd/reconciler/main.go @@ -24,6 +24,7 @@ import ( "k8s.io/klog/v2/textlogger" "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/api/configsync/v1beta1" + "kpt.dev/configsync/pkg/applier" "kpt.dev/configsync/pkg/declared" "kpt.dev/configsync/pkg/importer/filesystem" "kpt.dev/configsync/pkg/importer/filesystem/cmpath" @@ -180,7 +181,6 @@ func main() { ReconcilerScope: declared.Scope(*scope), ResyncPeriod: *resyncPeriod, PollingPeriod: *pollingPeriod, - RetryPeriod: configsync.DefaultReconcilerRetryPeriod, StatusUpdatePeriod: configsync.DefaultReconcilerSyncStatusUpdatePeriod, SourceRoot: absSourceDir, RepoRoot: absRepoRoot, @@ -193,7 +193,7 @@ func main() { SyncDir: relSyncDir, SyncName: *syncName, ReconcilerName: *reconcilerName, - StatusMode: *statusMode, + StatusMode: applier.InventoryStatusMode(*statusMode), ReconcileTimeout: *reconcileTimeout, APIServerTimeout: *apiServerTimeout, RenderingEnabled: *renderingEnabled, diff --git a/e2e/nomostest/prometheus_metrics.go b/e2e/nomostest/prometheus_metrics.go index 36bf9b938f..8c0ff1b8f9 100644 --- a/e2e/nomostest/prometheus_metrics.go +++ b/e2e/nomostest/prometheus_metrics.go @@ -256,8 +256,12 @@ func reconcilerOperationMetrics(nt *NT, syncLabels prometheusmodel.LabelSet, op return func(ctx context.Context, v1api prometheusv1.API) error { var err error err = multierr.Append(err, metricAPICallDurationViewOperationHasStatus(ctx, nt, v1api, syncLabels, string(op.Operation), ocmetrics.StatusSuccess)) - err = multierr.Append(err, metricApplyOperationsViewHasValueAtLeast(ctx, nt, v1api, syncLabels, string(op.Operation), ocmetrics.StatusSuccess, op.Count)) + // err = multierr.Append(err, metricAPICallDurationViewOperationMustNotHaveStatus(ctx, nt, v1api, syncLabels, string(op.Operation), ocmetrics.StatusError)) + // err = multierr.Append(err, metricApplyOperationsViewMustNotHaveStatus(ctx, nt, v1api, syncLabels, ocmetrics.RemediatorController, string(op.Operation), ocmetrics.StatusError)) + // err = multierr.Append(err, metricApplyOperationsViewMustNotHaveStatus(ctx, nt, v1api, syncLabels, ocmetrics.ApplierController, string(op.Operation), ocmetrics.StatusError)) + err = multierr.Append(err, metricApplyOperationsViewHasValueAtLeast(ctx, nt, v1api, syncLabels, ocmetrics.ApplierController, string(op.Operation), ocmetrics.StatusSuccess, op.Count)) err = multierr.Append(err, metricRemediateDurationViewHasStatus(ctx, nt, v1api, syncLabels, ocmetrics.StatusSuccess)) + // err = multierr.Append(err, metricRemediateDurationViewMustNotHaveStatus(ctx, nt, v1api, syncLabels, ocmetrics.StatusError)) return err } } @@ -462,12 +466,25 @@ func metricAPICallDurationViewOperationHasStatus(ctx context.Context, nt *NT, v1 return metricExists(ctx, nt, v1api, query) } -func metricApplyOperationsViewHasValueAtLeast(ctx context.Context, nt *NT, v1api prometheusv1.API, syncLabels prometheusmodel.LabelSet, operation, status string, value int) error { +func metricAPICallDurationViewOperationMustNotHaveStatus(ctx context.Context, nt *NT, v1api prometheusv1.API, syncLabels prometheusmodel.LabelSet, operation, status string) error { + metricName := ocmetrics.APICallDurationView.Name + // APICallDurationView is a distribution. Query count to aggregate. + metricName = fmt.Sprintf("%s%s%s", prometheusConfigSyncMetricPrefix, metricName, prometheusDistributionCountSuffix) + labels := prometheusmodel.LabelSet{ + prometheusmodel.LabelName(ocmetrics.KeyComponent.Name()): prometheusmodel.LabelValue(ocmetrics.OtelCollectorName), + prometheusmodel.LabelName(ocmetrics.KeyOperation.Name()): prometheusmodel.LabelValue(operation), + prometheusmodel.LabelName(ocmetrics.KeyStatus.Name()): prometheusmodel.LabelValue(status), + }.Merge(syncLabels) + query := fmt.Sprintf("%s%s", metricName, labels) + return metricMustNotExist(ctx, nt, v1api, query) +} + +func metricApplyOperationsViewHasValueAtLeast(ctx context.Context, nt *NT, v1api prometheusv1.API, syncLabels prometheusmodel.LabelSet, controller, operation, status string, value int) error { metricName := ocmetrics.ApplyOperationsView.Name metricName = fmt.Sprintf("%s%s", prometheusConfigSyncMetricPrefix, metricName) labels := prometheusmodel.LabelSet{ prometheusmodel.LabelName(ocmetrics.KeyComponent.Name()): prometheusmodel.LabelValue(ocmetrics.OtelCollectorName), - prometheusmodel.LabelName(ocmetrics.KeyController.Name()): prometheusmodel.LabelValue(ocmetrics.ApplierController), + prometheusmodel.LabelName(ocmetrics.KeyController.Name()): prometheusmodel.LabelValue(controller), prometheusmodel.LabelName(ocmetrics.KeyOperation.Name()): prometheusmodel.LabelValue(operation), prometheusmodel.LabelName(ocmetrics.KeyStatus.Name()): prometheusmodel.LabelValue(status), }.Merge(syncLabels) @@ -476,6 +493,20 @@ func metricApplyOperationsViewHasValueAtLeast(ctx context.Context, nt *NT, v1api return metricExistsWithValueAtLeast(ctx, nt, v1api, query, float64(value)) } +func metricApplyOperationsViewMustNotHaveStatus(ctx context.Context, nt *NT, v1api prometheusv1.API, syncLabels prometheusmodel.LabelSet, controller, operation, status string) error { + metricName := ocmetrics.ApplyOperationsView.Name + metricName = fmt.Sprintf("%s%s", prometheusConfigSyncMetricPrefix, metricName) + labels := prometheusmodel.LabelSet{ + prometheusmodel.LabelName(ocmetrics.KeyComponent.Name()): prometheusmodel.LabelValue(ocmetrics.OtelCollectorName), + prometheusmodel.LabelName(ocmetrics.KeyController.Name()): prometheusmodel.LabelValue(controller), + prometheusmodel.LabelName(ocmetrics.KeyOperation.Name()): prometheusmodel.LabelValue(operation), + prometheusmodel.LabelName(ocmetrics.KeyStatus.Name()): prometheusmodel.LabelValue(status), + }.Merge(syncLabels) + // ApplyOperationsView is a count, so we don't need to aggregate + query := fmt.Sprintf("%s%s", metricName, labels) + return metricMustNotExist(ctx, nt, v1api, query) +} + func metricRemediateDurationViewHasStatus(ctx context.Context, nt *NT, v1api prometheusv1.API, syncLabels prometheusmodel.LabelSet, status string) error { metricName := ocmetrics.RemediateDurationView.Name // RemediateDurationView is a distribution. Query count to aggregate. @@ -488,6 +519,18 @@ func metricRemediateDurationViewHasStatus(ctx context.Context, nt *NT, v1api pro return metricExists(ctx, nt, v1api, query) } +func metricRemediateDurationViewMustNotHaveStatus(ctx context.Context, nt *NT, v1api prometheusv1.API, syncLabels prometheusmodel.LabelSet, status string) error { + metricName := ocmetrics.RemediateDurationView.Name + // RemediateDurationView is a distribution. Query count to aggregate. + metricName = fmt.Sprintf("%s%s%s", prometheusConfigSyncMetricPrefix, metricName, prometheusDistributionCountSuffix) + labels := prometheusmodel.LabelSet{ + prometheusmodel.LabelName(ocmetrics.KeyComponent.Name()): prometheusmodel.LabelValue(ocmetrics.OtelCollectorName), + prometheusmodel.LabelName(ocmetrics.KeyStatus.Name()): prometheusmodel.LabelValue(status), + }.Merge(syncLabels) + query := fmt.Sprintf("%s%s", metricName, labels) + return metricMustNotExist(ctx, nt, v1api, query) +} + // metricQueryNow performs the specified query with the default timeout. // Query is debug logged. Warnings are logged. Errors are returned. func metricQueryNow(ctx context.Context, nt *NT, v1api prometheusv1.API, query string) (prometheusmodel.Value, error) { @@ -505,6 +548,36 @@ func metricQueryNow(ctx context.Context, nt *NT, v1api prometheusv1.API, query s return response, nil } +// metricResultMustNotExist validates that the query response includes zero +// vector or matrix results. Response is debug logged. +// This function does not perform the query, just validates and logs it. +func metricResultMustNotExist(nt *NT, query string, response prometheusmodel.Value) error { + switch result := response.(type) { + case prometheusmodel.Vector: + if len(result) > 0 { + nt.Logger.Debugf("prometheus vector response:\n%s", result) + return fmt.Errorf("unexpected results from prometheus query (found %d, expected 0): %s", len(result), query) + } + return nil + case prometheusmodel.Matrix: + if len(result) > 0 { + nt.Logger.Debugf("prometheus matrix response:\n%s", result) + return fmt.Errorf("unexpected results from prometheus query (found %d, expected 0): %s", len(result), query) + } + return nil + default: + return fmt.Errorf("unsupported prometheus response: %T", response) + } +} + +func metricMustNotExist(ctx context.Context, nt *NT, v1api prometheusv1.API, query string) error { + response, err := metricQueryNow(ctx, nt, v1api, query) + if err != nil { + return err + } + return metricResultMustNotExist(nt, query, response) +} + // metricResultMustExist validates that the query response includes at least one // vector or matrix result. Response is debug logged. // This function does not perform the query, just validates and logs it. diff --git a/e2e/testcases/custom_resources_test.go b/e2e/testcases/custom_resources_test.go index d98eb72d9a..7ec419b43f 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..c62b8e7722 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 drift prevention is only supported in the multi-repo mode, and utilizes the following 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 9ebd7e164e..9be27f8280 100644 --- a/e2e/testcases/multi_sync_test.go +++ b/e2e/testcases/multi_sync_test.go @@ -284,6 +284,12 @@ func TestConflictingDefinitions_RootToNamespace(t *testing.T) { ntopts.RepoSyncPermissions(policy.RBACAdmin()), // NS Reconciler manages Roles ) + // Increase reconciler logging + rootSyncObj := fake.RootSyncObjectV1Beta1(rootSyncNN.Name) + nt.Must(nt.KubeClient.MergePatch(rootSyncObj, `{"spec": {"override": {"logLevels": [{"containerName": "reconciler", "logLevel": 5}]}}}`)) + repoSyncObj := fake.RepoSyncObjectV1Beta1(repoSyncNN.Namespace, repoSyncNN.Name) + nt.Must(nt.KubeClient.MergePatch(repoSyncObj, `{"spec": {"override": {"logLevels": [{"containerName": "reconciler", "logLevel": 5}]}}}`)) + podRoleFilePath := fmt.Sprintf("acme/namespaces/%s/pod-role.yaml", testNs) nt.T.Logf("Add a Role to root: %s", configsync.RootSyncName) roleObj := rootPodRole() @@ -395,40 +401,36 @@ func TestConflictingDefinitions_NamespaceToRoot(t *testing.T) { ntopts.RepoSyncPermissions(policy.RBACAdmin()), // NS reconciler manages Roles ) + // Increase reconciler logging + rootSyncObj := fake.RootSyncObjectV1Beta1(rootSyncNN.Name) + nt.Must(nt.KubeClient.MergePatch(rootSyncObj, `{"spec": {"override": {"logLevels": [{"containerName": "reconciler", "logLevel": 5}]}}}`)) + repoSyncObj := fake.RepoSyncObjectV1Beta1(repoSyncNN.Namespace, repoSyncNN.Name) + nt.Must(nt.KubeClient.MergePatch(repoSyncObj, `{"spec": {"override": {"logLevels": [{"containerName": "reconciler", "logLevel": 5}]}}}`)) + podRoleFilePath := fmt.Sprintf("acme/namespaces/%s/pod-role.yaml", testNs) - nt.T.Logf("Add a Role to Namespace repo: %s", configsync.RootSyncName) + nt.T.Logf("Add a Role to Namespace repo: %v", repoSyncNN) nsRoleObj := namespacePodRole() nt.Must(nt.NonRootRepos[repoSyncNN].Add(podRoleFilePath, nsRoleObj)) nt.Must(nt.NonRootRepos[repoSyncNN].CommitAndPush("declare Role")) - if err := nt.WatchForAllSyncs(); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.WatchForAllSyncs()) - err := nt.Validate("pods", testNs, &rbacv1.Role{}, + // Validate Role is managed by the RepoSync reconciler + nt.Must(nt.Validate(nsRoleObj.Name, nsRoleObj.Namespace, &rbacv1.Role{}, roleHasRules(nsRoleObj.Rules), - testpredicates.IsManagedBy(nt.Scheme, declared.Scope(repoSyncNN.Namespace), repoSyncNN.Name)) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.IsManagedBy(nt.Scheme, declared.Scope(repoSyncNN.Namespace), repoSyncNN.Name))) // Test Role managed by the RepoSync, NOT the RootSync nt.MetricsExpectations.AddObjectApply(configsync.RepoSyncKind, repoSyncNN, nsRoleObj) // Validate no errors from root reconciler. - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) // Validate no errors from namespace reconciler. - err = nomostest.ValidateStandardMetricsForRepoSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRepoSync(nt, metrics.Summary{ Sync: repoSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) nt.T.Logf("Declare a conflicting Role in the Root repo: %s", configsync.RootSyncName) rootRoleObj := rootPodRole() @@ -436,61 +438,56 @@ func TestConflictingDefinitions_NamespaceToRoot(t *testing.T) { nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("add conflicting pod role to Root")) nt.T.Logf("The RootSync should update the Role") - err = nt.WatchForAllSyncs(nomostest.RootSyncOnly()) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.WatchForAllSyncs(nomostest.RootSyncOnly())) + + // The RepoSync remediator won't try to revert a change to a managed object + // if it was adopted by a RepoSync reconciler. + // So without a new commit or pod replacement, the reconciler won't + // automatically re-sync until the ~1hr resync period expires. + // nt.T.Logf("Push a new commit with an empty file to trigger re-sync by the RepoSync reconciler") + // nt.Must(nt.NonRootRepos[repoSyncNN].AddFile("acme/empty.txt", []byte{})) + // nt.Must(nt.NonRootRepos[repoSyncNN].CommitAndPush("declare Role")) + nt.T.Logf("The RepoSync %s reports a problem since it can't sync the declaration.", testNs) nt.WaitForRepoSyncSyncError(repoSyncNN.Namespace, repoSyncNN.Name, status.ManagementConflictErrorCode, "declared in another repository", nil) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, rootRoleObj) // Validate no errors from root reconciler. - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) // 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{}, + nt.Must(nt.Validate(nsRoleObj.Name, nsRoleObj.Namespace, &rbacv1.Role{}, roleHasRules(rootPodRole().Rules), - testpredicates.IsManagedBy(nt.Scheme, declared.RootReconciler, configsync.RootSyncName)) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.IsManagedBy(nt.Scheme, declared.RootReconciler, configsync.RootSyncName))) nt.T.Logf("Remove the Role from the Namespace repo %s", repoSyncNN) nt.Must(nt.NonRootRepos[repoSyncNN].Remove(podRoleFilePath)) nt.Must(nt.NonRootRepos[repoSyncNN].CommitAndPush("remove conflicting pod role from Namespace repo")) - if err := nt.WatchForAllSyncs(); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.WatchForAllSyncs()) nt.T.Logf("Ensure the Role still matches the one in the Root repo %s", configsync.RootSyncName) - err = nt.Validate("pods", testNs, &rbacv1.Role{}, + nt.Must(nt.Validate(nsRoleObj.Name, nsRoleObj.Namespace, &rbacv1.Role{}, roleHasRules(rootPodRole().Rules), - testpredicates.IsManagedBy(nt.Scheme, declared.RootReconciler, configsync.RootSyncName)) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.IsManagedBy(nt.Scheme, declared.RootReconciler, configsync.RootSyncName))) // Test Role managed by the RootSync, NOT the RepoSync nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, rootRoleObj) @@ -498,20 +495,14 @@ func TestConflictingDefinitions_NamespaceToRoot(t *testing.T) { nt.MetricsExpectations.RemoveObject(configsync.RepoSyncKind, repoSyncNN, nsRoleObj) // Validate no errors from root reconciler. - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) // Validate no errors from namespace reconciler. - err = nomostest.ValidateStandardMetricsForRepoSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRepoSync(nt, metrics.Summary{ Sync: repoSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) } func TestConflictingDefinitions_RootToRoot(t *testing.T) { diff --git a/e2e/testcases/preserve_fields_test.go b/e2e/testcases/preserve_fields_test.go index 30c8ca13c7..cf5f4ea1cf 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,9 +354,13 @@ func TestAddUpdateDeleteLabels(t *testing.T) { nt.T.Fatal(err) } - // Checking that label is updated after syncing an update. + var updatedLabels []string + updatedLabels = append(updatedLabels, defaultLabels...) + updatedLabels = append(updatedLabels, "baz") + + // Checking that label was added after syncing. err = nt.Validate(cmName, ns, &corev1.ConfigMap{}, - testpredicates.HasExactlyLabelKeys(append(defaultLabels, "baz")...)) + testpredicates.HasExactlyLabelKeys(updatedLabels...)) if err != nil { nt.T.Fatal(err) } @@ -366,7 +374,7 @@ 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)) + testpredicates.HasExactlyLabelKeys(defaultLabels...)) if err != nil { nt.T.Fatal(err) } diff --git a/e2e/testcases/stress_test.go b/e2e/testcases/stress_test.go index aca9bf0021..5d98bdcacf 100644 --- a/e2e/testcases/stress_test.go +++ b/e2e/testcases/stress_test.go @@ -248,7 +248,7 @@ func TestStressLargeRequest(t *testing.T) { }, Override: &v1beta1.RootSyncOverrideSpec{ OverrideSpec: v1beta1.OverrideSpec{ - StatusMode: applier.StatusDisabled, + StatusMode: applier.StatusDisabled.String(), Resources: []v1beta1.ContainerResourcesSpec{reconcilerOverride}, }, }, @@ -351,7 +351,7 @@ func TestStressManyDeployments(t *testing.T) { syncPath := filepath.Join(gitproviders.DefaultSyncDir, "stress-test") ns := "stress-test-ns" - deployCount := 1000 + deployCount := 4000 nt.T.Logf("Adding a test namespace and %d deployments", deployCount) nt.Must(nt.RootRepos[configsync.RootSyncName].Add(fmt.Sprintf("%s/ns-%s.yaml", syncPath, ns), fake.NamespaceObject(ns))) @@ -418,6 +418,363 @@ spec: } } +// TestStressProfileResourcesByObjectCount loops through multiple steps. Each +// step adds and removes incrementally more Deployments. This is useful for +// profiling the resource requests, usage, and limits relative to the number of +// managed objects. +// Skipped by default, because it takes about 2 hours. +func TestStressProfileResourcesByObjectCount(t *testing.T) { + nt := nomostest.New(t, nomostesting.Reconciliation1, ntopts.Unstructured, + ntopts.StressTest, + ntopts.WithReconcileTimeout(configsync.DefaultReconcileTimeout)) + + syncPath := filepath.Join(gitproviders.DefaultSyncDir, "stress-test") + ns := "stress-test-ns" + + steps := 8 + deploysPerStep := 500 + + for step := 1; step <= steps; step++ { + deployCount := deploysPerStep * step + + nt.T.Logf("Adding a test namespace and %d deployments", deployCount) + nt.Must(nt.RootRepos[configsync.RootSyncName].Add(fmt.Sprintf("%s/ns-%s.yaml", syncPath, ns), fake.NamespaceObject(ns))) + + for i := 1; i <= deployCount; i++ { + name := fmt.Sprintf("pause-%d", i) + nt.Must(nt.RootRepos[configsync.RootSyncName].AddFile( + fmt.Sprintf("%s/namespaces/%s/deployment-%s.yaml", syncPath, ns, name), + []byte(fmt.Sprintf(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: %s + namespace: %s + labels: + app: %s + nomos-test: enabled +spec: + replicas: 1 + selector: + matchLabels: + app: %s + template: + metadata: + labels: + app: %s + spec: + containers: + - name: pause + image: registry.k8s.io/pause:3.7 + ports: + - containerPort: 80 + resources: + limits: + cpu: 250m + memory: 256Mi + requests: + cpu: 250m + memory: 256Mi +`, name, ns, name, name, name)))) + } + + nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush(fmt.Sprintf("Adding a test namespace and %d deployments", deployCount))) + + // Validate that the resources sync without the reconciler running out of + // memory, getting OOMKilled, and crash looping. + if err := nt.WatchForAllSyncs(); err != nil { + nt.T.Fatal(err) + } + + nt.T.Logf("Verify the number of Deployment objects") + validateNumberOfObjectsEquals(nt, kinds.Deployment(), deployCount, + client.MatchingLabels{metadata.ManagedByKey: metadata.ManagedByValue}, + client.InNamespace(ns)) + + nt.T.Log("Removing resources from Git") + nt.Must(nt.RootRepos[configsync.RootSyncName].Remove(syncPath)) + nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("Removing resources from Git")) + + // Validate that the resources sync without the reconciler running out of + // memory, getting OOMKilled, and crash looping. + if err := nt.WatchForAllSyncs(); err != nil { + nt.T.Fatal(err) + } + } +} + +// TestStressProfileResourcesByObjectCount loops through multiple steps. Each +// step provisions multiple RootSyncs, each with many Deployments applied and +// then pruned. Each subsequent step manages more and more Deployments. +// This is useful for profiling the resource requests, usage, and limits +// relative to the number of managed objects. +// Skipped by default, because it takes about 2 hours. +func TestStressProfileResourcesByObjectCountWithMultiSync(t *testing.T) { + nt := nomostest.New(t, nomostesting.Reconciliation1, ntopts.Unstructured, + ntopts.StressTest, + ntopts.WithReconcileTimeout(configsync.DefaultReconcileTimeout)) + + steps := 8 + deploysPerStep := 500 + syncCount := 10 + + // Create the namespace using the default root-sync. + // Use the same namespace for all other RSyncs to ensure they all show up in watches, whether it's cluster-scope or namespace-scope. + ns := "stress-test-ns" + nt.T.Logf("Adding test namespace: %s", ns) + nt.Must(nt.RootRepos[configsync.RootSyncName].Add(fmt.Sprintf("%s/ns-%s.yaml", gitproviders.DefaultSyncDir, ns), fake.NamespaceObject(ns))) + + for syncIndex := 1; syncIndex <= syncCount; syncIndex++ { + syncName := fmt.Sprintf("sync-%d", syncIndex) + // Use the same Git repo for all the RootSyncs, but have them sync from different directories. + syncPath := syncName + syncObj := nomostest.RootSyncObjectV1Beta1FromOtherRootRepo(nt, syncName, configsync.RootSyncName) + syncObj.Spec.Git.Dir = syncPath + syncObj.Spec.SafeOverride().LogLevels = []v1beta1.ContainerLogLevelOverride{ + { + ContainerName: "reconciler", + LogLevel: 3, + }, + } + // Manage the RootSyncs with the parent root-sync + nt.Must(nt.RootRepos[configsync.RootSyncName].Add( + fmt.Sprintf("%s/namespaces/%s/rootsync-%s.yaml", gitproviders.DefaultSyncDir, configsync.ControllerNamespace, syncName), + syncObj)) + } + + for step := 1; step <= steps; step++ { + totalDeployCount := deploysPerStep * step + nt.T.Logf("Starting step %d: %d Deployments total", step, totalDeployCount) + + for syncIndex := 1; syncIndex <= syncCount; syncIndex++ { + syncName := fmt.Sprintf("sync-%d", syncIndex) + syncPath := syncName + deployCount := totalDeployCount / syncCount + + nt.T.Logf("Adding %d deployments for RootSync %s", deployCount, syncName) + + for deployIndex := 1; deployIndex <= deployCount; deployIndex++ { + deployName := fmt.Sprintf("pause-%d-%d", syncIndex, deployIndex) + nt.Must(nt.RootRepos[configsync.RootSyncName].AddFile( + fmt.Sprintf("%s/namespaces/%s/deployment-%s.yaml", syncPath, ns, deployName), + []byte(fmt.Sprintf(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: %s + namespace: %s + labels: + app: %s + nomos-test: enabled +spec: + replicas: 1 + selector: + matchLabels: + app: %s + template: + metadata: + labels: + app: %s + spec: + containers: + - name: pause + image: registry.k8s.io/pause:3.7 + ports: + - containerPort: 80 + resources: + limits: + cpu: 250m + memory: 256Mi + requests: + cpu: 250m + memory: 256Mi +`, deployName, ns, deployName, deployName, deployName)))) + } + + nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush(fmt.Sprintf("Adding %d deployments for %d RootSyncs", deployCount, syncCount))) + } + + // Validate that the resources sync without the reconciler running out of + // memory, getting OOMKilled, and crash looping. + if err := nt.WatchForAllSyncs(); err != nil { + nt.T.Fatal(err) + } + + nt.T.Logf("Verify the number of Deployment objects") + validateNumberOfObjectsEquals(nt, kinds.Deployment(), totalDeployCount, + client.MatchingLabels{metadata.ManagedByKey: metadata.ManagedByValue}, + client.InNamespace(ns)) + + nt.T.Log("Removing resources from Git") + for syncIndex := 1; syncIndex <= syncCount; syncIndex++ { + syncName := fmt.Sprintf("sync-%d", syncIndex) + syncPath := syncName + nt.Must(nt.RootRepos[configsync.RootSyncName].Remove(syncPath)) + // Add an empty sync directory so the reconciler doesn't error that it can't find it. + nt.Must(nt.RootRepos[configsync.RootSyncName].AddEmptyDir(syncPath)) + } + nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("Removing resources from Git")) + + // Validate that the resources sync without the reconciler running out of + // memory, getting OOMKilled, and crash looping. + if err := nt.WatchForAllSyncs(); err != nil { + nt.T.Fatal(err) + } + + nt.T.Logf("Verify all Deployments deleted") + validateNumberOfObjectsEquals(nt, kinds.Deployment(), 0, + client.MatchingLabels{metadata.ManagedByKey: metadata.ManagedByValue}, + client.InNamespace(ns)) + } + + nt.T.Log("Removing RootSyncs from Git") + for syncIndex := 1; syncIndex <= syncCount; syncIndex++ { + syncName := fmt.Sprintf("sync-%d", syncIndex) + nt.Must(nt.RootRepos[configsync.RootSyncName].Remove( + fmt.Sprintf("%s/namespaces/%s/rootsync-%s.yaml", gitproviders.DefaultSyncDir, configsync.ControllerNamespace, syncName))) + } + + nt.T.Logf("Removing test namespace: %s", ns) + nt.Must(nt.RootRepos[configsync.RootSyncName].Remove(fmt.Sprintf("%s/ns-%s.yaml", gitproviders.DefaultSyncDir, ns))) + + // Validate that the resources sync without the reconciler running out of + // memory, getting OOMKilled, and crash looping. + if err := nt.WatchForAllSyncs(); err != nil { + nt.T.Fatal(err) + } +} + +// TestStressProfileResourcesByRootSyncCount applies 10,000 Deployments, across +// 10 different RootSyncs, each managing 1000 Deployments. +// This is useful for profiling the resource requests, usage, and limits +// relative to the total number of objects on the cluster, with a single +// resource type, in the same namespace. +func TestStressProfileResourcesByRootSyncCount(t *testing.T) { + nt := nomostest.New(t, nomostesting.Reconciliation1, ntopts.Unstructured, + ntopts.StressTest, + ntopts.WithReconcileTimeout(configsync.DefaultReconcileTimeout)) + + nt.T.Log("Stop the CS webhook by removing the webhook configuration") + nomostest.StopWebhook(nt) + + // Create the namespace using the default root-sync. + // Use the same namespace for all other RSyncs to ensure they all show up in watches, whether it's cluster-scope or namespace-scope. + ns := "stress-test-ns" + nt.T.Logf("Adding test namespace: %s", ns) + nt.Must(nt.RootRepos[configsync.RootSyncName].Add(fmt.Sprintf("%s/ns-%s.yaml", gitproviders.DefaultSyncDir, ns), fake.NamespaceObject(ns))) + + syncCount := 10 + deployCount := 1000 + + for i := 1; i <= syncCount; i++ { + syncName := fmt.Sprintf("sync-%d", i) + // Use the same Git repo for all the RootSyncs, but have them sync from different directories. + syncPath := syncName + syncObj := nomostest.RootSyncObjectV1Beta1FromOtherRootRepo(nt, syncName, configsync.RootSyncName) + syncObj.Spec.Git.Dir = syncPath + syncObj.Spec.SafeOverride().LogLevels = []v1beta1.ContainerLogLevelOverride{ + { + ContainerName: "reconciler", + LogLevel: 3, + }, + } + // Manage the RootSyncs with the parent root-sync + nt.Must(nt.RootRepos[configsync.RootSyncName].Add( + fmt.Sprintf("%s/namespaces/%s/rootsync-%s.yaml", gitproviders.DefaultSyncDir, configsync.ControllerNamespace, syncName), + syncObj)) + + // For each RootSync, make 100 Deployments with unique names + for j := 1; j <= deployCount; j++ { + deployName := fmt.Sprintf("pause-%d-%d", i, j) + nt.Must(nt.RootRepos[configsync.RootSyncName].AddFile( + fmt.Sprintf("%s/namespaces/%s/deployment-%s.yaml", syncPath, ns, deployName), + []byte(fmt.Sprintf(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: %s + namespace: %s + labels: + app: %s + nomos-test: enabled +spec: + replicas: 1 + selector: + matchLabels: + app: %s + template: + metadata: + labels: + app: %s + spec: + containers: + - name: pause + image: registry.k8s.io/pause:3.7 + ports: + - containerPort: 80 + resources: + limits: + cpu: 250m + memory: 256Mi + requests: + cpu: 250m + memory: 256Mi +`, deployName, ns, deployName, deployName, deployName)))) + } + } + + nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush(fmt.Sprintf("Adding %d RootSyncs each with %d deployments", syncCount, deployCount))) + + // Wait for root-sync to sync + if err := nt.WatchForAllSyncs(); err != nil { + nt.T.Fatal(err) + } + + // Wait for the other RootSyncs to sync + nt.T.Log("Waiting for RootSyncs to be synced...") + latestCommit := commitForRepo(nt.RootRepos[configsync.RootSyncName]) + for i := 1; i <= syncCount; i++ { + syncName := fmt.Sprintf("sync-%d", i) + syncPath := syncName + syncObj := nomostest.RootSyncObjectV1Beta1FromOtherRootRepo(nt, syncName, configsync.RootSyncName) + syncObj.Spec.Git.Dir = syncPath + waitForSync(nt, latestCommit, syncObj) + } + + nt.T.Logf("Verify the number of Deployment objects") + validateNumberOfObjectsEquals(nt, kinds.Deployment(), syncCount*deployCount, + client.MatchingLabels{metadata.ManagedByKey: metadata.ManagedByValue}, + client.InNamespace(ns)) + + nt.T.Log("Removing Deployments from Git") + for i := 1; i <= syncCount; i++ { + syncPath := fmt.Sprintf("sync-%d", i) + nt.Must(nt.RootRepos[configsync.RootSyncName].Remove(syncPath)) + // Add an empty sync directory so the reconciler doesn't error that it can't find it. + nt.Must(nt.RootRepos[configsync.RootSyncName].AddEmptyDir(syncPath)) + } + nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("Removing resources from Git")) + + // Validate that the resources sync without the reconciler running out of + // memory, getting OOMKilled, and crash looping. + if err := nt.WatchForAllSyncs(); err != nil { + nt.T.Fatal(err) + } + + nt.T.Log("Removing RootSyncs from Git") + for i := 1; i <= syncCount; i++ { + syncName := fmt.Sprintf("sync-%d", i) + nt.Must(nt.RootRepos[configsync.RootSyncName].Remove( + fmt.Sprintf("%s/namespaces/%s/rootsync-%s.yaml", gitproviders.DefaultSyncDir, configsync.ControllerNamespace, syncName))) + } + + // Validate that the resources sync without the reconciler running out of + // memory, getting OOMKilled, and crash looping. + if err := nt.WatchForAllSyncs(); err != nil { + nt.T.Fatal(err) + } +} + // TestStressMemoryUsageGit applies 100 CRDs and then 50 objects for each // resource. This stressed both the number of watches and the number of objects, // which increases memory usage. @@ -883,12 +1240,15 @@ func truncateSourceErrors() testpredicates.Predicate { } func validateNumberOfObjectsEquals(nt *nomostest.NT, gvk schema.GroupVersionKind, count int, opts ...client.ListOption) { - nsList := &metav1.PartialObjectMetadataList{} - nsList.SetGroupVersionKind(gvk) - if err := nt.KubeClient.List(nsList, opts...); err != nil { - nt.T.Error(err) - } - if len(nsList.Items) != count { - nt.T.Errorf("Expected cluster to have %d %s objects, but found %d", count, gvk, len(nsList.Items)) - } + nomostest.Wait(nt.T, fmt.Sprintf("wait for %d %s objects", count, gvk), nt.DefaultWaitTimeout, func() error { + nsList := &metav1.PartialObjectMetadataList{} + nsList.SetGroupVersionKind(gvk) + if err := nt.KubeClient.List(nsList, opts...); err != nil { + return err + } + if len(nsList.Items) != count { + return fmt.Errorf("expected cluster to have %d %s objects, but found %d", count, gvk, len(nsList.Items)) + } + return nil + }) } diff --git a/go.mod b/go.mod index 8b670a878b..be0cb06845 100644 --- a/go.mod +++ b/go.mod @@ -30,6 +30,7 @@ require ( github.com/spf13/cobra v1.7.0 github.com/spyzhov/ajson v0.9.0 github.com/stretchr/testify v1.8.4 + github.com/wk8/go-ordered-map v1.0.0 go.opencensus.io v0.24.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.26.0 @@ -52,7 +53,7 @@ require ( k8s.io/kubectl v0.28.9 k8s.io/kubernetes v1.28.9 k8s.io/utils v0.0.0-20230726121419-3b25d923346b - sigs.k8s.io/cli-utils v0.35.1-0.20240504222723-227a03f4a7f9 + sigs.k8s.io/cli-utils v0.36.1-0.20240525003310-87074c9799d2 sigs.k8s.io/controller-runtime v0.16.3 sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20231023142458-b9f29826ee83 sigs.k8s.io/kind v0.20.0 diff --git a/go.sum b/go.sum index 65000acb17..3314706e3f 100644 --- a/go.sum +++ b/go.sum @@ -348,6 +348,7 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= @@ -357,6 +358,8 @@ github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXl github.com/urfave/cli v1.22.4/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0= github.com/vbatts/tar-split v0.11.2 h1:Via6XqJr0hceW4wff3QRzD5gAk/tatMw/4ZA7cTlIME= github.com/vbatts/tar-split v0.11.2/go.mod h1:vV3ZuO2yWSVsz+pfFzDG/upWH1JhjOiEaWq6kXyQ3VI= +github.com/wk8/go-ordered-map v1.0.0 h1:BV7z+2PaK8LTSd/mWgY12HyMAo5CEgkHqbkVq2thqr8= +github.com/wk8/go-ordered-map v1.0.0/go.mod h1:9ZIbRunKbuvfPKyBP1SIKLcXNlv74YCOZ3t3VTS6gRk= github.com/xlab/treeprint v1.2.0 h1:HzHnuAF1plUN2zGlAFHbSQP2qJ0ZAD3XF5XD7OesXRQ= github.com/xlab/treeprint v1.2.0/go.mod h1:gj5Gd3gPdKtR1ikdDK6fnFLdmIS0X30kTTuNd/WEJu0= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= @@ -690,8 +693,10 @@ k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= -sigs.k8s.io/cli-utils v0.35.1-0.20240504222723-227a03f4a7f9 h1:fe6u8xC3vudos+7qXxaxEbB6xS0MLjv4tZN3t1sAWIo= -sigs.k8s.io/cli-utils v0.35.1-0.20240504222723-227a03f4a7f9/go.mod h1:uCFC3BPXB3xHFQyKkWUlTrncVDCKzbdDfqZqRTCrk24= +sigs.k8s.io/cli-utils v0.36.1-0.20240521200942-9702fb836460 h1:W3zarTyu2J5AJUEvul9Yo3lJo1bSdKgV5guWZ4WvYr0= +sigs.k8s.io/cli-utils v0.36.1-0.20240521200942-9702fb836460/go.mod h1:uCFC3BPXB3xHFQyKkWUlTrncVDCKzbdDfqZqRTCrk24= +sigs.k8s.io/cli-utils v0.36.1-0.20240525003310-87074c9799d2 h1:XDyZuBsUagBzTd/z3V4S8ddbejiyA7tgmuzSfwKTkPI= +sigs.k8s.io/cli-utils v0.36.1-0.20240525003310-87074c9799d2/go.mod h1:uCFC3BPXB3xHFQyKkWUlTrncVDCKzbdDfqZqRTCrk24= sigs.k8s.io/controller-runtime v0.16.3 h1:2TuvuokmfXvDUamSx1SuAOO3eTyye+47mJCigwG62c4= sigs.k8s.io/controller-runtime v0.16.3/go.mod h1:j7bialYoSn142nv9sCOJmQgDXQXxnroFU4VnX/brVJ0= sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20231023142458-b9f29826ee83 h1:37G7lMdeXe0kogUnwCa1K+EIVYR5f4BqM0bqITyaDBI= diff --git a/manifests/reposync-crd.yaml b/manifests/reposync-crd.yaml index e2b7c4f73a..8cb0ee2db9 100644 --- a/manifests/reposync-crd.yaml +++ b/manifests/reposync-crd.yaml @@ -932,6 +932,10 @@ spec: format: date-time nullable: true type: string + objectCount: + description: objectCount is the number of objects detected in + the source of truth, after rendering, if applicable. + type: integer ociStatus: description: ociStatus contains fields describing the status of an OCI source of truth. @@ -1089,6 +1093,10 @@ spec: format: date-time nullable: true type: string + objectCount: + description: objectCount is the number of objects intended to + be synced to the cluster. + type: integer ociStatus: description: ociStatus contains fields describing the status of an OCI source of truth. @@ -2014,6 +2022,10 @@ spec: format: date-time nullable: true type: string + objectCount: + description: objectCount is the number of objects detected in + the source of truth, after rendering, if applicable. + type: integer ociStatus: description: ociStatus contains fields describing the status of an OCI source of truth. @@ -2171,6 +2183,10 @@ spec: format: date-time nullable: true type: string + objectCount: + description: objectCount is the number of objects intended to + be synced to the cluster. + type: integer ociStatus: description: ociStatus contains fields describing the status of an OCI source of truth. diff --git a/manifests/rootsync-crd.yaml b/manifests/rootsync-crd.yaml index 94ea9d2085..e6154ff10d 100644 --- a/manifests/rootsync-crd.yaml +++ b/manifests/rootsync-crd.yaml @@ -987,6 +987,10 @@ spec: format: date-time nullable: true type: string + objectCount: + description: objectCount is the number of objects detected in + the source of truth, after rendering, if applicable. + type: integer ociStatus: description: ociStatus contains fields describing the status of an OCI source of truth. @@ -1144,6 +1148,10 @@ spec: format: date-time nullable: true type: string + objectCount: + description: objectCount is the number of objects intended to + be synced to the cluster. + type: integer ociStatus: description: ociStatus contains fields describing the status of an OCI source of truth. @@ -2124,6 +2132,10 @@ spec: format: date-time nullable: true type: string + objectCount: + description: objectCount is the number of objects detected in + the source of truth, after rendering, if applicable. + type: integer ociStatus: description: ociStatus contains fields describing the status of an OCI source of truth. @@ -2281,6 +2293,10 @@ spec: format: date-time nullable: true type: string + objectCount: + description: objectCount is the number of objects intended to + be synced to the cluster. + type: integer ociStatus: description: ociStatus contains fields describing the status of an OCI source of truth. diff --git a/pkg/api/configsync/register.go b/pkg/api/configsync/register.go index fcd4ca100d..4b2e073289 100644 --- a/pkg/api/configsync/register.go +++ b/pkg/api/configsync/register.go @@ -14,7 +14,9 @@ package configsync -import "time" +import ( + "time" +) const ( // GroupName is the name of the group of configsync resources. @@ -23,7 +25,8 @@ const ( // ConfigSyncPrefix is the prefix for all ConfigSync annotations and labels. ConfigSyncPrefix = GroupName + "/" - // FieldManager is the field manager name for server-side apply. + // FieldManager is the field manager name used by the reconciler. + // This avoids conflicts between the reconciler and reconciler-manager. FieldManager = GroupName // ControllerNamespace is the Namespace used for Nomos controllers diff --git a/pkg/api/configsync/v1alpha1/sync_types.go b/pkg/api/configsync/v1alpha1/sync_types.go index 4c18878c26..50eff5581d 100644 --- a/pkg/api/configsync/v1alpha1/sync_types.go +++ b/pkg/api/configsync/v1alpha1/sync_types.go @@ -71,6 +71,11 @@ type SourceStatus struct { // +optional Commit string `json:"commit,omitempty"` + // objectCount is the number of objects detected in the source of truth, + // after rendering, if applicable. + // +optional + ObjectCount int `json:"objectCount,omitempty"` + // lastUpdate is the timestamp of when this status was last updated by a // reconciler. // +nullable @@ -142,6 +147,10 @@ type SyncStatus struct { // +optional Commit string `json:"commit,omitempty"` + // objectCount is the number of objects intended to be synced to the cluster. + // +optional + ObjectCount int `json:"objectCount,omitempty"` + // lastUpdate is the timestamp of when this status was last updated by a // reconciler. // +nullable diff --git a/pkg/api/configsync/v1alpha1/zz_generated.conversion.go b/pkg/api/configsync/v1alpha1/zz_generated.conversion.go index a683d230ff..6a8f6e37e1 100644 --- a/pkg/api/configsync/v1alpha1/zz_generated.conversion.go +++ b/pkg/api/configsync/v1alpha1/zz_generated.conversion.go @@ -1241,6 +1241,7 @@ func autoConvert_v1alpha1_SourceStatus_To_v1beta1_SourceStatus(in *SourceStatus, out.Oci = (*v1beta1.OciStatus)(unsafe.Pointer(in.Oci)) out.Helm = (*v1beta1.HelmStatus)(unsafe.Pointer(in.Helm)) out.Commit = in.Commit + out.ObjectCount = in.ObjectCount out.LastUpdate = in.LastUpdate out.Errors = *(*[]v1beta1.ConfigSyncError)(unsafe.Pointer(&in.Errors)) out.ErrorSummary = (*v1beta1.ErrorSummary)(unsafe.Pointer(in.ErrorSummary)) @@ -1257,6 +1258,7 @@ func autoConvert_v1beta1_SourceStatus_To_v1alpha1_SourceStatus(in *v1beta1.Sourc out.Oci = (*OciStatus)(unsafe.Pointer(in.Oci)) out.Helm = (*HelmStatus)(unsafe.Pointer(in.Helm)) out.Commit = in.Commit + out.ObjectCount = in.ObjectCount out.LastUpdate = in.LastUpdate out.Errors = *(*[]ConfigSyncError)(unsafe.Pointer(&in.Errors)) out.ErrorSummary = (*ErrorSummary)(unsafe.Pointer(in.ErrorSummary)) @@ -1315,6 +1317,7 @@ func autoConvert_v1alpha1_SyncStatus_To_v1beta1_SyncStatus(in *SyncStatus, out * out.Oci = (*v1beta1.OciStatus)(unsafe.Pointer(in.Oci)) out.Helm = (*v1beta1.HelmStatus)(unsafe.Pointer(in.Helm)) out.Commit = in.Commit + out.ObjectCount = in.ObjectCount out.LastUpdate = in.LastUpdate out.Errors = *(*[]v1beta1.ConfigSyncError)(unsafe.Pointer(&in.Errors)) out.ErrorSummary = (*v1beta1.ErrorSummary)(unsafe.Pointer(in.ErrorSummary)) @@ -1331,6 +1334,7 @@ func autoConvert_v1beta1_SyncStatus_To_v1alpha1_SyncStatus(in *v1beta1.SyncStatu out.Oci = (*OciStatus)(unsafe.Pointer(in.Oci)) out.Helm = (*HelmStatus)(unsafe.Pointer(in.Helm)) out.Commit = in.Commit + out.ObjectCount = in.ObjectCount out.LastUpdate = in.LastUpdate out.Errors = *(*[]ConfigSyncError)(unsafe.Pointer(&in.Errors)) out.ErrorSummary = (*ErrorSummary)(unsafe.Pointer(in.ErrorSummary)) diff --git a/pkg/api/configsync/v1beta1/sync_types.go b/pkg/api/configsync/v1beta1/sync_types.go index f5f5291916..5bef877002 100644 --- a/pkg/api/configsync/v1beta1/sync_types.go +++ b/pkg/api/configsync/v1beta1/sync_types.go @@ -71,6 +71,11 @@ type SourceStatus struct { // +optional Commit string `json:"commit,omitempty"` + // objectCount is the number of objects detected in the source of truth, + // after rendering, if applicable. + // +optional + ObjectCount int `json:"objectCount,omitempty"` + // lastUpdate is the timestamp of when this status was last updated by a // reconciler. // +nullable @@ -142,6 +147,10 @@ type SyncStatus struct { // +optional Commit string `json:"commit,omitempty"` + // objectCount is the number of objects intended to be synced to the cluster. + // +optional + ObjectCount int `json:"objectCount,omitempty"` + // lastUpdate is the timestamp of when this status was last updated by a // reconciler. // +nullable diff --git a/pkg/applier/applier.go b/pkg/applier/applier.go index c25f402edb..8ab5872143 100644 --- a/pkg/applier/applier.go +++ b/pkg/applier/applier.go @@ -26,9 +26,11 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" "kpt.dev/configsync/pkg/api/configmanagement" "kpt.dev/configsync/pkg/api/configsync" + "kpt.dev/configsync/pkg/api/kpt.dev/v1alpha1" "kpt.dev/configsync/pkg/applier/stats" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/declared" @@ -68,28 +70,60 @@ var ( type Applier interface { // Apply creates, updates, or prunes all managed resources, depending on // the new desired resource objects. - // Returns the set of GVKs which were successfully applied and any errors. + // Errors and inventory events are sent to the superEventHandler. + // Returns the status of the applied objects and statistics of the sync. // This is called by the reconciler when changes are detected in the // source of truth (git, OCI, helm) and periodically. - Apply(ctx context.Context, desiredResources []client.Object) status.MultiError - // Errors returns the errors encountered during apply. - // This method may be called while Destroy is running, to get the set of - // errors encountered so far. - Errors() status.MultiError + Apply(ctx context.Context, superEventHandler func(SuperEvent), desiredResources []client.Object) (ObjectStatusMap, *stats.SyncStats) } // Destroyer is a bulk client for deleting all the managed resource objects // tracked in a single ResourceGroup inventory. type Destroyer interface { // Destroy deletes all managed resources. - // Returns any errors encountered while destroying. + // Errors and inventory events are sent to the superEventHandler. + // Returns the status of the destroyed objects and statistics of the sync. // This is called by the reconciler finalizer when deletion propagation is // enabled. - Destroy(ctx context.Context) status.MultiError - // Errors returns the errors encountered during destroy. - // This method may be called while Destroy is running, to get the set of - // errors encountered so far. - Errors() status.MultiError + Destroy(ctx context.Context, superEventHandler func(SuperEvent)) (ObjectStatusMap, *stats.SyncStats) +} + +// SuperEventType is the type used by SuperEvent.Type +type SuperEventType string + +const ( + // SuperErrorEventType is the type of the SuperErrorEvent + SuperErrorEventType SuperEventType = "SuperErrorEvent" + // SuperInventoryEventType is the type of the SuperInventoryEvent + SuperInventoryEventType SuperEventType = "SuperInventoryEvent" +) + +// SuperEvent is sent to the superEventHandler by the supervisor. +type SuperEvent interface { + // Type returns the type of the event. + Type() SuperEventType +} + +// SuperErrorEvent is sent after the supervisor has errored. +// Generally, the supervisor will still continue until success or timeout. +type SuperErrorEvent struct { + Error status.Error +} + +// Type returns the type of the event. +func (e SuperErrorEvent) Type() SuperEventType { + return SuperErrorEventType +} + +// SuperInventoryEvent is sent after the inventory is updated. +// It may or may not have actually changed. +type SuperInventoryEvent struct { + Inventory *v1alpha1.ResourceGroup +} + +// Type returns the type of the event. +func (e SuperInventoryEvent) Type() SuperEventType { + return SuperInventoryEventType } // Supervisor is a bulk client for applying and deleting a mutable set of @@ -125,11 +159,6 @@ type supervisor struct { // execMux prevents concurrent Apply/Destroy calls execMux sync.Mutex - // errorMux prevents concurrent modifications to the cached set of errors - errorMux sync.RWMutex - // errs received from the current (if running) or previous Apply/Destroy. - // These errors is cleared at the start of the Apply/Destroy methods. - errs status.MultiError } var _ Applier = &supervisor{} @@ -497,14 +526,14 @@ func (a *supervisor) checkInventoryObjectSize(ctx context.Context, c client.Clie } // applyInner triggers a kpt live apply library call to apply a set of resources. -func (a *supervisor) applyInner(ctx context.Context, objs []client.Object) status.MultiError { +func (a *supervisor) applyInner(ctx context.Context, superEventHandler func(SuperEvent), objs []client.Object) (ObjectStatusMap, *stats.SyncStats) { a.checkInventoryObjectSize(ctx, a.clientSet.Client) eh := eventHandler{ isDestroy: false, clientSet: a.clientSet, } - s := stats.NewSyncStats() + syncStats := stats.NewSyncStats() objStatusMap := make(ObjectStatusMap) // disabledObjs are objects for which the management are disabled // through annotation. @@ -513,10 +542,10 @@ func (a *supervisor) applyInner(ctx context.Context, objs []client.Object) statu klog.Infof("%v objects to be disabled: %v", len(disabledObjs), core.GKNNs(disabledObjs)) disabledCount, err := eh.handleDisabledObjects(ctx, a.inventory, disabledObjs) if err != nil { - a.addError(err) - return a.Errors() + a.sendErrorEvent(err, superEventHandler) + return objStatusMap, syncStats } - s.DisableObjs = &stats.DisabledObjStats{ + syncStats.DisableObjs = &stats.DisabledObjStats{ Total: uint64(len(disabledObjs)), Succeeded: disabledCount, } @@ -524,8 +553,8 @@ func (a *supervisor) applyInner(ctx context.Context, objs []client.Object) statu klog.Infof("%v objects to be applied: %v", len(enabledObjs), core.GKNNs(enabledObjs)) resources, err := toUnstructured(enabledObjs) if err != nil { - a.addError(err) - return a.Errors() + a.sendErrorEvent(err, superEventHandler) + return objStatusMap, syncStats } unknownTypeResources := make(map[core.ID]struct{}) @@ -571,14 +600,21 @@ func (a *supervisor) applyInner(ctx context.Context, objs []client.Object) statu } case event.ActionGroupType: klog.Info(e.ActionGroupEvent) + if e.ActionGroupEvent.Action == event.InventoryAction && e.ActionGroupEvent.Status == event.Finished { + inventory, err := a.getInventory(ctx) + if err != nil { + a.sendErrorEvent(err, superEventHandler) + } + a.sendInventoryEvent(inventory, superEventHandler) + } case event.ErrorType: klog.Info(e.ErrorEvent) - if util.IsRequestTooLargeError(e.ErrorEvent.Err) { - a.addError(largeResourceGroupError(e.ErrorEvent.Err, idFromInventory(a.inventory))) - } else { - a.addError(e.ErrorEvent.Err) + err := e.ErrorEvent.Err + if util.IsRequestTooLargeError(err) { + err = largeResourceGroupError(err, idFromInventory(a.inventory)) } - s.ErrorTypeEvents++ + a.sendErrorEvent(err, superEventHandler) + syncStats.ErrorTypeEvents++ case event.WaitType: // Pending events are sent for any objects that haven't reconciled // when the WaitEvent starts. They're not very useful to the user. @@ -588,78 +624,69 @@ func (a *supervisor) applyInner(ctx context.Context, objs []client.Object) statu } else { klog.V(1).Info(e.WaitEvent) } - a.addError(eh.processWaitEvent(e.WaitEvent, s.WaitEvent, objStatusMap)) + if err := eh.processWaitEvent(e.WaitEvent, syncStats.WaitEvent, objStatusMap); err != nil { + a.sendErrorEvent(err, superEventHandler) + } case event.ApplyType: if e.ApplyEvent.Error != nil { klog.Info(e.ApplyEvent) } else { klog.V(1).Info(e.ApplyEvent) } - a.addError(eh.processApplyEvent(ctx, e.ApplyEvent, s.ApplyEvent, objStatusMap, unknownTypeResources, resourceMap)) + if err := eh.processApplyEvent(ctx, e.ApplyEvent, syncStats.ApplyEvent, objStatusMap, unknownTypeResources, resourceMap); err != nil { + a.sendErrorEvent(err, superEventHandler) + } case event.PruneType: if e.PruneEvent.Error != nil { klog.Info(e.PruneEvent) } else { klog.V(1).Info(e.PruneEvent) } - a.addError(eh.processPruneEvent(ctx, e.PruneEvent, s.PruneEvent, objStatusMap)) + if err := eh.processPruneEvent(ctx, e.PruneEvent, syncStats.PruneEvent, objStatusMap); err != nil { + a.sendErrorEvent(err, superEventHandler) + } default: klog.Infof("Unhandled event (%s): %v", e.Type, e) } } - errs := a.Errors() - if errs == nil { - klog.V(4).Infof("Apply completed without error: all resources are up to date.") - } - if s.Empty() { - klog.V(4).Infof("Applier made no new progress") - } else { - klog.Infof("Applier made new progress: %s", s.String()) - objStatusMap.Log(klog.V(0)) - } - return errs + return objStatusMap, syncStats } -// Errors returns the errors encountered during the last apply or current apply -// if still running. -// Errors implements the Applier and Destroyer interfaces. -func (a *supervisor) Errors() status.MultiError { - a.errorMux.RLock() - defer a.errorMux.RUnlock() - - if a.errs != nil { - // Return a copy to avoid persisting caller modifications - return status.Wrap(a.errs.Errors()...) - } - return nil +func (a *supervisor) sendErrorEvent(err error, superEventHandler func(SuperEvent)) { + superEventHandler(SuperErrorEvent{Error: wrapError(err)}) } -func (a *supervisor) addError(err error) { - if err == nil { - return - } - a.errorMux.Lock() - defer a.errorMux.Unlock() - - if _, ok := err.(status.Error); !ok { - // Wrap as an applier.Error to indicate the source of the error - err = Error(err) +func wrapError(err error) status.Error { + if statusErr, ok := err.(status.Error); ok { + return statusErr } - - a.errs = status.Append(a.errs, err) + // Wrap as an applier.Error to indicate the source of the error + return Error(err) } -func (a *supervisor) invalidateErrors() { - a.errorMux.Lock() - defer a.errorMux.Unlock() +func (a *supervisor) sendInventoryEvent(inventory *v1alpha1.ResourceGroup, superEventHandler func(SuperEvent)) { + superEventHandler(SuperInventoryEvent{Inventory: inventory}) +} - a.errs = nil +func (a *supervisor) getInventory(ctx context.Context) (*v1alpha1.ResourceGroup, error) { + rg := &v1alpha1.ResourceGroup{} + rgRef := types.NamespacedName{ + Name: a.inventory.Name(), + Namespace: a.inventory.Namespace(), + } + if err := a.clientSet.Client.Get(ctx, rgRef, rg); err != nil { + if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) { + return nil, nil + } + return nil, status.APIServerError(err, "reading inventory") + } + return rg, nil } // destroyInner triggers a kpt live destroy library call to destroy a set of resources. -func (a *supervisor) destroyInner(ctx context.Context) status.MultiError { - s := stats.NewSyncStats() +func (a *supervisor) destroyInner(ctx context.Context, superEventHandler func(SuperEvent)) (ObjectStatusMap, *stats.SyncStats) { + syncStats := stats.NewSyncStats() objStatusMap := make(ObjectStatusMap) eh := eventHandler{ isDestroy: true, @@ -692,14 +719,21 @@ func (a *supervisor) destroyInner(ctx context.Context) status.MultiError { } case event.ActionGroupType: klog.Info(e.ActionGroupEvent) + if e.ActionGroupEvent.Action == event.InventoryAction && e.ActionGroupEvent.Status == event.Finished { + inventory, err := a.getInventory(ctx) + if err != nil { + a.sendErrorEvent(err, superEventHandler) + } + a.sendInventoryEvent(inventory, superEventHandler) + } case event.ErrorType: klog.Info(e.ErrorEvent) - if util.IsRequestTooLargeError(e.ErrorEvent.Err) { - a.addError(largeResourceGroupError(e.ErrorEvent.Err, idFromInventory(a.inventory))) - } else { - a.addError(e.ErrorEvent.Err) + err := e.ErrorEvent.Err + if util.IsRequestTooLargeError(err) { + err = largeResourceGroupError(err, idFromInventory(a.inventory)) } - s.ErrorTypeEvents++ + a.sendErrorEvent(err, superEventHandler) + syncStats.ErrorTypeEvents++ case event.WaitType: // Pending events are sent for any objects that haven't reconciled // when the WaitEvent starts. They're not very useful to the user. @@ -709,60 +743,45 @@ func (a *supervisor) destroyInner(ctx context.Context) status.MultiError { } else { klog.V(1).Info(e.WaitEvent) } - a.addError(eh.processWaitEvent(e.WaitEvent, s.WaitEvent, objStatusMap)) + if err := eh.processWaitEvent(e.WaitEvent, syncStats.WaitEvent, objStatusMap); err != nil { + a.sendErrorEvent(err, superEventHandler) + } case event.DeleteType: if e.DeleteEvent.Error != nil { klog.Info(e.DeleteEvent) } else { klog.V(1).Info(e.DeleteEvent) } - a.addError(eh.processDeleteEvent(ctx, e.DeleteEvent, s.DeleteEvent, objStatusMap)) + if err := eh.processDeleteEvent(ctx, e.DeleteEvent, syncStats.DeleteEvent, objStatusMap); err != nil { + a.sendErrorEvent(err, superEventHandler) + } default: klog.Infof("Unhandled event (%s): %v", e.Type, e) } } - - errs := a.Errors() - if errs == nil { - klog.V(4).Infof("Destroy completed without error: all resources are deleted.") - } - if s.Empty() { - klog.V(4).Infof("Destroyer made no new progress") - } else { - klog.Infof("Destroyer made new progress: %s.", s.String()) - objStatusMap.Log(klog.V(0)) - } - return errs + return objStatusMap, syncStats } -// Apply all managed resource objects and return any errors. +// Apply all managed resource objects and return their status. // Apply implements the Applier interface. -func (a *supervisor) Apply(ctx context.Context, desiredResource []client.Object) status.MultiError { +func (a *supervisor) Apply(ctx context.Context, superEventHandler func(SuperEvent), desiredResource []client.Object) (ObjectStatusMap, *stats.SyncStats) { a.execMux.Lock() defer a.execMux.Unlock() - // Ideally we want to avoid invalidating errors that will continue to happen, - // but for now, invalidate all errors until they recur. - // TODO: improve error cache invalidation to make rsync status more stable - a.invalidateErrors() - return a.applyInner(ctx, desiredResource) + return a.applyInner(ctx, superEventHandler, desiredResource) } -// Destroy all managed resource objects and return any errors. +// Destroy all managed resource objects and return their status. // Destroy implements the Destroyer interface. -func (a *supervisor) Destroy(ctx context.Context) status.MultiError { +func (a *supervisor) Destroy(ctx context.Context, superEventHandler func(SuperEvent)) (ObjectStatusMap, *stats.SyncStats) { a.execMux.Lock() defer a.execMux.Unlock() - // Ideally we want to avoid invalidating errors that will continue to happen, - // but for now, invalidate all errors until they recur. - // TODO: improve error cache invalidation to make rsync status more stable - a.invalidateErrors() - return a.destroyInner(ctx) + return a.destroyInner(ctx, superEventHandler) } // newInventoryUnstructured creates an inventory object as an unstructured. -func newInventoryUnstructured(kind, name, namespace, statusMode string) *unstructured.Unstructured { +func newInventoryUnstructured(kind, name, namespace string, statusMode InventoryStatusMode) *unstructured.Unstructured { id := InventoryID(name, namespace) u := resourcegroup.Unstructured(name, namespace, id) core.SetLabel(u, metadata.ManagedByKey, metadata.ManagedByValue) @@ -770,15 +789,15 @@ func newInventoryUnstructured(kind, name, namespace, statusMode string) *unstruc core.SetLabel(u, metadata.SyncNameLabel, name) core.SetLabel(u, metadata.SyncKindLabel, kind) core.SetAnnotation(u, metadata.ResourceManagementKey, metadata.ResourceManagementEnabled) - core.SetAnnotation(u, StatusModeKey, statusMode) + core.SetAnnotation(u, StatusModeKey, statusMode.String()) return u } // InventoryID returns the inventory id of an inventory object. // The inventory object generated by ConfigSync is in the same namespace as RootSync or RepoSync. // The inventory ID is assigned as _. -func InventoryID(name, namespace string) string { - return namespace + "_" + name +func InventoryID(syncName, syncNamespace string) string { + return fmt.Sprintf("%s_%s", syncNamespace, syncName) } // handleDisabledObjects removes the specified objects from the inventory, and diff --git a/pkg/applier/applier_test.go b/pkg/applier/applier_test.go index 9fc7d97d3a..20b2e1f7c0 100644 --- a/pkg/applier/applier_test.go +++ b/pkg/applier/applier_test.go @@ -94,10 +94,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", }) testObj2 := newTestObj("test-2") @@ -280,8 +281,22 @@ func TestApply(t *testing.T) { applier, err := NewNamespaceSupervisor(cs, syncScope, syncName, 5*time.Minute) require.NoError(t, err) - errs := applier.Apply(context.Background(), objs) + var errs status.MultiError + superEventHandler := func(event SuperEvent) { + if errEvent, ok := event.(SuperErrorEvent); ok { + if errs == nil { + errs = errEvent.Error + } else { + errs = status.Append(errs, errEvent.Error) + } + } + } + + // TODO: Test statusMap & stats + _, _ = applier.Apply(context.Background(), superEventHandler, objs) + // testutil.AssertEqual(t, tc.expectedGVKs, gvks) testerrors.AssertEqual(t, tc.expectedError, errs) + fakeClient.Check(t, tc.expectedServerObjs...) }) } diff --git a/pkg/applier/clientset.go b/pkg/applier/clientset.go index f46194c055..d321ebb6e6 100644 --- a/pkg/applier/clientset.go +++ b/pkg/applier/clientset.go @@ -19,12 +19,16 @@ 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/declared" + "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" ) @@ -49,11 +53,11 @@ type ClientSet struct { InvClient inventory.Client Client client.Client Mapper meta.RESTMapper - StatusMode string + StatusMode InventoryStatusMode } // 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 InventoryStatusMode, syncName string, syncScope declared.Scope) (*ClientSet, error) { matchVersionKubeConfigFlags := util.NewMatchVersionFlags(configFlags) f := util.NewFactory(matchVersionKubeConfigFlags) @@ -71,9 +75,23 @@ func NewClientSet(c client.Client, configFlags *genericclioptions.ConfigFlags, s return nil, err } + packageID := metadata.PackageID(syncName, + declared.SyncNamespaceFromScope(syncScope), + declared.SyncKindFromScope(syncScope)) + + // 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 +100,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/applier/constants.go b/pkg/applier/constants.go index b88ebeca88..31cc5e0db4 100644 --- a/pkg/applier/constants.go +++ b/pkg/applier/constants.go @@ -19,10 +19,10 @@ import "kpt.dev/configsync/pkg/api/configsync" const ( // StatusEnabled is used to allow kpt applier to inject the actuation status // into the ResourceGroup object. - StatusEnabled = "enabled" + StatusEnabled InventoryStatusMode = "enabled" // StatusDisabled is used to stop kpt applier to inject the actuation status // into the ResourceGroup object. - StatusDisabled = "disabled" + StatusDisabled InventoryStatusMode = "disabled" // StatusModeKey annotates a ResourceGroup CR // to communicate with the ResourceGroup controller. @@ -30,3 +30,12 @@ const ( // ignores the ResourceGroup CR. StatusModeKey = configsync.ConfigSyncPrefix + "status" ) + +// InventoryStatusMode is an enum describing how to report object status in the inventory. +type InventoryStatusMode string + +// String returns the InventoryStatusMode as a string. +// Satisfies the Stringer interface. +func (m InventoryStatusMode) String() string { + return string(m) +} diff --git a/pkg/applier/destroyer_test.go b/pkg/applier/destroyer_test.go index 18b934b315..94c1fb563d 100644 --- a/pkg/applier/destroyer_test.go +++ b/pkg/applier/destroyer_test.go @@ -198,7 +198,19 @@ func TestDestroy(t *testing.T) { destroyer, err := NewNamespaceSupervisor(cs, "test-namespace", "rs", 5*time.Minute) require.NoError(t, err) - errs := destroyer.Destroy(context.Background()) + var errs status.MultiError + superEventHandler := func(event SuperEvent) { + if errEvent, ok := event.(SuperErrorEvent); ok { + if errs == nil { + errs = errEvent.Error + } else { + errs = status.Append(errs, errEvent.Error) + } + } + } + + // TODO: Test statusMap & stats + _, _ = destroyer.Destroy(context.Background(), superEventHandler) testerrors.AssertEqual(t, tc.multiErr, errs) }) } diff --git a/pkg/applier/fake/applier.go b/pkg/applier/fake/applier.go index 4b12bcab93..0c6c4f2de8 100644 --- a/pkg/applier/fake/applier.go +++ b/pkg/applier/fake/applier.go @@ -18,7 +18,9 @@ import ( "context" "fmt" + "kpt.dev/configsync/pkg/api/kpt.dev/v1alpha1" "kpt.dev/configsync/pkg/applier" + "kpt.dev/configsync/pkg/applier/stats" "kpt.dev/configsync/pkg/status" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -31,9 +33,7 @@ type Applier struct { ApplyInputs []ApplierInputs ApplyOutputs []ApplierOutputs - ApplyCalls, ErrorsCalls int - - currentErrors status.MultiError + ApplyCalls int } // ApplierInputs stores inputs for fake.Applier.Apply() @@ -43,11 +43,13 @@ type ApplierInputs struct { // ApplierOutputs stores outputs for fake.Applier.Apply() type ApplierOutputs struct { - Errors status.MultiError + Errors []status.Error + ObjectStatusMap applier.ObjectStatusMap + SyncStats *stats.SyncStats } // Apply fakes applier.Applier.Apply() -func (a *Applier) Apply(_ context.Context, objects []client.Object) status.MultiError { +func (a *Applier) Apply(_ context.Context, superEventHandler func(applier.SuperEvent), objects []client.Object) (applier.ObjectStatusMap, *stats.SyncStats) { a.ApplyInputs = append(a.ApplyInputs, ApplierInputs{ Objects: objects, }) @@ -56,13 +58,20 @@ func (a *Applier) Apply(_ context.Context, objects []client.Object) status.Multi } outputs := a.ApplyOutputs[a.ApplyCalls] a.ApplyCalls++ - a.currentErrors = outputs.Errors - return outputs.Errors -} -// Errors fakes applier.Applier.Errors() -func (a *Applier) Errors() status.MultiError { - return a.currentErrors + superEventHandler(applier.SuperInventoryEvent{ + Inventory: &v1alpha1.ResourceGroup{}, + }) + for _, err := range outputs.Errors { + superEventHandler(applier.SuperErrorEvent{ + Error: err, + }) + } + superEventHandler(applier.SuperInventoryEvent{ + Inventory: &v1alpha1.ResourceGroup{}, + }) + + return outputs.ObjectStatusMap, outputs.SyncStats } var _ applier.Applier = &Applier{} diff --git a/pkg/applier/management_conflict_err.go b/pkg/applier/management_conflict_err.go index 2ff36b4386..8d2f8723ec 100644 --- a/pkg/applier/management_conflict_err.go +++ b/pkg/applier/management_conflict_err.go @@ -15,6 +15,7 @@ package applier import ( + "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/metadata" "kpt.dev/configsync/pkg/status" "sigs.k8s.io/controller-runtime/pkg/client" @@ -23,10 +24,12 @@ import ( // KptManagementConflictError indicates that the passed resource is illegally // declared in multiple repositories. // TODO: merge with status.ManagementConflictError if cli-utils supports reporting the conflicting manager in InventoryOverlapError. -func KptManagementConflictError(resource client.Object) status.Error { +func KptManagementConflictError(resource client.Object) status.ManagementConflictError { + newManager := core.GetAnnotation(resource, metadata.ResourceManagerKey) + currentManager := "UNKNOWN" return status.ManagementConflictErrorBuilder. Sprintf("The %q reconciler cannot manage resources declared in another repository. "+ "Remove the declaration for this resource from either the current repository, or the managed repository.", - resource.GetAnnotations()[metadata.ResourceManagerKey]). - BuildWithResources(resource) + newManager). + BuildWithConflictingManagers(resource, newManager, currentManager) } diff --git a/pkg/applier/utils.go b/pkg/applier/utils.go index 86c2221b63..0446e62bec 100644 --- a/pkg/applier/utils.go +++ b/pkg/applier/utils.go @@ -23,6 +23,7 @@ import ( "golang.org/x/net/context" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "kpt.dev/configsync/pkg/api/kpt.dev/v1alpha1" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/metadata" "kpt.dev/configsync/pkg/status" @@ -87,6 +88,16 @@ func idFrom(identifier object.ObjMetadata) core.ID { } } +func idFromInvObjMeta(objMeta v1alpha1.ObjMetadata) core.ID { + return core.ID{ + GroupKind: objMeta.GK(), + ObjectKey: client.ObjectKey{ + Namespace: objMeta.Namespace, + Name: objMeta.Name, + }, + } +} + func idFromInventory(rg *live.InventoryResourceGroup) core.ID { return core.ID{ GroupKind: live.ResourceGroupGVK.GroupKind(), @@ -126,7 +137,7 @@ func getObjectSize(u *unstructured.Unstructured) (int, error) { return len(data), nil } -func annotateStatusMode(ctx context.Context, c client.Client, u *unstructured.Unstructured, statusMode string) error { +func annotateStatusMode(ctx context.Context, c client.Client, u *unstructured.Unstructured, statusMode InventoryStatusMode) error { err := c.Get(ctx, client.ObjectKey{Name: u.GetName(), Namespace: u.GetNamespace()}, u) if err != nil { if errors.IsNotFound(err) { @@ -138,7 +149,7 @@ func annotateStatusMode(ctx context.Context, c client.Client, u *unstructured.Un if annotations == nil { annotations = make(map[string]string) } - annotations[StatusModeKey] = statusMode + annotations[StatusModeKey] = statusMode.String() u.SetAnnotations(annotations) return c.Update(ctx, u) } diff --git a/pkg/core/scheme.go b/pkg/core/scheme.go index b0dd98388e..1d442c43aa 100644 --- a/pkg/core/scheme.go +++ b/pkg/core/scheme.go @@ -40,6 +40,7 @@ import ( configsyncv1alpha1 "kpt.dev/configsync/pkg/api/configsync/v1alpha1" configsyncv1beta1 "kpt.dev/configsync/pkg/api/configsync/v1beta1" hubv1 "kpt.dev/configsync/pkg/api/hub/v1" + kptv1alpha1 "kpt.dev/configsync/pkg/api/kpt.dev/v1alpha1" ) // Scheme is a reference to the global scheme. @@ -70,6 +71,7 @@ func init() { utilruntime.Must(configsyncv1alpha1.AddToScheme(scheme.Scheme)) utilruntime.Must(configsyncv1alpha1.RegisterConversions(scheme.Scheme)) utilruntime.Must(scheme.Scheme.SetVersionPriority(configsyncv1beta1.SchemeGroupVersion, configsyncv1alpha1.SchemeGroupVersion)) + utilruntime.Must(kptv1alpha1.AddToScheme(scheme.Scheme)) // Hub/Fleet types utilruntime.Must(hubv1.AddToScheme(scheme.Scheme)) diff --git a/pkg/declared/scope.go b/pkg/declared/scope.go index 3931af0fe8..2b8138269a 100644 --- a/pkg/declared/scope.go +++ b/pkg/declared/scope.go @@ -16,6 +16,8 @@ package declared import ( "k8s.io/apimachinery/pkg/util/validation" + "kpt.dev/configsync/pkg/api/configmanagement" + "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/status" ) @@ -41,3 +43,27 @@ func ValidateScope(s string) error { } return nil } + +// ScopeFromSync returns the scope associated with a specific RootSync or RepoSync. +func ScopeFromSync(syncKind, syncNamespace string) Scope { + if syncKind == configsync.RootSyncKind { + return RootReconciler + } + return Scope(syncNamespace) +} + +// SyncNamespaceFromScope returns the namespace associated with a specific scope. +func SyncNamespaceFromScope(syncScope Scope) string { + if syncScope == RootReconciler { + return configmanagement.ControllerNamespace + } + return string(syncScope) +} + +// SyncKindFromScope returns the namespace associated with a specific scope. +func SyncKindFromScope(syncScope Scope) string { + if syncScope == RootReconciler { + return configsync.RootSyncKind + } + return configsync.RepoSyncKind +} diff --git a/pkg/diff/precedence.go b/pkg/diff/precedence.go index 0f376e3dee..49fe1a3d12 100644 --- a/pkg/diff/precedence.go +++ b/pkg/diff/precedence.go @@ -60,10 +60,10 @@ func CanManage(scope declared.Scope, syncName string, obj client.Object, op admi } oldManager := core.GetAnnotation(obj, metadata.ResourceManagerKey) newManager := declared.ResourceManager(scope, syncName) - reconciler, _ := reconcilerName(newManager) + reconciler, _ := ReconcilerNameFromManager(newManager) err := ValidateManager(reconciler, oldManager, core.IDOf(obj), op) if err != nil { - klog.V(3).Infof("diff.CanManage? %v", err) + klog.V(1).Infof("diff.CanManage? %v", err) return false } return true @@ -89,7 +89,7 @@ func ValidateManager(reconciler, manager string, id core.ID, op admissionv1.Oper return nil } - oldReconciler, syncScope := reconcilerName(manager) + oldReconciler, syncScope := ReconcilerNameFromManager(manager) if err := declared.ValidateScope(string(syncScope)); err != nil { // All managers are allowed to manage an object with an invalid manager. @@ -145,7 +145,9 @@ func ValidateManager(reconciler, manager string, id core.ID, op admissionv1.Oper return nil } -func reconcilerName(manager string) (string, declared.Scope) { +// ReconcilerNameFromManager parses the manager annotation value and returns the +// reconciler deployment name and scope. +func ReconcilerNameFromManager(manager string) (string, declared.Scope) { syncScope, syncName := declared.ManagerScopeAndName(manager) var reconciler string if syncScope == declared.RootReconciler { 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..cbadabab2d --- /dev/null +++ b/pkg/metadata/package_id.go @@ -0,0 +1,30 @@ +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..c472de94c4 --- /dev/null +++ b/pkg/metadata/package_id_test.go @@ -0,0 +1,111 @@ +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 1f92df621f..4b426fded9 100644 --- a/pkg/parse/annotations.go +++ b/pkg/parse/annotations.go @@ -18,7 +18,6 @@ import ( "encoding/json" "fmt" - "kpt.dev/configsync/pkg/api/configmanagement" "kpt.dev/configsync/pkg/applier" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/declared" @@ -38,14 +37,13 @@ func addAnnotationsAndLabels(objs []ast.FileObject, scope declared.Scope, syncNa if err != nil { return fmt.Errorf("marshaling sourceContext: %w", err) } - var inventoryID string - if scope == declared.RootReconciler { - inventoryID = applier.InventoryID(syncName, configmanagement.ControllerNamespace) - } else { - inventoryID = applier.InventoryID(syncName, string(scope)) - } + // TODO: Package Ref Label + syncNamespace := declared.SyncNamespaceFromScope(scope) + packageID := metadata.PackageID(syncName, syncNamespace, declared.SyncKindFromScope(scope)) + inventoryID := applier.InventoryID(syncName, syncNamespace) 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, declared.ResourceManager(scope, syncName)) core.SetAnnotation(obj, metadata.SyncTokenAnnotationKey, commitHash) diff --git a/pkg/parse/annotations_test.go b/pkg/parse/annotations_test.go index fc92985267..d2a2d4277a 100644 --- a/pkg/parse/annotations_test.go +++ b/pkg/parse/annotations_test.go @@ -18,14 +18,20 @@ 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.ScopeFromSync(syncKind, syncNamespace) testcases := []struct { name string actual []ast.FileObject @@ -50,11 +56,13 @@ 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/namespace.go b/pkg/parse/namespace.go index 337f62edd2..423465f58a 100644 --- a/pkg/parse/namespace.go +++ b/pkg/parse/namespace.go @@ -21,6 +21,7 @@ import ( "sync" "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/api/configsync/v1beta1" @@ -249,6 +250,38 @@ func (p *namespace) setRenderingStatusWithRetires(ctx context.Context, newStatus return nil } +// SyncStatus implements the Parser interface +// SyncStatus gets the RootSync sync status. +// TODO: figure out how to parse errors... +func (p *namespace) SyncStatus(ctx context.Context) (syncStatus, error) { + p.mux.Lock() + defer p.mux.Unlock() + + rs := &v1beta1.RepoSync{} + if err := p.Client.Get(ctx, reposync.ObjectKey(p.Scope, p.SyncName), rs); err != nil { + return syncStatus{}, status.APIServerError(err, fmt.Sprintf("failed to get the RepoSync object for the %v namespace", p.Scope)) + } + + syncing := false + for _, condition := range rs.Status.Conditions { + if condition.Type == v1beta1.RepoSyncSyncing { + if condition.Status == metav1.ConditionTrue { + syncing = true + } + break + } + } + + return syncStatus{ + syncing: syncing, + commit: rs.Status.Sync.Commit, + objectCount: rs.Status.Sync.ObjectCount, + // TODO: figure out how to parse errors... + // errs: rs.Status.Sync.Errors, + lastUpdate: rs.Status.Sync.LastUpdate, + }, nil +} + // SetSyncStatus implements the Parser interface // SetSyncStatus sets the RepoSync sync status. // `errs` includes the errors encountered during the apply step; @@ -315,19 +348,6 @@ func (p *namespace) setSyncStatusWithRetries(ctx context.Context, newStatus sync return nil } -// SyncErrors returns all the sync errors, including remediator errors, -// validation errors, applier errors, and watch update errors. -// SyncErrors implements the Parser interface -func (p *namespace) SyncErrors() status.MultiError { - return p.Errors() -} - -// Syncing returns true if the updater is running. -// SyncErrors implements the Parser interface -func (p *namespace) Syncing() bool { - return p.Updating() -} - // K8sClient implements the Parser interface func (p *namespace) K8sClient() client.Client { return p.Client diff --git a/pkg/parse/opts.go b/pkg/parse/opts.go index 12750aeefc..41b210c618 100644 --- a/pkg/parse/opts.go +++ b/pkg/parse/opts.go @@ -88,22 +88,23 @@ type Options struct { // Parser represents a parser that can be pointed at and continuously parse a source. type Parser interface { + SyncStatusUpdater + parseSource(ctx context.Context, state sourceState) ([]ast.FileObject, status.MultiError) setSourceStatus(ctx context.Context, newStatus sourceStatus) error setRenderingStatus(ctx context.Context, oldStatus, newStatus renderingStatus) error - SetSyncStatus(ctx context.Context, newStatus syncStatus) error options() *Options - // SyncErrors returns all the sync errors, including remediator errors, - // validation errors, applier errors, and watch update errors. - SyncErrors() status.MultiError - // Syncing returns true if the updater is running. - Syncing() bool - // K8sClient returns the Kubernetes client that talks to the API server. - K8sClient() client.Client // setRequiresRendering sets the requires-rendering annotation on the RSync setRequiresRendering(ctx context.Context, renderingRequired bool) error } +type SyncStatusUpdater interface { + // SetSyncStatus updates the status.sync field of the RSync + SetSyncStatus(ctx context.Context, newStatus syncStatus) error + // SyncStatus gets updates the status.sync field of the RSync + SyncStatus(ctx context.Context) (syncStatus, error) +} + func (o *Options) k8sClient() client.Client { return o.Client } diff --git a/pkg/parse/root.go b/pkg/parse/root.go index 8d2555f8d3..9b59becba3 100644 --- a/pkg/parse/root.go +++ b/pkg/parse/root.go @@ -25,6 +25,7 @@ import ( "github.com/google/go-cmp/cmp" 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" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" @@ -222,6 +223,7 @@ func (p *root) setSourceStatusWithRetries(ctx context.Context, newStatus sourceS func setSourceStatusFields(source *v1beta1.SourceStatus, p Parser, newStatus sourceStatus, denominator int) { cse := status.ToCSE(newStatus.errs) source.Commit = newStatus.commit + source.ObjectCount = newStatus.objectCount switch p.options().SourceType { case v1beta1.GitSource: source.Git = &v1beta1.GitStatus{ @@ -374,13 +376,68 @@ func setRenderingStatusFields(rendering *v1beta1.RenderingStatus, p Parser, newS rendering.LastUpdate = newStatus.lastUpdate } +// SyncStatus implements the Parser interface +// SyncStatus gets the RootSync sync status. +// TODO: figure out how to parse errors... +func (p *root) SyncStatus(ctx context.Context) (syncStatus, error) { + p.mux.Lock() + defer p.mux.Unlock() + + rs := &v1beta1.RootSync{} + if err := p.Client.Get(ctx, rootsync.ObjectKey(p.SyncName), rs); err != nil { + if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) { + return syncStatus{}, nil + } + return syncStatus{}, status.APIServerError(err, "failed to get RootSync") + } + + syncing := false + for _, condition := range rs.Status.Conditions { + if condition.Type == v1beta1.RootSyncSyncing { + if condition.Status == metav1.ConditionTrue { + syncing = true + } + break + } + } + + return syncStatus{ + syncing: syncing, + commit: rs.Status.Sync.Commit, + objectCount: rs.Status.Sync.ObjectCount, + // TODO: figure out how to parse errors... + // errs: rs.Status.Sync.Errors, + lastUpdate: rs.Status.Sync.LastUpdate, + }, nil +} + // SetSyncStatus implements the Parser interface // SetSyncStatus sets the RootSync sync status. // `errs` includes the errors encountered during the apply step; func (p *root) SetSyncStatus(ctx context.Context, newStatus syncStatus) error { p.mux.Lock() defer p.mux.Unlock() - return p.setSyncStatusWithRetries(ctx, newStatus, defaultDenominator) + + if err := p.setSyncStatusWithRetries(ctx, newStatus, defaultDenominator); err != nil { + return err + } + + // Extract conflict errors from sync errors. + var conflictErrs []status.ManagementConflictError + if newStatus.errs != nil { + for _, err := range newStatus.errs.Errors() { + if conflictErr, ok := err.(status.ManagementConflictError); ok { + conflictErrs = append(conflictErrs, conflictErr) + } + } + } + // Report conflict errors to the remote RootSync, if both are RootSyncs. + // RepoSync reconcilers do not have permission to update RootSync status, + // nor RepoSyncs in a different namespace. + if err := reportRootSyncConflicts(ctx, p.K8sClient(), conflictErrs); err != nil { + return fmt.Errorf("failed to report remote conflicts: %w", err) + } + return nil } func (p *root) setSyncStatusWithRetries(ctx context.Context, newStatus syncStatus, denominator int) error { @@ -443,6 +500,7 @@ func (p *root) setSyncStatusWithRetries(ctx context.Context, newStatus syncStatu func setSyncStatusFields(syncStatus *v1beta1.Status, newStatus syncStatus, denominator int) { cse := status.ToCSE(newStatus.errs) syncStatus.Sync.Commit = newStatus.commit + syncStatus.Sync.ObjectCount = newStatus.objectCount syncStatus.Sync.Git = syncStatus.Source.Git syncStatus.Sync.Oci = syncStatus.Source.Oci syncStatus.Sync.Helm = syncStatus.Source.Helm @@ -482,6 +540,38 @@ func summarizeErrors(sourceStatus v1beta1.SourceStatus, syncStatus v1beta1.SyncS return errorSources, errorSummary } +// reportRootSyncConflicts reports conflicts to the RootSync that manages the +// conflicting resources. +func reportRootSyncConflicts(ctx context.Context, k8sClient client.Client, conflictErrs []status.ManagementConflictError) error { + if len(conflictErrs) == 0 { + return nil + } + conflictingManagerErrors := map[string][]status.ManagementConflictError{} + for _, conflictError := range conflictErrs { + conflictingManager := conflictError.ConflictingManager() + err := conflictError.ConflictingManagerError() + conflictingManagerErrors[conflictingManager] = append(conflictingManagerErrors[conflictingManager], err) + } + + for conflictingManager, conflictErrors := range conflictingManagerErrors { + scope, name := declared.ManagerScopeAndName(conflictingManager) + if scope == declared.RootReconciler { + // RootSync applier uses PolicyAdoptAll. + // So it may fight, if the webhook is disabled. + // Report the conflict to the other RootSync to make it easier to detect. + klog.Infof("Detected conflict with RootSync manager %q", conflictingManager) + if err := prependRootSyncRemediatorStatus(ctx, k8sClient, name, conflictErrors, defaultDenominator); err != nil { + return fmt.Errorf("failed to update RootSync %q to prepend remediator conflicts: %w", name, err) + } + } else { + // RepoSync applier uses PolicyAdoptIfNoInventory. + // So it won't fight, even if the webhook is disabled. + klog.Infof("Detected conflict with RepoSync manager %q", conflictingManager) + } + } + return nil +} + // addImplicitNamespaces hydrates the given FileObjects by injecting implicit // namespaces into the list before returning it. Implicit namespaces are those // that are declared by an object's metadata namespace field but are not present @@ -548,19 +638,6 @@ func (p *root) addImplicitNamespaces(objs []ast.FileObject) ([]ast.FileObject, s return objs, errs } -// SyncErrors returns all the sync errors, including remediator errors, -// validation errors, applier errors, and watch update errors. -// SyncErrors implements the Parser interface -func (p *root) SyncErrors() status.MultiError { - return p.Errors() -} - -// Syncing returns true if the updater is running. -// SyncErrors implements the Parser interface -func (p *root) Syncing() bool { - return p.Updating() -} - // K8sClient implements the Parser interface func (p *root) K8sClient() client.Client { return p.Client diff --git a/pkg/parse/root_test.go b/pkg/parse/root_test.go index 5f8f10a676..0db9008666 100644 --- a/pkg/parse/root_test.go +++ b/pkg/parse/root_test.go @@ -108,6 +108,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -120,6 +121,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -145,6 +147,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -161,6 +164,7 @@ 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(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -179,6 +183,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -191,6 +196,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -207,6 +213,7 @@ 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(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -225,6 +232,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -251,6 +259,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -277,6 +286,7 @@ func TestRoot_Parse(t *testing.T) { fake.WithRootSyncSourceType(v1beta1.GitSource), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), core.Label(metadata.DeclaredVersionLabel, "v1beta1"), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, fmt.Sprintf("namespaces/%s/test.yaml", configsync.ControllerNamespace)), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -303,6 +313,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -314,6 +325,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/configmap.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -326,6 +338,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("bar"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -345,6 +358,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -368,6 +382,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -379,6 +394,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -390,6 +406,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/configmap.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -401,6 +418,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -412,6 +430,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/configmap.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -424,6 +443,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("bar"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -436,6 +456,7 @@ func TestRoot_Parse(t *testing.T) { "", core.Name("baz"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -478,10 +499,11 @@ func TestRoot_Parse(t *testing.T) { DiscoveryInterface: syncertest.NewDiscoveryClient(kinds.Namespace(), kinds.Role()), Converter: converter, Updater: Updater{ - Scope: declared.RootReconciler, - Resources: &declared.Resources{}, - Remediator: &remediatorfake.Remediator{}, - Applier: fakeApplier, + Scope: declared.RootReconciler, + Resources: &declared.Resources{}, + Remediator: &remediatorfake.Remediator{}, + Applier: fakeApplier, + StatusUpdatePeriod: configsync.DefaultReconcilerSyncStatusUpdatePeriod, }, mux: &sync.Mutex{}, }, @@ -490,6 +512,8 @@ func TestRoot_Parse(t *testing.T) { NamespaceStrategy: tc.namespaceStrategy, }, } + // Updater uses the root options to known how to update the RootSync sync status. + parser.Options.Updater.SyncStatusUpdater = parser for _, o := range tc.existingObjects { if err := parser.Client.Create(context.Background(), o); err != nil { t.Fatal(err) @@ -550,6 +574,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.DeclaredFieldsKey, `{"f:metadata":{"f:annotations":{},"f:labels":{}},"f:rules":{}}`), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -563,6 +588,7 @@ func TestRoot_DeclaredFields(t *testing.T) { expectedObjsToApply: []ast.FileObject{ fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Label(metadata.DeclaredVersionLabel, "v1"), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), @@ -579,6 +605,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.DeclaredFieldsKey, `{"f:metadata":{"f:annotations":{},"f:labels":{}},"f:rules":{}}`), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -593,6 +620,7 @@ func TestRoot_DeclaredFields(t *testing.T) { expectedObjsToApply: []ast.FileObject{ fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Label(metadata.DeclaredVersionLabel, "v1"), 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"), @@ -611,6 +639,7 @@ func TestRoot_DeclaredFields(t *testing.T) { existingObjects: []client.Object{ fake.NamespaceObject("foo", core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), core.Annotation(metadata.SyncTokenAnnotationKey, ""), @@ -625,6 +654,7 @@ func TestRoot_DeclaredFields(t *testing.T) { expectedObjsToApply: []ast.FileObject{ fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Label(metadata.DeclaredVersionLabel, "v1"), 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"), @@ -643,6 +673,7 @@ func TestRoot_DeclaredFields(t *testing.T) { existingObjects: []client.Object{ fake.NamespaceObject("foo", core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), core.Annotation(metadata.SyncTokenAnnotationKey, ""), @@ -656,6 +687,7 @@ func TestRoot_DeclaredFields(t *testing.T) { expectedObjsToApply: []ast.FileObject{ fake.Role(core.Namespace("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Label(metadata.DeclaredVersionLabel, "v1"), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), @@ -697,10 +729,11 @@ func TestRoot_DeclaredFields(t *testing.T) { Converter: converter, WebhookEnabled: tc.webhookEnabled, Updater: Updater{ - Scope: declared.RootReconciler, - Resources: &declared.Resources{}, - Remediator: &remediatorfake.Remediator{}, - Applier: fakeApplier, + Scope: declared.RootReconciler, + Resources: &declared.Resources{}, + Remediator: &remediatorfake.Remediator{}, + Applier: fakeApplier, + StatusUpdatePeriod: configsync.DefaultReconcilerSyncStatusUpdatePeriod, }, mux: &sync.Mutex{}, }, @@ -708,7 +741,8 @@ func TestRoot_DeclaredFields(t *testing.T) { SourceFormat: filesystem.SourceFormatUnstructured, NamespaceStrategy: configsync.NamespaceStrategyExplicit, }, - } + } // Updater uses the root options to known how to update the RootSync sync status. + parser.Options.Updater.SyncStatusUpdater = parser for _, o := range tc.existingObjects { t.Log("creating obj", o.GetObjectKind().GroupVersionKind().Kind) if err := parser.Client.Create(context.Background(), o); err != nil { @@ -846,6 +880,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -858,6 +893,7 @@ func TestRoot_Parse_Discovery(t *testing.T) { "", core.Name("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -882,6 +918,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "cluster/crd.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -892,6 +929,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/foo/role.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -905,6 +943,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, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(metadata.ResourceManagerKey, ":root_my-rs"), core.Annotation(metadata.SourcePathAnnotationKey, "namespaces/obj.yaml"), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), @@ -917,6 +956,7 @@ func TestRoot_Parse_Discovery(t *testing.T) { "", core.Name("foo"), core.Label(metadata.ManagedByKey, metadata.ManagedByValue), + core.Label(metadata.ParentPackageIDLabel, metadata.PackageID(rootSyncName, configmanagement.ControllerNamespace, configsync.RootSyncKind)), core.Annotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), core.Annotation(metadata.ResourceManagementKey, metadata.ResourceManagementEnabled), core.Annotation(metadata.GitContextKey, nilGitContext), @@ -956,10 +996,11 @@ func TestRoot_Parse_Discovery(t *testing.T) { DiscoveryInterface: tc.discoveryClient, Converter: converter, Updater: Updater{ - Scope: declared.RootReconciler, - Resources: &declared.Resources{}, - Remediator: &remediatorfake.Remediator{}, - Applier: fakeApplier, + Scope: declared.RootReconciler, + Resources: &declared.Resources{}, + Remediator: &remediatorfake.Remediator{}, + Applier: fakeApplier, + StatusUpdatePeriod: configsync.DefaultReconcilerSyncStatusUpdatePeriod, }, mux: &sync.Mutex{}, }, @@ -968,6 +1009,8 @@ func TestRoot_Parse_Discovery(t *testing.T) { NamespaceStrategy: configsync.NamespaceStrategyImplicit, }, } + // Updater uses the root options to known how to update the RootSync sync status. + parser.Options.Updater.SyncStatusUpdater = parser state := &reconcilerState{} err := parseAndUpdate(context.Background(), parser, triggerReimport, state) testerrors.AssertEqual(t, tc.expectedError, err, "expected error to match") @@ -1041,10 +1084,11 @@ func TestRoot_SourceReconcilerErrorsMetricValidation(t *testing.T) { Client: syncertest.NewClient(t, core.Scheme, fake.RootSyncObjectV1Beta1(rootSyncName)), DiscoveryInterface: syncertest.NewDiscoveryClient(kinds.Namespace(), kinds.Role()), Updater: Updater{ - Scope: declared.RootReconciler, - Resources: &declared.Resources{}, - Remediator: &remediatorfake.Remediator{}, - Applier: fakeApplier, + Scope: declared.RootReconciler, + Resources: &declared.Resources{}, + Remediator: &remediatorfake.Remediator{}, + Applier: fakeApplier, + StatusUpdatePeriod: configsync.DefaultReconcilerSyncStatusUpdatePeriod, }, mux: &sync.Mutex{}, }, @@ -1052,6 +1096,8 @@ func TestRoot_SourceReconcilerErrorsMetricValidation(t *testing.T) { SourceFormat: filesystem.SourceFormatUnstructured, }, } + // Updater uses the root options to known how to update the RootSync sync status. + parser.Options.Updater.SyncStatusUpdater = parser state := &reconcilerState{} err := parseAndUpdate(context.Background(), parser, triggerReimport, state) testerrors.AssertEqual(t, tc.expectedError, err, "expected error to match") @@ -1066,13 +1112,15 @@ func TestRoot_SourceReconcilerErrorsMetricValidation(t *testing.T) { func TestRoot_SourceAndSyncReconcilerErrorsMetricValidation(t *testing.T) { testCases := []struct { name string - applyErrors status.MultiError + applyErrors []status.Error expectedError status.MultiError expectedMetrics []*view.Row }{ { - name: "single reconciler error in sync component", - applyErrors: applier.Error(errors.New("sync error")), + name: "single reconciler error in sync component", + applyErrors: []status.Error{ + applier.Error(errors.New("sync error")), + }, expectedError: applier.Error(errors.New("sync error")), expectedMetrics: []*view.Row{ {Data: &view.LastValueData{Value: 0}, Tags: []tag.Tag{{Key: metrics.KeyComponent, Value: "source"}, {Key: metrics.KeyErrorClass, Value: "1xxx"}}}, @@ -1085,10 +1133,10 @@ func TestRoot_SourceAndSyncReconcilerErrorsMetricValidation(t *testing.T) { }, { name: "multiple reconciler errors in sync component", - applyErrors: status.Wrap( + applyErrors: []status.Error{ applier.Error(errors.New("sync error")), status.InternalError("internal error"), - ), + }, expectedError: status.Wrap( applier.Error(errors.New("sync error")), status.InternalError("internal error"), @@ -1121,10 +1169,11 @@ func TestRoot_SourceAndSyncReconcilerErrorsMetricValidation(t *testing.T) { Options: &Options{ Parser: fakeConfigParser, Updater: Updater{ - Scope: declared.RootReconciler, - Resources: &declared.Resources{}, - Remediator: &remediatorfake.Remediator{}, - Applier: fakeApplier, + Scope: declared.RootReconciler, + Resources: &declared.Resources{}, + Remediator: &remediatorfake.Remediator{}, + Applier: fakeApplier, + StatusUpdatePeriod: configsync.DefaultReconcilerSyncStatusUpdatePeriod, }, SyncName: rootSyncName, ReconcilerName: rootReconcilerName, @@ -1136,6 +1185,8 @@ func TestRoot_SourceAndSyncReconcilerErrorsMetricValidation(t *testing.T) { SourceFormat: filesystem.SourceFormatUnstructured, }, } + // Updater uses the root options to known how to update the RootSync sync status. + parser.Options.Updater.SyncStatusUpdater = parser state := &reconcilerState{} err := parseAndUpdate(context.Background(), parser, triggerReimport, state) testerrors.AssertEqual(t, tc.expectedError, err, "expected error to match") diff --git a/pkg/parse/run.go b/pkg/parse/run.go index 95224732fe..db10e85dfb 100644 --- a/pkg/parse/run.go +++ b/pkg/parse/run.go @@ -25,7 +25,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" "kpt.dev/configsync/pkg/core" - "kpt.dev/configsync/pkg/declared" "kpt.dev/configsync/pkg/hydrate" "kpt.dev/configsync/pkg/importer/filesystem/cmpath" "kpt.dev/configsync/pkg/metadata" @@ -34,7 +33,6 @@ import ( "kpt.dev/configsync/pkg/status" "kpt.dev/configsync/pkg/util" webhookconfiguration "kpt.dev/configsync/pkg/webhook/configuration" - "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -82,18 +80,24 @@ func Run(ctx context.Context, p Parser, nsControllerState *namespacecontroller.S // Use timers, not tickers. // Tickers can cause memory leaks and continuous execution, when execution // takes longer than the tick duration. - runTimer := time.NewTimer(opts.PollingPeriod) - defer runTimer.Stop() + // schedule first re-parse + reparseTimer := time.NewTimer(opts.PollingPeriod) + defer reparseTimer.Stop() + + // schedule first re-sync resyncTimer := time.NewTimer(opts.ResyncPeriod) defer resyncTimer.Stop() + // schedule first retry, with backoff & jitter retryTimer := time.NewTimer(opts.RetryPeriod) defer retryTimer.Stop() + // schedule first status update (between sync attempts) statusUpdateTimer := time.NewTimer(opts.StatusUpdatePeriod) defer statusUpdateTimer.Stop() + // schedule first namespace update check nsEventPeriod := time.Second nsEventTimer := time.NewTimer(nsEventPeriod) defer nsEventTimer.Stop() @@ -110,12 +114,12 @@ func Run(ctx context.Context, p Parser, nsControllerState *namespacecontroller.S case <-ctx.Done(): return - // Re-apply even if no changes have been detected. + // Re-sync even if no changes have been detected. // This case should be checked first since it resets the cache. - // If the reconciler is in the process of reconciling a given commit, the resync won't - // happen until the ongoing reconciliation is done. + // If the reconciler is attempting to sync a given commit, the re-sync + // will be delayed until the current sync attempt is done. case <-resyncTimer.C: - klog.Infof("It is time for a force-resync") + klog.Infof("Run loop: re-syncing") // Reset the cache partially to make sure all the steps of a parse-apply-watch loop will run. // The cached sourceState will not be reset to avoid reading all the source files unnecessarily. // The cached needToRetry will not be reset to avoid resetting the backoff retries. @@ -127,21 +131,24 @@ func Run(ctx context.Context, p Parser, nsControllerState *namespacecontroller.S // state of backoff retry. statusUpdateTimer.Reset(opts.StatusUpdatePeriod) // Schedule status update attempt - // Re-import declared resources from the filesystem (from git-sync). - // If the reconciler is in the process of reconciling a given commit, the re-import won't - // happen until the ongoing reconciliation is done. - case <-runTimer.C: + // Re-parse declared resource objects from the filesystem (from git-sync). + // If the reconciler is attempting to sync a given commit, the re-parse + // will be delayed until the current sync attempt is done. + case <-reparseTimer.C: + klog.Infof("Run loop: re-parsing") runFn(ctx, p, triggerReimport, state) - runTimer.Reset(opts.PollingPeriod) // Schedule re-import attempt + reparseTimer.Reset(opts.PollingPeriod) // Schedule re-import attempt // we should not reset retryTimer under this `case` since it is not aware of the // state of backoff retry. statusUpdateTimer.Reset(opts.StatusUpdatePeriod) // Schedule status update attempt // Retry if there was an error, conflict, or any watches need to be updated. case <-retryTimer.C: + klog.Infof("Run loop: re-trying") var trigger string if opts.managementConflict() { + klog.Warning("Management conflict detected. Triggering new sync attempt.") // Reset the cache partially to make sure all the steps of a parse-apply-watch loop will run. // The cached sourceState will not be reset to avoid reading all the source files unnecessarily. // The cached needToRetry will not be reset to avoid resetting the backoff retries. @@ -177,19 +184,24 @@ func Run(ctx context.Context, p Parser, nsControllerState *namespacecontroller.S runFn(ctx, p, trigger, state) // Reset retryTimer after `run` to make sure `retryDuration` happens between the end of one execution // of `run` and the start of the next execution. - retryTimer.Reset(retryDuration) + retryTimer.Reset(retryDuration) // Schedule retry statusUpdateTimer.Reset(opts.StatusUpdatePeriod) // Schedule status update attempt // Update the sync status to report management conflicts (from the remediator). case <-statusUpdateTimer.C: + klog.Infof("Run loop: status update") + // Skip sync status update if state.cache has been reset and not yet + // re-parsed. This avoids removing the last known sync status while + // waiting to parse a new source commit. // Skip sync status update if the .status.sync.commit is out of date. // This avoids overwriting a newer Syncing condition with the status // from an older commit. - if state.syncStatus.commit == state.sourceStatus.commit && + if state.cache.source.commit != "" && + state.syncStatus.commit == state.sourceStatus.commit && state.syncStatus.commit == state.renderingStatus.commit { klog.V(3).Info("Updating sync status (periodic while not syncing)") - if err := setSyncStatus(ctx, p, state, p.Syncing(), p.SyncErrors()); err != nil { + if err := p.options().Updater.SetSyncStatus(ctx, state); err != nil { klog.Warningf("failed to update sync status: %v", err) } } @@ -200,6 +212,7 @@ func Run(ctx context.Context, p Parser, nsControllerState *namespacecontroller.S // Execute the entire parse-apply-watch loop for a namespace event. case <-nsEventTimer.C: + klog.Infof("Run loop: namespace update check") if nsControllerState == nil { // If the Namespace Controller is not running, stop the timer without // closing the channel. @@ -512,9 +525,10 @@ func parseAndUpdate(ctx context.Context, p Parser, trigger string, state *reconc sourceErrs := parseSource(ctx, p, trigger, state) klog.V(3).Info("Parser stopped") newSourceStatus := sourceStatus{ - commit: state.cache.source.commit, - errs: sourceErrs, - lastUpdate: metav1.Now(), + commit: state.cache.source.commit, + objectCount: len(state.cache.objsToApply), + errs: sourceErrs, + lastUpdate: metav1.Now(), } if state.needToSetSourceStatus(newSourceStatus) { klog.V(3).Infof("Updating source status (after parse): %#v", newSourceStatus) @@ -532,24 +546,9 @@ func parseAndUpdate(ctx context.Context, p Parser, trigger string, state *reconc return sourceErrs } - // Create a new context with its cancellation function. - ctxForUpdateSyncStatus, cancel := context.WithCancel(context.Background()) - - go updateSyncStatusPeriodically(ctxForUpdateSyncStatus, p, state) - - klog.V(3).Info("Updater starting...") start := time.Now() - syncErrs := p.options().Update(ctx, &state.cache) + syncErrs := p.options().Update(ctx, state) metrics.RecordParserDuration(ctx, trigger, "update", metrics.StatusTagKey(syncErrs), start) - klog.V(3).Info("Updater stopped") - - // This is to terminate `updateSyncStatusPeriodically`. - cancel() - - klog.V(3).Info("Updating sync status (after sync)") - if err := setSyncStatus(ctx, p, state, false, syncErrs); err != nil { - syncErrs = status.Append(syncErrs, err) - } return status.Append(sourceErrs, syncErrs) } @@ -557,91 +556,21 @@ func parseAndUpdate(ctx context.Context, p Parser, trigger string, state *reconc // setSyncStatus updates `.status.sync` and the Syncing condition, if needed, // as well as `state.syncStatus` and `state.syncingConditionLastUpdate` if // the update is successful. -func setSyncStatus(ctx context.Context, p Parser, state *reconcilerState, syncing bool, syncErrs status.MultiError) error { +func setSyncStatus(ctx context.Context, syncStatusUpdater SyncStatusUpdater, updater *Updater, state *reconcilerState) error { // Update the RSync status, if necessary newSyncStatus := syncStatus{ - syncing: syncing, - commit: state.cache.source.commit, - errs: syncErrs, - lastUpdate: metav1.Now(), + syncing: updater.Updating(), + commit: state.cache.source.commit, + objectCount: updater.InventoryObjectCount(), + errs: updater.Errors(), + lastUpdate: metav1.Now(), } if state.needToSetSyncStatus(newSyncStatus) { - if err := p.SetSyncStatus(ctx, newSyncStatus); err != nil { + if err := syncStatusUpdater.SetSyncStatus(ctx, newSyncStatus); err != nil { return err } state.syncStatus = newSyncStatus state.syncingConditionLastUpdate = newSyncStatus.lastUpdate } - - // Extract conflict errors from sync errors. - var conflictErrs []status.ManagementConflictError - if syncErrs != nil { - for _, err := range syncErrs.Errors() { - if conflictErr, ok := err.(status.ManagementConflictError); ok { - conflictErrs = append(conflictErrs, conflictErr) - } - } - } - // Report conflict errors to the remote manager, if it's a RootSync. - if err := reportRootSyncConflicts(ctx, p.K8sClient(), conflictErrs); err != nil { - return fmt.Errorf("failed to report remote conflicts: %w", err) - } - return nil -} - -// updateSyncStatusPeriodically update the sync status periodically until the -// cancellation function of the context is called. -func updateSyncStatusPeriodically(ctx context.Context, p Parser, state *reconcilerState) { - klog.V(3).Info("Periodic sync status updates starting...") - updatePeriod := p.options().StatusUpdatePeriod - updateTimer := time.NewTimer(updatePeriod) - defer updateTimer.Stop() - for { - select { - case <-ctx.Done(): - // ctx.Done() is closed when the cancellation function of the context is called. - klog.V(3).Info("Periodic sync status updates stopped") - return - - case <-updateTimer.C: - klog.V(3).Info("Updating sync status (periodic while syncing)") - if err := setSyncStatus(ctx, p, state, true, p.SyncErrors()); err != nil { - klog.Warningf("failed to update sync status: %v", err) - } - - updateTimer.Reset(updatePeriod) // Schedule status update attempt - } - } -} - -// reportRootSyncConflicts reports conflicts to the RootSync that manages the -// conflicting resources. -func reportRootSyncConflicts(ctx context.Context, k8sClient client.Client, conflictErrs []status.ManagementConflictError) error { - if len(conflictErrs) == 0 { - return nil - } - conflictingManagerErrors := map[string][]status.ManagementConflictError{} - for _, conflictError := range conflictErrs { - conflictingManager := conflictError.ConflictingManager() - err := conflictError.ConflictingManagerError() - conflictingManagerErrors[conflictingManager] = append(conflictingManagerErrors[conflictingManager], err) - } - - for conflictingManager, conflictErrors := range conflictingManagerErrors { - scope, name := declared.ManagerScopeAndName(conflictingManager) - if scope == declared.RootReconciler { - // RootSync applier uses PolicyAdoptAll. - // So it may fight, if the webhook is disabled. - // Report the conflict to the other RootSync to make it easier to detect. - klog.Infof("Detected conflict with RootSync manager %q", conflictingManager) - if err := prependRootSyncRemediatorStatus(ctx, k8sClient, name, conflictErrors, defaultDenominator); err != nil { - return fmt.Errorf("failed to update RootSync %q to prepend remediator conflicts: %w", name, err) - } - } else { - // RepoSync applier uses PolicyAdoptIfNoInventory. - // So it won't fight, even if the webhook is disabled. - klog.Infof("Detected conflict with RepoSync manager %q", conflictingManager) - } - } return nil } diff --git a/pkg/parse/run_test.go b/pkg/parse/run_test.go index f92294da14..1988060405 100644 --- a/pkg/parse/run_test.go +++ b/pkg/parse/run_test.go @@ -84,6 +84,7 @@ func newParser(t *testing.T, fs FileSource, renderingEnabled bool, retryPeriod t {}, // One Apply call, no errors }, }, + StatusUpdatePeriod: configsync.DefaultReconcilerSyncStatusUpdatePeriod, }, mux: &sync.Mutex{}, RenderingEnabled: renderingEnabled, @@ -91,7 +92,8 @@ func newParser(t *testing.T, fs FileSource, renderingEnabled bool, retryPeriod t ResyncPeriod: 2 * time.Second, PollingPeriod: pollingPeriod, } - + // Updater uses the root options to known how to update the RootSync sync status. + parser.Options.Updater.SyncStatusUpdater = parser return parser } diff --git a/pkg/parse/source_test.go b/pkg/parse/source_test.go index e7a28da047..ad91f0f1c4 100644 --- a/pkg/parse/source_test.go +++ b/pkg/parse/source_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/wait" + "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/declared" "kpt.dev/configsync/pkg/hydrate" @@ -112,8 +113,9 @@ func TestReadConfigFiles(t *testing.T) { Client: syncertest.NewClient(t, core.Scheme, fake.RootSyncObjectV1Beta1(rootSyncName)), DiscoveryInterface: syncertest.NewDiscoveryClient(kinds.Namespace(), kinds.Role()), Updater: Updater{ - Scope: declared.RootReconciler, - Resources: &declared.Resources{}, + Scope: declared.RootReconciler, + Resources: &declared.Resources{}, + StatusUpdatePeriod: configsync.DefaultReconcilerSyncStatusUpdatePeriod, }, mux: &sync.Mutex{}, }, @@ -121,6 +123,8 @@ func TestReadConfigFiles(t *testing.T) { SourceFormat: filesystem.SourceFormatUnstructured, }, } + // Updater uses the root options to known how to update the RootSync sync status. + parser.Options.Updater.SyncStatusUpdater = parser // set the necessary FileSource of parser parser.SourceDir = symDir diff --git a/pkg/parse/state.go b/pkg/parse/state.go index 194904b90c..a353d2a873 100644 --- a/pkg/parse/state.go +++ b/pkg/parse/state.go @@ -24,13 +24,14 @@ import ( ) type sourceStatus struct { - commit string - errs status.MultiError - lastUpdate metav1.Time + commit string + objectCount int + errs status.MultiError + lastUpdate metav1.Time } func (gs sourceStatus) equal(other sourceStatus) bool { - return gs.commit == other.commit && status.DeepEqual(gs.errs, other.errs) + return gs.commit == other.commit && gs.objectCount == other.objectCount && status.DeepEqual(gs.errs, other.errs) } type renderingStatus struct { @@ -48,14 +49,15 @@ func (rs renderingStatus) equal(other renderingStatus) bool { } type syncStatus struct { - syncing bool - commit string - errs status.MultiError - lastUpdate metav1.Time + syncing bool + commit string + objectCount int + errs status.MultiError + lastUpdate metav1.Time } func (gs syncStatus) equal(other syncStatus) bool { - return gs.syncing == other.syncing && gs.commit == other.commit && status.DeepEqual(gs.errs, other.errs) + return gs.syncing == other.syncing && gs.commit == other.commit && gs.objectCount == other.objectCount && status.DeepEqual(gs.errs, other.errs) } type reconcilerState struct { diff --git a/pkg/parse/updater.go b/pkg/parse/updater.go index 322973a52d..20a4a7f74c 100644 --- a/pkg/parse/updater.go +++ b/pkg/parse/updater.go @@ -19,7 +19,9 @@ import ( "sync" "time" + orderedmap "github.com/wk8/go-ordered-map" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/klog/v2" "kpt.dev/configsync/pkg/applier" @@ -45,13 +47,25 @@ type Updater struct { // Applier is a bulk client for applying a set of desired resource objects and // tracking them in a ResourceGroup inventory. Applier applier.Applier + // StatusUpdatePeriod is how often to update the sync status of the RSync + // while Update is running. + StatusUpdatePeriod time.Duration + // SyncStatusUpdater is used to update the RSync sync status while Update is + // running. + SyncStatusUpdater SyncStatusUpdater - errorMux sync.RWMutex - validationErrs status.MultiError - watchErrs status.MultiError + // statusMux prevents race conditions reading and writing status fields. + statusMux sync.RWMutex + validationErrs status.MultiError + applyErrs status.MultiError + watchErrs status.MultiError + inventoryObjectCount int + updating bool + commit string - updateMux sync.RWMutex - updating bool + // updateMux prevents the Update method from executing in parallel. + updateMux sync.RWMutex + initialized bool } func (u *Updater) needToUpdateWatch() bool { @@ -65,55 +79,134 @@ func (u *Updater) managementConflict() bool { // Errors returns the latest known set of errors from the updater. // This method is safe to call while Update is running. func (u *Updater) Errors() status.MultiError { - u.errorMux.RLock() - defer u.errorMux.RUnlock() + u.statusMux.RLock() + defer u.statusMux.RUnlock() + + // Ordering here is important. It needs to be the same as Updater.Update. + var updateErrs status.MultiError + updateErrs = status.Append(updateErrs, u.validationErrs) + updateErrs = status.Append(updateErrs, u.applyErrs) + updateErrs = status.Append(updateErrs, u.watchErrs) + return u.prependRemediatorErrors(updateErrs) +} + +// prependRemediatorErrors combines remediator and updater errors. +func (u *Updater) prependRemediatorErrors(updateErrs status.MultiError) status.MultiError { + updaterConflictErrors, remainingUpdateErrs := extractConflictErrors(updateErrs) + conflictErrs := dedupeConflictErrors(updaterConflictErrors, u.Remediator.ConflictErrors()) + + var fightErrs status.MultiError + for _, fightErr := range u.Remediator.FightErrors() { + fightErrs = status.Append(fightErrs, fightErr) + } var errs status.MultiError - errs = status.Append(errs, u.conflictErrors()) - errs = status.Append(errs, u.fightErrors()) - errs = status.Append(errs, u.validationErrs) - errs = status.Append(errs, u.Applier.Errors()) - errs = status.Append(errs, u.watchErrs) + errs = status.Append(errs, conflictErrs) + errs = status.Append(errs, fightErrs) + errs = status.Append(errs, remainingUpdateErrs) return errs } -// conflictErrors converts []ManagementConflictError into []MultiErrors. -// This method is safe to call while Update is running. -func (u *Updater) conflictErrors() status.MultiError { - var errs status.MultiError - for _, conflictErr := range u.Remediator.ConflictErrors() { - errs = status.Append(errs, conflictErr) +func extractConflictErrors(updateErrs status.MultiError) ([]status.ManagementConflictError, status.MultiError) { + var conflictErrs []status.ManagementConflictError + var remainingErrs status.MultiError + if updateErrs != nil { + for _, updateErr := range updateErrs.Errors() { + if conflictErr, ok := updateErr.(status.ManagementConflictError); ok { + conflictErrs = append(conflictErrs, conflictErr) + } else { + remainingErrs = status.Append(remainingErrs, updateErr) + } + } } - return errs + return conflictErrs, remainingErrs } -// fightErrors converts []Error into []MultiErrors. -// This method is safe to call while Update is running. -func (u *Updater) fightErrors() status.MultiError { - var errs status.MultiError - for _, fightErr := range u.Remediator.FightErrors() { - errs = status.Append(errs, fightErr) +// dedupeConflictErrors de-dupes conflict errors from the remediator and updater. +// Remediator conflict errors take precedence, because they have more detail. +func dedupeConflictErrors(updaterConflictErrors, remediatorConflictErrors []status.ManagementConflictError) status.MultiError { + conflictErrMap := orderedmap.New() + for _, conflictErr := range updaterConflictErrors { + conflictErrMap.Set(conflictErr.ConflictingObjectID(), conflictErr) } - return errs + for _, conflictErr := range remediatorConflictErrors { + conflictErrMap.Set(conflictErr.ConflictingObjectID(), conflictErr) + } + var conflictErrs status.MultiError + for pair := conflictErrMap.Oldest(); pair != nil; pair = pair.Next() { + conflictErrs = status.Append(conflictErrs, pair.Value.(status.ManagementConflictError)) + } + return conflictErrs } func (u *Updater) setValidationErrs(errs status.MultiError) { - u.errorMux.Lock() - defer u.errorMux.Unlock() + u.statusMux.Lock() + defer u.statusMux.Unlock() u.validationErrs = errs } +func (u *Updater) applyErrors() status.MultiError { + u.statusMux.RLock() + defer u.statusMux.RUnlock() + return u.applyErrs +} + +func (u *Updater) addApplyError(err status.Error) { + u.statusMux.Lock() + defer u.statusMux.Unlock() + u.applyErrs = status.Append(u.applyErrs, err) +} + +func (u *Updater) resetApplyErrors() { + u.statusMux.Lock() + defer u.statusMux.Unlock() + u.applyErrs = nil +} + func (u *Updater) setWatchErrs(errs status.MultiError) { - u.errorMux.Lock() - defer u.errorMux.Unlock() + u.statusMux.Lock() + defer u.statusMux.Unlock() u.watchErrs = errs } +func (u *Updater) InventoryObjectCount() int { + u.statusMux.RLock() + defer u.statusMux.RUnlock() + return u.inventoryObjectCount +} + +func (u *Updater) setInventoryObjectCount(count int) { + u.statusMux.Lock() + defer u.statusMux.Unlock() + u.inventoryObjectCount = count +} + // Updating returns true if the Update method is running. func (u *Updater) Updating() bool { + u.statusMux.RLock() + defer u.statusMux.RUnlock() return u.updating } +func (u *Updater) setUpdating(updating bool) { + u.statusMux.Lock() + defer u.statusMux.Unlock() + u.updating = updating +} + +// Commit returns the commit last synced (if not syncing) or being synced (if syncing) +func (u *Updater) Commit() string { + u.statusMux.RLock() + defer u.statusMux.RUnlock() + return u.commit +} + +func (u *Updater) setCommit(commit string) { + u.statusMux.Lock() + defer u.statusMux.Unlock() + u.commit = commit +} + // declaredCRDs returns the list of CRDs which are present in the updater's // declared resources. func (u *Updater) declaredCRDs() ([]*v1beta1.CustomResourceDefinition, status.MultiError) { @@ -143,22 +236,118 @@ func (u *Updater) declaredCRDs() ([]*v1beta1.CustomResourceDefinition, status.Mu // Any errors returned will be prepended with any known conflict errors from the // remediator. This is required to preserve errors that have been reported by // another reconciler. -func (u *Updater) Update(ctx context.Context, cache *cacheForCommit) status.MultiError { +func (u *Updater) Update(ctx context.Context, state *reconcilerState) status.MultiError { u.updateMux.Lock() - u.updating = true + u.setUpdating(true) defer func() { - u.updating = false + u.setUpdating(false) u.updateMux.Unlock() }() - updateErrs := u.update(ctx, cache) + // When the reconciler restarts/reschedules, the Updater state is reset. + // So restore the Updater state from the RSync status. + if !u.initialized { + syncStatus, err := u.SyncStatusUpdater.SyncStatus(ctx) + if err != nil { + updateErrs := status.Append(nil, err) + return u.prependRemediatorErrors(updateErrs) + } + u.setCommit(syncStatus.commit) + u.setInventoryObjectCount(syncStatus.objectCount) + // TODO: Figure out how to categorizes errors into the different kinds of errors + // For now, the errors aren't even being parsed, so we can skip categorizing them. + // This means the errors will always be reset when a new sync starts for + // the first time after the reconciler was restart/rescheduled. + u.initialized = true + } - // Prepend current conflict and fight errors - var errs status.MultiError - errs = status.Append(errs, u.conflictErrors()) - errs = status.Append(errs, u.fightErrors()) - errs = status.Append(errs, updateErrs) - return errs + // Sync the latest commit from source + if u.Commit() != state.cache.source.commit { + u.setCommit(state.cache.source.commit) + } + + klog.V(3).Info("Updating sync status (before sync)") + if err := u.SetSyncStatus(ctx, state); err != nil { + updateErrs := status.Append(nil, err) + return u.prependRemediatorErrors(updateErrs) + } + + // Start periodic sync status updates. + // This allows reporting errors received from applier events while applying. + ctxForUpdateSyncStatus, cancel := context.WithCancel(ctx) + doneCh := u.startPeriodicSyncStatusUpdates(ctxForUpdateSyncStatus, state) + + klog.V(3).Info("Updater starting...") + updateErrs := u.update(ctx, &state.cache) + updateErrs = u.prependRemediatorErrors(updateErrs) + klog.V(3).Info("Updater stopped") + + // Stop periodic sync status updates and wait for completion, to avoid update conflicts. + cancel() + <-doneCh + + u.setUpdating(false) + + klog.V(3).Info("Updating sync status (after sync)") + if err := u.SetSyncStatus(ctx, state); err != nil { + updateErrs = status.Append(updateErrs, err) + } + + return updateErrs +} + +// SetSyncStatus updates `.status.sync` and the Syncing condition, if needed, +// as well as `state.syncStatus` and `state.syncingConditionLastUpdate` if +// the update is successful. +func (u *Updater) SetSyncStatus(ctx context.Context, state *reconcilerState) error { + // Update the RSync status, if necessary + newSyncStatus := syncStatus{ + syncing: u.Updating(), + commit: u.Commit(), + objectCount: u.InventoryObjectCount(), + errs: u.Errors(), + lastUpdate: metav1.Now(), + } + if state.needToSetSyncStatus(newSyncStatus) { + if err := u.SyncStatusUpdater.SetSyncStatus(ctx, newSyncStatus); err != nil { + return err + } + state.syncStatus = newSyncStatus + state.syncingConditionLastUpdate = newSyncStatus.lastUpdate + } + return nil +} + +// startPeriodicSyncStatusUpdates updates the sync status periodically until the +// cancellation function of the context is called. +// The returned done channel will be closed after the background goroutine has +// stopped running. +func (u *Updater) startPeriodicSyncStatusUpdates(ctx context.Context, state *reconcilerState) chan struct{} { + klog.V(3).Info("Periodic sync status updates starting...") + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + updatePeriod := u.StatusUpdatePeriod + updateTimer := time.NewTimer(updatePeriod) + defer updateTimer.Stop() + for { + select { + case <-ctx.Done(): + // ctx.Done() is closed when the cancellation function of the context is called. + klog.V(3).Info("Periodic sync status updates stopped") + return + + case <-updateTimer.C: + klog.V(3).Info("Updating sync status (periodic while syncing)") + if err := u.SetSyncStatus(ctx, state); err != nil { + klog.Warningf("failed to update sync status: %v", err) + } + // Schedule next status update + updateTimer.Reset(updatePeriod) + } + } + }() + return doneCh } // update performs most of the work for `Update`, making it easier to @@ -188,8 +377,19 @@ func (u *Updater) update(ctx context.Context, cache *cacheForCommit) status.Mult // Apply the declared resources if !cache.applied { + // Update InventoryObjectCount after each inventory update + superEventHandler := func(event applier.SuperEvent) { + if inventoryEvent, ok := event.(applier.SuperInventoryEvent); ok { + objectCount := 0 + inv := inventoryEvent.Inventory + if inv != nil { + objectCount = len(inv.Spec.Resources) + } + u.setInventoryObjectCount(objectCount) + } + } declaredObjs, _ := u.Resources.DeclaredObjects() - if err := u.apply(ctx, declaredObjs, cache.source.commit); err != nil { + if err := u.apply(ctx, superEventHandler, declaredObjs, cache.source.commit); err != nil { return err } // Only mark the commit as applied if there were no (non-blocking) parse errors. @@ -235,15 +435,36 @@ func (u *Updater) declare(ctx context.Context, objs []client.Object, commit stri return objs, nil } -func (u *Updater) apply(ctx context.Context, objs []client.Object, commit string) status.MultiError { +func (u *Updater) apply(ctx context.Context, superEventHandler func(applier.SuperEvent), objs []client.Object, commit string) status.MultiError { + // Wrap the event handler to collect errors + var err status.MultiError + wrapper := func(event applier.SuperEvent) { + if errEvent, ok := event.(applier.SuperErrorEvent); ok { + if err == nil { + err = errEvent.Error + } else { + err = status.Append(err, errEvent.Error) + } + u.addApplyError(errEvent.Error) + } + superEventHandler(event) + } klog.V(1).Info("Applier starting...") start := time.Now() - err := u.Applier.Apply(ctx, objs) + u.resetApplyErrors() + objStatusMap, syncStats := u.Applier.Apply(ctx, wrapper, objs) + if syncStats.Empty() { + klog.V(4).Info("Applier made no new progress") + } else { + klog.Infof("Applier made new progress: %s", syncStats.String()) + objStatusMap.Log(klog.V(0)) + } metrics.RecordApplyDuration(ctx, metrics.StatusTagKey(err), commit, start) if err != nil { klog.Warningf("Failed to apply declared resources: %v", err) return err } + klog.V(4).Info("Apply completed without error: all resources are up to date.") klog.V(3).Info("Applier stopped") return nil } diff --git a/pkg/reconciler/finalizer/base_finalizer.go b/pkg/reconciler/finalizer/base_finalizer.go new file mode 100644 index 0000000000..09f7384f69 --- /dev/null +++ b/pkg/reconciler/finalizer/base_finalizer.go @@ -0,0 +1,46 @@ +package finalizer + +import ( + "context" + + "k8s.io/klog/v2" + "kpt.dev/configsync/pkg/applier" + "kpt.dev/configsync/pkg/status" +) + +type baseFinalizer struct { + Destroyer applier.Destroyer +} + +// destroy calls Destroyer.Destroy, collects errors, and handles logging, +// similar to Updater.apply. +func (bf *baseFinalizer) destroy(ctx context.Context) status.MultiError { + var err status.MultiError + superEventHandler := func(event applier.SuperEvent) { + if errEvent, ok := event.(applier.SuperErrorEvent); ok { + if err == nil { + err = errEvent.Error + } else { + err = status.Append(err, errEvent.Error) + } + } + } + klog.V(1).Info("Destroyer starting...") + // start := time.Now() + objStatusMap, syncStats := bf.Destroyer.Destroy(ctx, superEventHandler) + if syncStats.Empty() { + klog.V(4).Info("Destroyer made no new progress") + } else { + klog.Infof("Destroyer made new progress: %s", syncStats.String()) + objStatusMap.Log(klog.V(0)) + } + // TODO: should we have a destroy duration metric? + // metrics.RecordApplyDuration(ctx, metrics.StatusTagKey(errs), commit, start) + if err != nil { + klog.Warningf("Failed to destroy declared resources: %v", err) + return err + } + klog.V(4).Info("Destroyer completed without error: all resources are deleted.") + klog.V(3).Info("Applier stopped") + return nil +} diff --git a/pkg/reconciler/finalizer/finalizer.go b/pkg/reconciler/finalizer/finalizer.go index c2ef932b00..a7b94cac6a 100644 --- a/pkg/reconciler/finalizer/finalizer.go +++ b/pkg/reconciler/finalizer/finalizer.go @@ -37,14 +37,18 @@ type Finalizer interface { func New(scope declared.Scope, destroyer applier.Destroyer, c client.Client, stopControllers context.CancelFunc, controllersStopped <-chan struct{}) Finalizer { if scope == declared.RootReconciler { return &RootSyncFinalizer{ - Destroyer: destroyer, + baseFinalizer: baseFinalizer{ + Destroyer: destroyer, + }, Client: c, StopControllers: stopControllers, ControllersStopped: controllersStopped, } } return &RepoSyncFinalizer{ - Destroyer: destroyer, + baseFinalizer: baseFinalizer{ + Destroyer: destroyer, + }, Client: c, StopControllers: stopControllers, ControllersStopped: controllersStopped, diff --git a/pkg/reconciler/finalizer/reposync_finalizer.go b/pkg/reconciler/finalizer/reposync_finalizer.go index fe5517098e..459b7c5afd 100644 --- a/pkg/reconciler/finalizer/reposync_finalizer.go +++ b/pkg/reconciler/finalizer/reposync_finalizer.go @@ -19,8 +19,8 @@ import ( "fmt" "k8s.io/klog/v2" + "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/api/configsync/v1beta1" - "kpt.dev/configsync/pkg/applier" "kpt.dev/configsync/pkg/reposync" "kpt.dev/configsync/pkg/status" "kpt.dev/configsync/pkg/util/mutate" @@ -31,8 +31,9 @@ import ( // to destroy all managed user objects previously applied from source. // Implements the Finalizer interface. type RepoSyncFinalizer struct { - Destroyer applier.Destroyer - Client client.Client + baseFinalizer + + Client client.Client // StopControllers will be called by the Finalize() method to stop the Parser and Remediator. StopControllers context.CancelFunc @@ -96,7 +97,7 @@ func (f *RepoSyncFinalizer) AddFinalizer(ctx context.Context, syncObj client.Obj return &mutate.NoUpdateError{} } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to add finalizer: %w", err) } @@ -119,7 +120,7 @@ func (f *RepoSyncFinalizer) RemoveFinalizer(ctx context.Context, syncObj client. return &mutate.NoUpdateError{} } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to remove finalizer: %w", err) } @@ -140,7 +141,7 @@ func (f *RepoSyncFinalizer) setFinalizingCondition(ctx context.Context, syncObj return &mutate.NoUpdateError{} } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to set ReconcilerFinalizing condition: %w", err) } @@ -161,7 +162,7 @@ func (f *RepoSyncFinalizer) removeFinalizingCondition(ctx context.Context, syncO return &mutate.NoUpdateError{} } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to remove ReconcilerFinalizing condition: %w", err) } @@ -176,7 +177,7 @@ func (f *RepoSyncFinalizer) removeFinalizingCondition(ctx context.Context, syncO // deleteManagedObjects uses the destroyer to delete managed objects and then // updates the ReconcilerFinalizerFailure condition on the specified object. func (f *RepoSyncFinalizer) deleteManagedObjects(ctx context.Context, syncObj *v1beta1.RepoSync) error { - destroyErrs := f.Destroyer.Destroy(ctx) + destroyErrs := f.destroy(ctx) // Update the FinalizerFailure condition whether the destroy succeeded or failed if _, updateErr := f.updateFailureCondition(ctx, syncObj, destroyErrs); updateErr != nil { updateErr = fmt.Errorf("updating FinalizerFailure condition: %w", updateErr) @@ -207,7 +208,7 @@ func (f *RepoSyncFinalizer) updateFailureCondition(ctx context.Context, syncObj } } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to set ReconcilerFinalizerFailure condition: %w", err) } diff --git a/pkg/reconciler/finalizer/reposync_finalizer_test.go b/pkg/reconciler/finalizer/reposync_finalizer_test.go index 22b1899780..b54501c6bf 100644 --- a/pkg/reconciler/finalizer/reposync_finalizer_test.go +++ b/pkg/reconciler/finalizer/reposync_finalizer_test.go @@ -294,7 +294,9 @@ func TestRepoSyncFinalize(t *testing.T) { } fakeDestroyer := newFakeDestroyer(tc.destroyErrs, destroyFunc) finalizer := &RepoSyncFinalizer{ - Destroyer: fakeDestroyer, + baseFinalizer: baseFinalizer{ + Destroyer: fakeDestroyer, + }, Client: fakeClient, StopControllers: stopFunc, ControllersStopped: continueCh, diff --git a/pkg/reconciler/finalizer/rootsync_finalizer.go b/pkg/reconciler/finalizer/rootsync_finalizer.go index 38ebcaa002..e3c53da836 100644 --- a/pkg/reconciler/finalizer/rootsync_finalizer.go +++ b/pkg/reconciler/finalizer/rootsync_finalizer.go @@ -19,8 +19,8 @@ import ( "fmt" "k8s.io/klog/v2" + "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/api/configsync/v1beta1" - "kpt.dev/configsync/pkg/applier" "kpt.dev/configsync/pkg/rootsync" "kpt.dev/configsync/pkg/status" "kpt.dev/configsync/pkg/util/mutate" @@ -31,8 +31,9 @@ import ( // to destroy all managed user objects previously applied from source. // Implements the Finalizer interface. type RootSyncFinalizer struct { - Destroyer applier.Destroyer - Client client.Client + baseFinalizer + + Client client.Client // StopControllers will be called by the Finalize() method to stop the Parser and Remediator. StopControllers context.CancelFunc @@ -96,7 +97,7 @@ func (f *RootSyncFinalizer) AddFinalizer(ctx context.Context, syncObj client.Obj return &mutate.NoUpdateError{} } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to add finalizer: %w", err) } @@ -119,7 +120,7 @@ func (f *RootSyncFinalizer) RemoveFinalizer(ctx context.Context, syncObj client. return &mutate.NoUpdateError{} } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to remove finalizer: %w", err) } @@ -140,7 +141,7 @@ func (f *RootSyncFinalizer) setFinalizingCondition(ctx context.Context, syncObj return &mutate.NoUpdateError{} } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to set ReconcilerFinalizing condition: %w", err) } @@ -161,7 +162,7 @@ func (f *RootSyncFinalizer) removeFinalizingCondition(ctx context.Context, syncO return &mutate.NoUpdateError{} } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to remove ReconcilerFinalizing condition: %w", err) } @@ -176,7 +177,7 @@ func (f *RootSyncFinalizer) removeFinalizingCondition(ctx context.Context, syncO // deleteManagedObjects uses the destroyer to delete managed objects and then // updates the ReconcilerFinalizerFailure condition on the specified object. func (f *RootSyncFinalizer) deleteManagedObjects(ctx context.Context, syncObj *v1beta1.RootSync) error { - destroyErrs := f.Destroyer.Destroy(ctx) + destroyErrs := f.destroy(ctx) // Update the FinalizerFailure condition whether the destroy succeeded or failed if _, updateErr := f.updateFailureCondition(ctx, syncObj, destroyErrs); updateErr != nil { updateErr = fmt.Errorf("updating FinalizerFailure condition: %w", updateErr) @@ -207,7 +208,7 @@ func (f *RootSyncFinalizer) updateFailureCondition(ctx context.Context, syncObj } } return nil - }) + }, client.FieldOwner(configsync.FieldManager)) if err != nil { return updated, fmt.Errorf("failed to set ReconcilerFinalizerFailure condition: %w", err) } diff --git a/pkg/reconciler/finalizer/rootsync_finalizer_test.go b/pkg/reconciler/finalizer/rootsync_finalizer_test.go index af44399d61..24c147b67d 100644 --- a/pkg/reconciler/finalizer/rootsync_finalizer_test.go +++ b/pkg/reconciler/finalizer/rootsync_finalizer_test.go @@ -28,7 +28,9 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" "kpt.dev/configsync/pkg/api/configsync/v1beta1" + "kpt.dev/configsync/pkg/api/kpt.dev/v1alpha1" "kpt.dev/configsync/pkg/applier" + "kpt.dev/configsync/pkg/applier/stats" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/kinds" "kpt.dev/configsync/pkg/metadata" @@ -300,7 +302,9 @@ func TestRootSyncFinalize(t *testing.T) { } fakeDestroyer := newFakeDestroyer(tc.destroyErrs, destroyFunc) finalizer := &RootSyncFinalizer{ - Destroyer: fakeDestroyer, + baseFinalizer: baseFinalizer{ + Destroyer: fakeDestroyer, + }, Client: fakeClient, StopControllers: stopFunc, ControllersStopped: continueCh, @@ -632,13 +636,38 @@ func newFakeDestroyer(errs status.MultiError, destroyFunc func(context.Context) } } -func (d *fakeDestroyer) Destroy(ctx context.Context) status.MultiError { +func (d *fakeDestroyer) Destroy(ctx context.Context, superEventHandler func(applier.SuperEvent)) (applier.ObjectStatusMap, *stats.SyncStats) { + var errs status.MultiError if d.destroyFunc != nil { - return d.destroyFunc(ctx) + errs = d.destroyFunc(ctx) + } else { + errs = d.errs } - return d.errs -} - -func (d *fakeDestroyer) Errors() status.MultiError { - return d.errs + superEventHandler(applier.SuperInventoryEvent{ + Inventory: &v1alpha1.ResourceGroup{ + Spec: v1alpha1.ResourceGroupSpec{ + Resources: []v1alpha1.ObjMetadata{ + // TODO: test InventoryObjectCount + }, + }, + }, + }) + if errs != nil { + for _, err := range errs.Errors() { + superEventHandler(applier.SuperErrorEvent{ + Error: err, + }) + } + } + superEventHandler(applier.SuperInventoryEvent{ + Inventory: &v1alpha1.ResourceGroup{ + Spec: v1alpha1.ResourceGroupSpec{ + Resources: []v1alpha1.ObjMetadata{ + // TODO: test InventoryObjectCount + }, + }, + }, + }) + // TODO: test ObjectStatusMap & SyncStats + return applier.ObjectStatusMap{}, &stats.SyncStats{} } diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 724bb6b61a..82994af294 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -77,9 +77,6 @@ type Options struct { // PollingPeriod is the period of time between checking the filesystem for // source updates to sync. PollingPeriod time.Duration - // RetryPeriod is the period of time between checking the filesystem for - // source updates to sync, after an error. - RetryPeriod time.Duration // StatusUpdatePeriod is how long the parser waits between updates of the // sync status, to account for management conflict errors from the remediator. StatusUpdatePeriod time.Duration @@ -105,7 +102,7 @@ type Options struct { // SyncDir is the relative path to the configurations in the source. SyncDir cmpath.Relative // StatusMode controls the kpt applier to inject the actuation status data or not - StatusMode string + StatusMode applier.InventoryStatusMode // ReconcileTimeout controls the reconcile/prune Timeout in kpt applier ReconcileTimeout string // APIServerTimeout is the client-side timeout used for talking to the API server @@ -194,7 +191,7 @@ 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) + clientSet, err := applier.NewClientSet(cl, configFlags, opts.StatusMode, opts.SyncName, opts.ReconcilerScope) if err != nil { klog.Fatalf("Error creating clients: %v", err) } @@ -241,17 +238,17 @@ func Run(opts Options) { SyncName: opts.SyncName, PollingPeriod: opts.PollingPeriod, ResyncPeriod: opts.ResyncPeriod, - RetryPeriod: opts.RetryPeriod, StatusUpdatePeriod: opts.StatusUpdatePeriod, DiscoveryInterface: discoveryClient, RenderingEnabled: opts.RenderingEnabled, Files: parse.Files{FileSource: fs}, WebhookEnabled: opts.WebhookEnabled, Updater: parse.Updater{ - Scope: opts.ReconcilerScope, - Resources: decls, - Applier: supervisor, - Remediator: rem, + Scope: opts.ReconcilerScope, + Resources: decls, + Applier: supervisor, + Remediator: rem, + StatusUpdatePeriod: opts.StatusUpdatePeriod, }, } // Only instantiate the converter when the webhook is enabled because the @@ -285,6 +282,9 @@ func Run(opts Options) { } } + // Updater needs the type-specific parser to known how to update the RSync sync status. + parseOpts.Updater.SyncStatusUpdater = parser + // Start listening to signals signalCtx := signals.SetupSignalHandler() diff --git a/pkg/reconcilermanager/constants.go b/pkg/reconcilermanager/constants.go index f69d4d4c3a..87193af8b3 100644 --- a/pkg/reconcilermanager/constants.go +++ b/pkg/reconcilermanager/constants.go @@ -14,9 +14,15 @@ package reconcilermanager +import "kpt.dev/configsync/pkg/api/configsync" + const ( // ManagerName is the name of the controller which creates reconcilers. ManagerName = "reconciler-manager" + + // FieldManager is the field manager used by the reconciler-manager. + // This avoids conflicts between the reconciler and reconciler-manager. + FieldManager = configsync.ConfigSyncPrefix + ManagerName ) const ( diff --git a/pkg/reconcilermanager/controllers/reconciler_base.go b/pkg/reconcilermanager/controllers/reconciler_base.go index 9cd35e5e34..3399ccecf0 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/reconciler_container_resources.go b/pkg/reconcilermanager/controllers/reconciler_container_resources.go index 0c3ca7a849..ddcbc786ef 100644 --- a/pkg/reconcilermanager/controllers/reconciler_container_resources.go +++ b/pkg/reconcilermanager/controllers/reconciler_container_resources.go @@ -77,10 +77,10 @@ func ReconcilerContainerResourceDefaultsForAutopilot() map[string]v1beta1.Contai return map[string]v1beta1.ContainerResourcesSpec{ reconcilermanager.Reconciler: { ContainerName: reconcilermanager.Reconciler, - CPURequest: resource.MustParse("700m"), - CPULimit: resource.MustParse("700m"), - MemoryRequest: resource.MustParse("512Mi"), - MemoryLimit: resource.MustParse("512Mi"), + CPURequest: resource.MustParse("125m"), + CPULimit: resource.MustParse("125m"), + MemoryRequest: resource.MustParse("128Mi"), + MemoryLimit: resource.MustParse("128Mi"), }, reconcilermanager.HydrationController: { ContainerName: reconcilermanager.HydrationController, diff --git a/pkg/reconcilermanager/controllers/reposync_controller.go b/pkg/reconcilermanager/controllers/reposync_controller.go index dbb336354f..7632db62d1 100644 --- a/pkg/reconcilermanager/controllers/reposync_controller.go +++ b/pkg/reconcilermanager/controllers/reposync_controller.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "math" "slices" "strings" "sync" @@ -31,6 +32,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation" @@ -39,6 +41,7 @@ import ( "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/api/configsync/v1beta1" hubv1 "kpt.dev/configsync/pkg/api/hub/v1" + "kpt.dev/configsync/pkg/applier" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/declared" "kpt.dev/configsync/pkg/kinds" @@ -47,6 +50,7 @@ import ( "kpt.dev/configsync/pkg/reconcilermanager" "kpt.dev/configsync/pkg/reposync" "kpt.dev/configsync/pkg/status" + "kpt.dev/configsync/pkg/util/bytes" "kpt.dev/configsync/pkg/util/compare" "kpt.dev/configsync/pkg/util/mutate" "kpt.dev/configsync/pkg/validate/raw/validate" @@ -320,11 +324,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. @@ -897,7 +905,7 @@ func (r *RepoSyncReconciler) populateContainerEnvs(ctx context.Context, rs *v1be ociConfig: rs.Spec.Oci, helmConfig: reposync.GetHelmBase(rs.Spec.Helm), pollPeriod: r.reconcilerPollingPeriod.String(), - statusMode: rs.Spec.SafeOverride().StatusMode, + statusMode: applier.InventoryStatusMode(rs.Spec.SafeOverride().StatusMode), reconcileTimeout: v1beta1.GetReconcileTimeout(rs.Spec.SafeOverride().ReconcileTimeout), apiServerTimeout: v1beta1.GetAPIServerTimeout(rs.Spec.SafeOverride().APIServerTimeout), requiresRendering: annotationEnabled(metadata.RequiresRenderingAnnotationKey, rs.GetAnnotations()), @@ -1103,7 +1111,7 @@ func (r *RepoSyncReconciler) updateSyncStatus(ctx context.Context, rs *v1beta1.R cmp.Diff(before.Status, rs.Status))) } return nil - }) + }, client.FieldOwner(reconcilermanager.FieldManager)) if err != nil { return updated, fmt.Errorf("Sync status update failed: %w", err) } @@ -1183,6 +1191,32 @@ func (r *RepoSyncReconciler) mutationsFor(ctx context.Context, rs *v1beta1.RepoS var containerResourceDefaults map[string]v1beta1.ContainerResourcesSpec if autopilot { containerResourceDefaults = ReconcilerContainerResourceDefaultsForAutopilot() + // Use the larger of the old or new source size for scaling. + // This helps keep memory high for large additions. + objectCount := rs.Status.Sync.ObjectCount + if rs.Status.Source.ObjectCount > objectCount { + objectCount = rs.Status.Source.ObjectCount + } + + memoryMinimum := float64(128) + memoryStepSize := float64(32) + memoryUsage := float64(objectCount)/5 + memoryMinimum + memoryLimit := math.Max(math.Ceil(memoryUsage/memoryStepSize)*memoryStepSize, memoryMinimum) + memoryQuantity := resource.NewQuantity(int64(memoryLimit*float64(bytes.MiB)), resource.BinarySI) + klog.V(3).Infof("%s reconciler container memory: %s - %s", r.syncKind, memoryQuantity, memoryQuantity) + + cpuMinimum := float64(125) + cpuRatio := float64(bytes.KB) / float64(bytes.KiB) + cpuLimit := math.Max(math.Ceil(memoryLimit*cpuRatio), cpuMinimum) + cpuQuantity := resource.NewMilliQuantity(int64(cpuLimit), resource.DecimalSI) + klog.V(3).Infof("%s reconciler container cpu: %s - %s", r.syncKind, cpuQuantity, cpuQuantity) + + defaults := containerResourceDefaults[reconcilermanager.Reconciler] + defaults.CPURequest = *cpuQuantity + defaults.CPULimit = *cpuQuantity + defaults.MemoryRequest = *memoryQuantity + defaults.MemoryLimit = *memoryQuantity + containerResourceDefaults[reconcilermanager.Reconciler] = defaults } else { containerResourceDefaults = ReconcilerContainerResourceDefaults() } diff --git a/pkg/reconcilermanager/controllers/rootsync_controller.go b/pkg/reconcilermanager/controllers/rootsync_controller.go index 4f841755af..de76bf6304 100644 --- a/pkg/reconcilermanager/controllers/rootsync_controller.go +++ b/pkg/reconcilermanager/controllers/rootsync_controller.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "math" "strconv" "strings" "sync" @@ -32,6 +33,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -41,6 +43,7 @@ import ( "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/api/configsync/v1beta1" hubv1 "kpt.dev/configsync/pkg/api/hub/v1" + "kpt.dev/configsync/pkg/applier" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/declared" "kpt.dev/configsync/pkg/kinds" @@ -49,6 +52,7 @@ import ( "kpt.dev/configsync/pkg/reconcilermanager" "kpt.dev/configsync/pkg/rootsync" "kpt.dev/configsync/pkg/status" + "kpt.dev/configsync/pkg/util/bytes" "kpt.dev/configsync/pkg/util/compare" "kpt.dev/configsync/pkg/util/mutate" "kpt.dev/configsync/pkg/validate/raw/validate" @@ -269,11 +273,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. @@ -773,7 +781,7 @@ func (r *RootSyncReconciler) populateContainerEnvs(ctx context.Context, rs *v1be ociConfig: rs.Spec.Oci, helmConfig: rootsync.GetHelmBase(rs.Spec.Helm), pollPeriod: r.reconcilerPollingPeriod.String(), - statusMode: rs.Spec.SafeOverride().StatusMode, + statusMode: applier.InventoryStatusMode(rs.Spec.SafeOverride().StatusMode), reconcileTimeout: v1beta1.GetReconcileTimeout(rs.Spec.SafeOverride().ReconcileTimeout), apiServerTimeout: v1beta1.GetAPIServerTimeout(rs.Spec.SafeOverride().APIServerTimeout), requiresRendering: annotationEnabled(metadata.RequiresRenderingAnnotationKey, rs.GetAnnotations()), @@ -1138,7 +1146,7 @@ func (r *RootSyncReconciler) updateSyncStatus(ctx context.Context, rs *v1beta1.R cmp.Diff(before.Status, rs.Status))) } return nil - }) + }, client.FieldOwner(reconcilermanager.FieldManager)) if err != nil { return updated, fmt.Errorf("Sync status update failed: %w", err) } @@ -1236,6 +1244,32 @@ func (r *RootSyncReconciler) mutationsFor(ctx context.Context, rs *v1beta1.RootS var containerResourceDefaults map[string]v1beta1.ContainerResourcesSpec if autopilot { containerResourceDefaults = ReconcilerContainerResourceDefaultsForAutopilot() + // Use the larger of the old or new source size for scaling. + // This helps keep memory high for large additions. + objectCount := rs.Status.Sync.ObjectCount + if rs.Status.Source.ObjectCount > objectCount { + objectCount = rs.Status.Source.ObjectCount + } + + memoryMinimum := float64(128) + memoryStepSize := float64(32) + memoryUsage := float64(objectCount)/5 + memoryMinimum + memoryLimit := math.Max(math.Ceil(memoryUsage/memoryStepSize)*memoryStepSize, memoryMinimum) + memoryQuantity := resource.NewQuantity(int64(memoryLimit*float64(bytes.MiB)), resource.BinarySI) + klog.V(3).Infof("%s reconciler container memory: %s - %s", r.syncKind, memoryQuantity, memoryQuantity) + + cpuMinimum := float64(125) + cpuRatio := float64(bytes.KB) / float64(bytes.KiB) + cpuLimit := math.Max(math.Ceil(memoryLimit*cpuRatio), cpuMinimum) + cpuQuantity := resource.NewMilliQuantity(int64(cpuLimit), resource.DecimalSI) + klog.V(3).Infof("%s reconciler container cpu: %s - %s", r.syncKind, cpuQuantity, cpuQuantity) + + defaults := containerResourceDefaults[reconcilermanager.Reconciler] + defaults.CPURequest = *cpuQuantity + defaults.CPULimit = *cpuQuantity + defaults.MemoryRequest = *memoryQuantity + defaults.MemoryLimit = *memoryQuantity + containerResourceDefaults[reconcilermanager.Reconciler] = defaults } else { containerResourceDefaults = ReconcilerContainerResourceDefaults() } diff --git a/pkg/reconcilermanager/controllers/util.go b/pkg/reconcilermanager/controllers/util.go index 9103c63100..efcc19af88 100644 --- a/pkg/reconcilermanager/controllers/util.go +++ b/pkg/reconcilermanager/controllers/util.go @@ -103,7 +103,7 @@ type reconcilerOptions struct { ociConfig *v1beta1.Oci helmConfig *v1beta1.HelmBase pollPeriod string - statusMode string + statusMode applier.InventoryStatusMode reconcileTimeout string apiServerTimeout string requiresRendering bool @@ -188,7 +188,7 @@ func reconcilerEnvs(opts reconcilerOptions) []corev1.EnvVar { }, corev1.EnvVar{ Name: reconcilermanager.StatusMode, - Value: statusMode, + Value: statusMode.String(), }, corev1.EnvVar{ Name: reconcilermanager.ReconcileTimeout, diff --git a/pkg/remediator/conflict/handler.go b/pkg/remediator/conflict/handler.go index a48a449491..b8c26655fe 100644 --- a/pkg/remediator/conflict/handler.go +++ b/pkg/remediator/conflict/handler.go @@ -32,11 +32,14 @@ type Handler interface { // ConflictErrors returns the management conflict errors (KNV1060) the remediator encounters. ConflictErrors() []status.ManagementConflictError + // HasConflictErrors returns true if conflicts exist. + HasConflictErrors() bool } // handler implements Handler. type handler struct { // mux guards the conflictErrs + // TODO: replace with RWMutex mux sync.Mutex // conflictErrs tracks all the conflict errors (KNV1060) the remediator encounters, // and report to RootSync|RepoSync status. @@ -90,3 +93,11 @@ func (h *handler) ConflictErrors() []status.ManagementConflictError { } return result } + +// HasConflictErrors returns true if conflicts exist. +func (h *handler) HasConflictErrors() bool { + h.mux.Lock() + defer h.mux.Unlock() + + return h.conflictErrs.Len() > 0 +} diff --git a/pkg/remediator/reconcile/reconciler.go b/pkg/remediator/reconcile/reconciler.go index 0dfa67fbf1..825445c5aa 100644 --- a/pkg/remediator/reconcile/reconciler.go +++ b/pkg/remediator/reconcile/reconciler.go @@ -25,6 +25,8 @@ import ( "kpt.dev/configsync/pkg/importer/analyzer/validation/nonhierarchical" "kpt.dev/configsync/pkg/metadata" "kpt.dev/configsync/pkg/metrics" + "kpt.dev/configsync/pkg/remediator/conflict" + "kpt.dev/configsync/pkg/remediator/queue" "kpt.dev/configsync/pkg/status" syncerclient "kpt.dev/configsync/pkg/syncer/client" syncerreconcile "kpt.dev/configsync/pkg/syncer/reconcile" @@ -47,7 +49,8 @@ type reconciler struct { // declared is the threadsafe in-memory representation of declared configuration. declared *declared.Resources - fightHandler fight.Handler + conflictHandler conflict.Handler + fightHandler fight.Handler } // newReconciler instantiates a new reconciler. @@ -56,14 +59,16 @@ func newReconciler( syncName string, applier syncerreconcile.Applier, declared *declared.Resources, + conflictHandler conflict.Handler, fightHandler fight.Handler, ) *reconciler { return &reconciler{ - scope: scope, - syncName: syncName, - applier: applier, - declared: declared, - fightHandler: fightHandler, + scope: scope, + syncName: syncName, + applier: applier, + declared: declared, + conflictHandler: conflictHandler, + fightHandler: fightHandler, } } @@ -85,6 +90,15 @@ func (r *reconciler) Remediate(ctx context.Context, id core.ID, obj client.Objec Actual: obj, } + // Build GVKNN from Diff, if possible + gvknn := queue.GVKNN{ID: id} + if objDiff.Actual != nil { + gvknn.Version = objDiff.Actual.GetObjectKind().GroupVersionKind().Version + } else if objDiff.Declared != nil { + gvknn.Version = objDiff.Declared.GetObjectKind().GroupVersionKind().Version + } + // else: GVKNN won't match any previous conflicts, so RemoveConflictError will be a No Op. + err := r.remediate(ctx, id, objDiff) // Record duration, even if there's an error @@ -95,6 +109,10 @@ func (r *reconciler) Remediate(ctx context.Context, id core.ID, obj client.Objec case syncerclient.ResourceConflictCode: // Record conflict, if there was one metrics.RecordResourceConflict(ctx, commit) + case status.ManagementConflictErrorCode: + // TODO: Casting will panic if another status.Error wraps the ManagementConflictError. Is that possible here? + r.conflictHandler.AddConflictError(gvknn, err.(status.ManagementConflictError)) + metrics.RecordResourceConflict(ctx, commit) case status.FightErrorCode: operation := objDiff.Operation(r.scope, r.syncName) metrics.RecordResourceFight(ctx, string(operation)) @@ -103,6 +121,9 @@ func (r *reconciler) Remediate(ctx context.Context, id core.ID, obj client.Objec return err } + // TODO: Convert RemoveConflictError to use ID, instead of GVKNN + // TODO: Should we resolve conflicts in the watcher or the worker? + // r.conflictHandler.RemoveConflictError(gvknn) r.fightHandler.RemoveFightError(id) return nil } @@ -113,6 +134,11 @@ func (r *reconciler) remediate(ctx context.Context, id core.ID, objDiff diff.Dif switch t := objDiff.Operation(r.scope, r.syncName); t { case diff.NoOp: return nil + case diff.ManagementConflict: + oldManager := core.GetAnnotation(objDiff.Actual, metadata.ResourceManagerKey) + newManager := declared.ResourceManager(r.scope, r.syncName) + klog.Warningf("Remediator skipping object %v: management conflict detected (current: %s, desired: %s)", id, oldManager, newManager) + return status.ManagementConflictErrorWrap(objDiff.Actual, newManager) case diff.Create: declared, err := objDiff.UnstructuredDeclared() if err != nil { diff --git a/pkg/remediator/reconcile/reconciler_test.go b/pkg/remediator/reconcile/reconciler_test.go index cbc894bc84..282d74f5e3 100644 --- a/pkg/remediator/reconcile/reconciler_test.go +++ b/pkg/remediator/reconcile/reconciler_test.go @@ -243,7 +243,8 @@ func TestRemediator_Reconcile(t *testing.T) { // Simulate the Parser having already parsed the resource and recorded it. d := makeDeclared(t, "unused", tc.declared) - r := newReconciler(declared.RootReconciler, configsync.RootSyncName, c.Applier(), d, testingfake.NewFightHandler()) + r := newReconciler(declared.RootReconciler, configsync.RootSyncName, c.Applier(), d, + testingfake.NewConflictHandler(), testingfake.NewFightHandler()) // Get the triggering object for the reconcile event. var obj client.Object @@ -365,7 +366,8 @@ func TestRemediator_Reconcile_Metrics(t *testing.T) { fakeApplier.UpdateError = tc.updateError fakeApplier.DeleteError = tc.deleteError - reconciler := newReconciler(declared.RootReconciler, configsync.RootSyncName, fakeApplier, d, testingfake.NewFightHandler()) + reconciler := newReconciler(declared.RootReconciler, configsync.RootSyncName, fakeApplier, d, + testingfake.NewConflictHandler(), testingfake.NewFightHandler()) // Get the triggering object for the reconcile event. var obj client.Object diff --git a/pkg/remediator/reconcile/worker.go b/pkg/remediator/reconcile/worker.go index 7e1fd38723..2895671757 100644 --- a/pkg/remediator/reconcile/worker.go +++ b/pkg/remediator/reconcile/worker.go @@ -25,6 +25,7 @@ import ( "k8s.io/klog/v2" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/declared" + "kpt.dev/configsync/pkg/remediator/conflict" "kpt.dev/configsync/pkg/remediator/queue" "kpt.dev/configsync/pkg/status" syncerclient "kpt.dev/configsync/pkg/syncer/client" @@ -42,10 +43,10 @@ type Worker struct { // NewWorker returns a new Worker for the given queue and declared resources. func NewWorker(scope declared.Scope, syncName string, a syncerreconcile.Applier, - q *queue.ObjectQueue, d *declared.Resources, fh fight.Handler) *Worker { + q *queue.ObjectQueue, d *declared.Resources, ch conflict.Handler, fh fight.Handler) *Worker { return &Worker{ objectQueue: q, - reconciler: newReconciler(scope, syncName, a, d, fh), + reconciler: newReconciler(scope, syncName, a, d, ch, fh), } } @@ -99,9 +100,32 @@ func (w *Worker) process(ctx context.Context, obj client.Object) error { id := core.IDOf(obj) var toRemediate client.Object if queue.WasDeleted(ctx, obj) { - // Passing a nil Object to the reconciler signals that the accompanying ID - // is for an Object that was deleted. - toRemediate = nil + // Casting should never panic, because WasDeleted already checked. + // The object should never be nil, otherwise the event handler would have errored. + objFromEvent := obj.(*queue.Deleted).Object + // Double-check whether the object still exists on the cluster. + // This is needed because watches with label filters send delete events + // when a required label is changed or removed. If the object still + // exists, either it was updated or deleted and recreated. If not, it + // means the object was actually deleted and hasn't been recreated + // while waiting to process the event. + latestObj, err := w.getObject(ctx, objFromEvent) + if err != nil { + // Failed to confirm object state. Enqueue for retry. + w.objectQueue.Retry(obj) + return fmt.Errorf("failed to remediate %q: %w", id, err) + } + if queue.WasDeleted(ctx, latestObj) { + // Confirmed not on the server. + // Passing a nil Object to the reconciler signals that the + // accompanying ID is for an Object that was deleted. + toRemediate = nil + } else { + // Not actually deleted, or if it was then it's since been recreated. + // Pass the latest Object state to the reconciler to handle as if + // in response to an update event. + toRemediate = latestObj + } } else { toRemediate = obj } @@ -128,25 +152,29 @@ func (w *Worker) process(ctx context.Context, obj client.Object) error { } // refresh updates the cached version of the object. -func (w *Worker) refresh(ctx context.Context, o client.Object) status.Error { - c := w.reconciler.GetClient() - - // Try to get an updated version of the object from the cluster. - u := &unstructured.Unstructured{} - u.SetGroupVersionKind(o.GetObjectKind().GroupVersionKind()) - err := c.Get(ctx, client.ObjectKey{Name: o.GetName(), Namespace: o.GetNamespace()}, u) - - switch { - case apierrors.IsNotFound(err): - // The object no longer exists on the cluster, so mark it deleted. - w.objectQueue.Add(queue.MarkDeleted(ctx, o)) - case err != nil: - // We encountered some other error that we don't know how to solve, so - // surface it. - return status.APIServerError(err, "failed to get updated object for worker cache", o) - default: - // Update the cached version of the resource. - w.objectQueue.Add(u) +func (w *Worker) refresh(ctx context.Context, obj client.Object) status.Error { + obj, err := w.getObject(ctx, obj) + if err != nil { + return err } + // Enqueue object for remediation + w.objectQueue.Add(obj) return nil } + +// getObject updates the object from the server. +// Wraps the supplied object with MarkDeleted, if NotFound. +func (w *Worker) getObject(ctx context.Context, obj client.Object) (client.Object, status.Error) { + uObj := &unstructured.Unstructured{} + uObj.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind()) + err := w.reconciler.GetClient().Get(ctx, client.ObjectKeyFromObject(obj), uObj) + if err != nil { + // If not found, wrap for processing as a deleted object + if apierrors.IsNotFound(err) { + return queue.MarkDeleted(ctx, obj), nil + } + // Surface any other errors + return uObj, status.APIServerError(err, "failed to get updated object for worker cache", obj) + } + return uObj, nil +} diff --git a/pkg/remediator/reconcile/worker_test.go b/pkg/remediator/reconcile/worker_test.go index b66abd6f40..3d263ed746 100644 --- a/pkg/remediator/reconcile/worker_test.go +++ b/pkg/remediator/reconcile/worker_test.go @@ -82,7 +82,8 @@ func TestWorker_Run_Remediates(t *testing.T) { } d := makeDeclared(t, randomCommitHash(), declaredObjs...) - w := NewWorker(declared.RootReconciler, configsync.RootSyncName, c.Applier(), q, d, syncertestfake.NewFightHandler()) + w := NewWorker(declared.RootReconciler, configsync.RootSyncName, c.Applier(), q, d, + syncertestfake.NewConflictHandler(), syncertestfake.NewFightHandler()) ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -189,7 +190,8 @@ func TestWorker_Run_RemediatesExisting(t *testing.T) { } d := makeDeclared(t, randomCommitHash(), declaredObjs...) - w := NewWorker(declared.RootReconciler, configsync.RootSyncName, c.Applier(), q, d, syncertestfake.NewFightHandler()) + w := NewWorker(declared.RootReconciler, configsync.RootSyncName, c.Applier(), q, d, + syncertestfake.NewConflictHandler(), syncertestfake.NewFightHandler()) ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -297,7 +299,8 @@ func TestWorker_ProcessNextObject(t *testing.T) { } d := makeDeclared(t, randomCommitHash(), tc.declared...) - w := NewWorker(declared.RootReconciler, configsync.RootSyncName, c.Applier(), q, d, syncertestfake.NewFightHandler()) + w := NewWorker(declared.RootReconciler, configsync.RootSyncName, c.Applier(), q, d, + syncertestfake.NewConflictHandler(), syncertestfake.NewFightHandler()) for _, obj := range tc.toProcess { if err := w.processNextObject(context.Background()); err != nil { @@ -317,7 +320,8 @@ func TestWorker_Run_CancelledWhenEmpty(t *testing.T) { defer q.ShutDown() c := testingfake.NewClient(t, core.Scheme) d := makeDeclared(t, randomCommitHash()) // no resources declared - w := NewWorker(declared.RootReconciler, configsync.RootSyncName, c.Applier(), q, d, syncertestfake.NewFightHandler()) + w := NewWorker(declared.RootReconciler, configsync.RootSyncName, c.Applier(), q, d, + syncertestfake.NewConflictHandler(), syncertestfake.NewFightHandler()) ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -390,7 +394,8 @@ func TestWorker_Run_CancelledWhenNotEmpty(t *testing.T) { d := makeDeclared(t, randomCommitHash(), declaredObjs...) a := &testingfake.Applier{Client: c} - w := NewWorker(declared.RootReconciler, configsync.RootSyncName, a, q, d, syncertestfake.NewFightHandler()) + w := NewWorker(declared.RootReconciler, configsync.RootSyncName, a, q, d, + syncertestfake.NewConflictHandler(), syncertestfake.NewFightHandler()) // Run worker in the background doneCh := make(chan struct{}) diff --git a/pkg/remediator/remediator.go b/pkg/remediator/remediator.go index b69f550347..5daa8f9ad6 100644 --- a/pkg/remediator/remediator.go +++ b/pkg/remediator/remediator.go @@ -98,7 +98,7 @@ func New(scope declared.Scope, syncName string, cfg *rest.Config, applier syncer fightHandler := fight.NewHandler() conflictHandler := conflict.NewHandler() for i := 0; i < numWorkers; i++ { - workers[i] = reconcile.NewWorker(scope, syncName, applier, q, decls, fightHandler) + workers[i] = reconcile.NewWorker(scope, syncName, applier, q, decls, conflictHandler, fightHandler) } remediator := &Remediator{ @@ -202,7 +202,7 @@ func (r *Remediator) UpdateWatches(ctx context.Context, gvks map[schema.GroupVer // ManagementConflict implements Interface. func (r *Remediator) ManagementConflict() bool { - return r.watchMgr.ManagementConflict() + return r.conflictHandler.HasConflictErrors() } // ConflictErrors implements Interface. diff --git a/pkg/remediator/watch/filteredwatcher.go b/pkg/remediator/watch/filteredwatcher.go index e76f97f48e..4f43a18f6b 100644 --- a/pkg/remediator/watch/filteredwatcher.go +++ b/pkg/remediator/watch/filteredwatcher.go @@ -27,6 +27,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" 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/klog/v2" @@ -73,9 +74,7 @@ const maxWatchRetryFactor = 18 type Runnable interface { Stop() Run(ctx context.Context) status.Error - ManagementConflict() bool SetManagementConflict(object client.Object, commit string) - ClearManagementConflict() removeAllManagementConflictErrorsWithGVK(gvk schema.GroupVersionKind) } @@ -94,21 +93,21 @@ const errorLoggingInterval = time.Second // - either present in the declared resources, // - or managed by the same reconciler. type filteredWatcher struct { - gvk string - startWatch WatchFunc - resources *declared.Resources - queue *queue.ObjectQueue - scope declared.Scope - syncName string + gvk string + startWatch WatchFunc + resources *declared.Resources + labelSelector labels.Selector + queue *queue.ObjectQueue + scope declared.Scope + syncName string // errorTracker maps an error to the time when the same error happened last time. errorTracker map[string]time.Time // The following fields are guarded by the mutex. - mux sync.Mutex - base watch.Interface - stopped bool - managementConflict bool - conflictHandler conflict.Handler + mux sync.Mutex + base watch.Interface + stopped bool + conflictHandler conflict.Handler } // filteredWatcher implements the Runnable interface. @@ -126,6 +125,7 @@ func NewFiltered(cfg watcherConfig) Runnable { base: watch.NewEmptyWatch(), errorTracker: make(map[string]time.Time), conflictHandler: cfg.conflictHandler, + labelSelector: cfg.labelSelector, } } @@ -156,26 +156,10 @@ func (w *filteredWatcher) addError(errorID string) bool { return false } -func (w *filteredWatcher) ManagementConflict() bool { - w.mux.Lock() - defer w.mux.Unlock() - return w.managementConflict -} - func (w *filteredWatcher) SetManagementConflict(object client.Object, commit string) { w.mux.Lock() defer w.mux.Unlock() - // The resource should be managed by a namespace reconciler, but now is updated. - // Most likely the resource is now managed by a root reconciler. - // In this case, set `managementConflict` true to trigger the namespace reconciler's - // parse-apply-watch loop. The kpt_applier should report the ManagementConflictError (KNV1060). - // No need to add the conflictError to the remediator because it will be surfaced by kpt_applier. - if w.scope != declared.RootReconciler { - w.managementConflict = true - return - } - manager, found := object.GetAnnotations()[metadata.ResourceManagerKey] // There should be no conflict if the resource manager key annotation is not found // because any reconciler can manage a non-ConfigSync managed resource. @@ -183,19 +167,21 @@ func (w *filteredWatcher) SetManagementConflict(object client.Object, commit str klog.Warningf("No management conflict as the object %q is not managed by Config Sync", core.IDOf(object)) return } + // Root reconciler can override resource managed by namespace reconciler. // It is not a conflict in this case. - if !declared.IsRootManager(manager) { + if w.scope == declared.RootReconciler && !declared.IsRootManager(manager) { klog.Warningf("No management conflict as the root reconciler %q should update the object %q that is managed by the namespace reconciler %q", w.syncName, core.IDOf(object), manager) return } - // The remediator detects conflict between two root reconcilers. - // Add the conflict error to the remediator, and the updateStatus goroutine will surface the ManagementConflictError (KNV1060). - // It also sets `managementConflict` true to keep retrying the parse-apply-watch loop - // so that the error can auto-resolve if the resource is removed from the conflicting manager's repository. - w.managementConflict = true + // The remediator detected a conflict between two reconcilers. + // Add the conflict error to the conflictHandler, and the updateStatus + // goroutine will surface it in the RSync status. + // This will also trigger the parse-apply-watch loop to retry, so the + // applier can remove the error, after the conflict has been resolved by the + // user, by removing it from the conflicting manager's source. newManager := declared.ResourceManager(w.scope, w.syncName) klog.Warningf("The remediator detects a management conflict for object %q between root reconcilers: %q and %q", core.GKNN(object), newManager, manager) @@ -204,12 +190,6 @@ func (w *filteredWatcher) SetManagementConflict(object client.Object, commit str metrics.RecordResourceConflict(context.Background(), commit) } -func (w *filteredWatcher) ClearManagementConflict() { - w.mux.Lock() - w.managementConflict = false - w.mux.Unlock() -} - func (w *filteredWatcher) removeAllManagementConflictErrorsWithGVK(gvk schema.GroupVersionKind) { w.conflictHandler.RemoveAllConflictErrors(gvk) } @@ -379,6 +359,7 @@ func (w *filteredWatcher) start(ctx context.Context, resourceVersion string) (bo timeoutSeconds := int64(minWatchTimeout.Seconds() * (rand.Float64() + 1.0)) options := metav1.ListOptions{ AllowWatchBookmarks: true, + LabelSelector: w.labelSelector.String(), ResourceVersion: resourceVersion, TimeoutSeconds: &timeoutSeconds, Watch: true, @@ -458,27 +439,31 @@ func (w *filteredWatcher) handle(ctx context.Context, event watch.Event) (string return "", false, nil } - if klog.V(5).Enabled() { - klog.V(5).Infof("Received watch event for object: %q (generation: %d): %s", - core.IDOf(object), object.GetGeneration(), log.AsJSON(object)) - } else { - klog.V(3).Infof("Received watch event for object: %q (generation: %d)", - core.IDOf(object), object.GetGeneration()) - } + klog.V(3).Infof("Received %s watch event for object: %q (generation: %d)", + event.Type, core.IDOf(object), object.GetGeneration()) + // Log object JSON at higher log level + klog.V(5).Infof("Object from %s watch event: %q (generation: %d): %s", + event.Type, core.IDOf(object), object.GetGeneration(), log.AsJSON(object)) // filter objects. if !w.shouldProcess(object) { - klog.V(4).Infof("Ignoring event for object: %q (generation: %d)", + klog.V(3).Infof("Ignoring event for object: %q (generation: %d)", core.IDOf(object), object.GetGeneration()) return object.GetResourceVersion(), true, nil } if deleted { - klog.V(2).Infof("Received watch event for deleted object %q (generation: %d)", + // When watching with label filtering, removing or changing a watched + // label results in a deletion event with the last known object state + // before label change. But to efficiently handle watch events, we don't + // want to query the apiserver from the event handler. So instead, we + // just enqueue the object as deleted and let the queue worker handle + // double-checking whether the object was actually deleted or not. + klog.V(2).Infof("Enqueuing deleted object for remediation: %q (generation: %d)", core.IDOf(object), object.GetGeneration()) object = queue.MarkDeleted(ctx, object) } else { - klog.V(2).Infof("Received watch event for created/updated object %q (generation: %d)", + klog.V(2).Infof("Enqueuing created/updated object for remediation: %q (generation: %d)", core.IDOf(object), object.GetGeneration()) } diff --git a/pkg/remediator/watch/filteredwatcher_test.go b/pkg/remediator/watch/filteredwatcher_test.go index 86b5845cca..47b5a5a22e 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 eb765a0ef3..27c25fbc86 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,8 @@ type Manager struct { // watcherFactory is the function to create a watcher. watcherFactory watcherFactory + labelSelector labels.Selector + // The following fields are guarded by the mutex. mux sync.Mutex // watcherMap maps GVKs to their associated watchers @@ -87,13 +91,21 @@ func NewManager(scope declared.Scope, syncName string, cfg *rest.Config, } } + packageID := metadata.PackageID(syncName, + declared.SyncNamespaceFromScope(scope), + declared.SyncKindFromScope(scope)) + return &Manager{ - scope: scope, - syncName: syncName, - cfg: cfg, - resources: decls, - watcherMap: make(map[schema.GroupVersionKind]Runnable), - watcherFactory: options.watcherFactory, + scope: scope, + syncName: syncName, + cfg: cfg, + resources: decls, + watcherMap: make(map[schema.GroupVersionKind]Runnable), + watcherFactory: options.watcherFactory, + // Only watch & remediate objects applied by this reconciler + labelSelector: labels.Set{ + metadata.ParentPackageIDLabel: packageID, + }.AsSelector(), queue: q, conflictHandler: ch, }, nil @@ -106,24 +118,6 @@ func (m *Manager) NeedsUpdate() bool { return m.needsUpdate } -// ManagementConflict returns true if any watcher notices any management conflicts. This function is threadsafe. -func (m *Manager) ManagementConflict() bool { - m.mux.Lock() - defer m.mux.Unlock() - - managementConflict := false - // If one of the watchers noticed a management conflict, the remediator will indicate that - // it needs an update so that the parse-apply-watch loop can also detect the conflict and - //report it as an error status. - for _, watcher := range m.watcherMap { - if watcher.ManagementConflict() { - managementConflict = true - watcher.ClearManagementConflict() - } - } - return managementConflict -} - // UpdateWatches accepts a map of GVKs that should be watched and takes the // following actions: // - stop watchers for any GroupVersionKind that is not present in the given @@ -196,6 +190,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 a62f1742d6..3cbaf5432b 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. diff --git a/pkg/status/management_conflict_error.go b/pkg/status/management_conflict_error.go index 4ccdd7d23b..ad1df399a2 100644 --- a/pkg/status/management_conflict_error.go +++ b/pkg/status/management_conflict_error.go @@ -19,6 +19,7 @@ import ( v1 "kpt.dev/configsync/pkg/api/configmanagement/v1" "kpt.dev/configsync/pkg/api/configsync/v1beta1" + "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/metadata" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -32,6 +33,8 @@ var ManagementConflictErrorBuilder = NewErrorBuilder(ManagementConflictErrorCode // ManagementConflictError indicates that the passed resource is illegally // declared in multiple repositories. type ManagementConflictError interface { + // ConflictingObjectID returns the ID of the object with the management conflict. + ConflictingObjectID() core.ID // ConflictingManager returns the annotation value of the other conflicting manager. ConflictingManager() string // CurrentManagerError returns the error that will be surfaced to the current manager. @@ -122,3 +125,7 @@ func (m *managementConflictErrorImpl) Is(target error) bool { } return m.underlying.Is(target) } + +func (m managementConflictErrorImpl) ConflictingObjectID() core.ID { + return core.IDOf(m.resource) +} diff --git a/pkg/syncer/syncertest/fake/conflict_handler.go b/pkg/syncer/syncertest/fake/conflict_handler.go index 6825cf5876..d6d39264f1 100644 --- a/pkg/syncer/syncertest/fake/conflict_handler.go +++ b/pkg/syncer/syncertest/fake/conflict_handler.go @@ -36,6 +36,11 @@ func (h *ConflictHandler) ConflictErrors() []status.ManagementConflictError { return nil } +// HasConflictErrors is a fake implementation of HasConflictErrors of conflict.Handler. +func (h *ConflictHandler) HasConflictErrors() bool { + return false +} + // RemoveAllConflictErrors is a fake implementation of RemoveAllConflictErrors of conflict.Handler. func (h *ConflictHandler) RemoveAllConflictErrors(schema.GroupVersionKind) { } diff --git a/pkg/syncer/syncertest/fake/storage.go b/pkg/syncer/syncertest/fake/storage.go index 01e9dab70c..b398286e7b 100644 --- a/pkg/syncer/syncertest/fake/storage.go +++ b/pkg/syncer/syncertest/fake/storage.go @@ -40,6 +40,7 @@ import ( "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/kinds" + "kpt.dev/configsync/pkg/reconcilermanager" "kpt.dev/configsync/pkg/util/log" "sigs.k8s.io/cli-utils/pkg/testutil" "sigs.k8s.io/controller-runtime/pkg/client" @@ -470,7 +471,9 @@ func (ms *MemoryStorage) validateCreateOptions(opts *client.CreateOptions) error return fmt.Errorf("invalid dry run option: %+v", opts.DryRun) } } - if opts.FieldManager != "" && opts.FieldManager != configsync.FieldManager { + if opts.FieldManager != "" && + opts.FieldManager != configsync.FieldManager && + opts.FieldManager != reconcilermanager.FieldManager { return fmt.Errorf("invalid field manager option: %v", opts.FieldManager) } return nil @@ -751,7 +754,9 @@ func (ms *MemoryStorage) validateUpdateOptions(opts *client.UpdateOptions) error return fmt.Errorf("invalid dry run option: %+v", opts.DryRun) } } - if opts.FieldManager != "" && opts.FieldManager != configsync.FieldManager { + if opts.FieldManager != "" && + opts.FieldManager != configsync.FieldManager && + opts.FieldManager != reconcilermanager.FieldManager { return fmt.Errorf("invalid field manager option: %v", opts.FieldManager) } return nil @@ -929,7 +934,9 @@ func (ms *MemoryStorage) validatePatchOptions(opts *client.PatchOptions, patch c return fmt.Errorf("invalid dry run option: %+v", opts.DryRun) } } - if opts.FieldManager != "" && opts.FieldManager != configsync.FieldManager { + if opts.FieldManager != "" && + opts.FieldManager != configsync.FieldManager && + opts.FieldManager != reconcilermanager.FieldManager { return fmt.Errorf("invalid field manager option: %v", opts.FieldManager) } if patch != client.Apply && opts.Force != nil { diff --git a/pkg/util/bytes/bytes.go b/pkg/util/bytes/bytes.go new file mode 100644 index 0000000000..243cc8c605 --- /dev/null +++ b/pkg/util/bytes/bytes.go @@ -0,0 +1,30 @@ +package bytes + +type Bit int + +type Byte int + +const ( + ByteSize Bit = 8 + + Kb Bit = 1000 // Kilobit + Mb Bit = 1000 * Kb // Megabit + Gb Bit = 1000 * Mb // Gigabit + Tb Bit = 1000 * Gb // Terabit + Pb Bit = 1000 * Tb // Petabit + Eb Bit = 1000 * Pb // Exabit + + KB Byte = 1000 // Kilobyte + MB Byte = 1000 * KB // Megabyte + GB Byte = 1000 * MB // Gigabyte + TB Byte = 1000 * GB // Terabyte + PB Byte = 1000 * TB // Petabyte + EB Byte = 1000 * PB // Exabyte + + KiB Byte = 1024 // Kibibyte + MiB Byte = 1024 * KiB // Mebibyte + GiB Byte = 1024 * MiB // Gibibyte + TiB Byte = 1024 * GiB // Tebibyte + PiB Byte = 1024 * TiB // Pebibyte + EiB Byte = 1024 * PiB // Exbibyte +) diff --git a/pkg/util/mutate/mutate.go b/pkg/util/mutate/mutate.go index 400bf0ed6e..1d5aedc6b6 100644 --- a/pkg/util/mutate/mutate.go +++ b/pkg/util/mutate/mutate.go @@ -58,8 +58,8 @@ func RetriableOrConflict(err error) bool { // unnecessary (`NoUpdateError` from the `mutate.Func`). Retries are quick, with // no backoff. // Returns an error if the status update fails OR if the mutate func fails. -func Spec(ctx context.Context, c client.Client, obj client.Object, mutateFn Func) (bool, error) { - return withRetry(ctx, &specClient{client: c}, obj, mutateFn) +func Spec(ctx context.Context, c client.Client, obj client.Object, mutateFn Func, opts ...client.UpdateOption) (bool, error) { + return withRetry(ctx, &specClient{client: c, updateOptions: opts}, obj, mutateFn) } // Status attempts to update the status of an object until successful or update @@ -67,7 +67,7 @@ func Spec(ctx context.Context, c client.Client, obj client.Object, mutateFn Func // quick, with no backoff. // Returns an error if the status update fails OR if the mutate func fails OR if // the generation changes before the status update succeeds. -func Status(ctx context.Context, c client.Client, obj client.Object, mutateFn Func) (bool, error) { +func Status(ctx context.Context, c client.Client, obj client.Object, mutateFn Func, opts ...client.UpdateOption) (bool, error) { oldGen := obj.GetGeneration() mutateFn2 := func() error { newGen := obj.GetGeneration() @@ -79,7 +79,7 @@ func Status(ctx context.Context, c client.Client, obj client.Object, mutateFn Fu } return mutateFn() } - return withRetry(ctx, &statusClient{client: c}, obj, mutateFn2) + return withRetry(ctx, &statusClient{client: c, updateOptions: opts}, obj, mutateFn2) } // withRetry attempts to update an object until successful or update becomes @@ -169,7 +169,8 @@ type updater interface { } type specClient struct { - client client.Client + client client.Client + updateOptions []client.UpdateOption } // Get the current spec of the specified object. @@ -179,7 +180,7 @@ func (c *specClient) Get(ctx context.Context, obj client.Object) error { // Update the spec of the specified object. func (c *specClient) Update(ctx context.Context, obj client.Object) error { - return c.client.Update(ctx, obj) + return c.client.Update(ctx, obj, c.updateOptions...) } // WrapError returns the specified error wrapped with extra context specific @@ -189,7 +190,8 @@ func (c *specClient) WrapError(_ context.Context, obj client.Object, err error) } type statusClient struct { - client client.Client + client client.Client + updateOptions []client.UpdateOption } // Get the current status of the specified object. @@ -199,7 +201,7 @@ func (c *statusClient) Get(ctx context.Context, obj client.Object) error { // Update the status of the specified object. func (c *statusClient) Update(ctx context.Context, obj client.Object) error { - return c.client.Status().Update(ctx, obj) + return c.client.Status().Update(ctx, obj, convertUpdateOptions(c.updateOptions)...) } // WrapError returns the specified error wrapped with extra context specific @@ -207,3 +209,15 @@ func (c *statusClient) Update(ctx context.Context, obj client.Object) error { func (c *statusClient) WrapError(_ context.Context, obj client.Object, err error) error { return fmt.Errorf("failed to update object status: %s: %w", kinds.ObjectSummary(obj), err) } + +// convertUpdateOptions converts []client.UpdateOption to []client.SubResourceUpdateOption +func convertUpdateOptions(optList []client.UpdateOption) []client.SubResourceUpdateOption { + if len(optList) == 0 { + return nil + } + opts := &client.UpdateOptions{} + opts.ApplyOptions(optList) + return []client.SubResourceUpdateOption{ + &client.SubResourceUpdateOptions{UpdateOptions: *opts}, + } +} diff --git a/vendor/github.com/wk8/go-ordered-map/.gitignore b/vendor/github.com/wk8/go-ordered-map/.gitignore new file mode 100644 index 0000000000..57872d0f1e --- /dev/null +++ b/vendor/github.com/wk8/go-ordered-map/.gitignore @@ -0,0 +1 @@ +/vendor/ diff --git a/vendor/github.com/wk8/go-ordered-map/.golangci.yml b/vendor/github.com/wk8/go-ordered-map/.golangci.yml new file mode 100644 index 0000000000..7623c1756d --- /dev/null +++ b/vendor/github.com/wk8/go-ordered-map/.golangci.yml @@ -0,0 +1,85 @@ +run: + tests: false + +linters: + disable-all: true + enable: + - asciicheck + - bidichk + - bodyclose + - containedctx + - contextcheck + - cyclop + - deadcode + - decorder + - depguard + - dogsled + - dupl + - durationcheck + - errcheck + - errchkjson + - errname + - errorlint + - exhaustive + - exportloopref + - forbidigo + - funlen + - gci + - gochecknoglobals + - gochecknoinits + - gocognit + - goconst + - gocritic + - gocyclo + - godox + - goerr113 + - gofmt + - gofumpt + - goheader + - goimports + - gomnd + - gomoddirectives + - gomodguard + - goprintffuncname + - gosec + - gosimple + - govet + - grouper + - ifshort + - importas + - ineffassign + - ireturn + - lll + - maintidx + - makezero + - misspell + - nakedret + - nestif + - nilerr + - nilnil + - noctx + - nolintlint + - paralleltest + - prealloc + - predeclared + - promlinter + - revive + - rowserrcheck + - sqlclosecheck + - staticcheck + - structcheck + - stylecheck + - tagliatelle + - tenv + - testpackage + - thelper + - tparallel + - typecheck + - unconvert + - unparam + - unused + - varcheck + - varnamelen + - wastedassign + - whitespace + - wrapcheck diff --git a/vendor/github.com/wk8/go-ordered-map/LICENSE b/vendor/github.com/wk8/go-ordered-map/LICENSE new file mode 100644 index 0000000000..8dada3edaf --- /dev/null +++ b/vendor/github.com/wk8/go-ordered-map/LICENSE @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "{}" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright {yyyy} {name of copyright owner} + + 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. diff --git a/vendor/github.com/wk8/go-ordered-map/Makefile b/vendor/github.com/wk8/go-ordered-map/Makefile new file mode 100644 index 0000000000..5186d9be64 --- /dev/null +++ b/vendor/github.com/wk8/go-ordered-map/Makefile @@ -0,0 +1,13 @@ +.DEFAULT_GOAL := all + +.PHONY: all +all: test lint + +# the TEST_FLAGS env var can be set to eg run only specific tests +.PHONY: test +test: + go test -v -count=1 -race -cover "$$TEST_FLAGS" + +.PHONY: lint +lint: + golangci-lint run diff --git a/vendor/github.com/wk8/go-ordered-map/README.md b/vendor/github.com/wk8/go-ordered-map/README.md new file mode 100644 index 0000000000..9c1675f8bf --- /dev/null +++ b/vendor/github.com/wk8/go-ordered-map/README.md @@ -0,0 +1,104 @@ +[![Build Status](https://circleci.com/gh/wk8/go-ordered-map.svg?style=svg)](https://app.circleci.com/pipelines/github/wk8/go-ordered-map) + +# Goland Ordered Maps + +Same as regular maps, but also remembers the order in which keys were inserted, akin to [Python's `collections.OrderedDict`s](https://docs.python.org/3.7/library/collections.html#ordereddict-objects). + +It offers the following features: +* optimal runtime performance (all operations are constant time) +* optimal memory usage (only one copy of values, no unnecessary memory allocation) +* allows iterating from newest or oldest keys indifferently, without memory copy, allowing to `break` the iteration, and in time linear to the number of keys iterated over rather than the total length of the ordered map +* takes and returns generic `interface{}`s +* idiomatic API, akin to that of [`container/list`](https://golang.org/pkg/container/list) + +## Installation +```bash +go get -u github.com/wk8/go-ordered-map +``` + +Or use your favorite golang vendoring tool! + +## Supported go versions + +All go versions >= 1.13 are supported. There's no reason for older versions to not also work, but they're not part of the build matrix. + +## Documentation + +[The full documentation is available on godoc.org](https://godoc.org/github.com/wk8/go-ordered-map). + +## Example / usage + +```go +package main + +import ( + "fmt" + + "github.com/wk8/go-ordered-map" +) + +func main() { + om := orderedmap.New() + + om.Set("foo", "bar") + om.Set("bar", "baz") + om.Set("coucou", "toi") + + fmt.Println(om.Get("foo")) // => bar, true + fmt.Println(om.Get("i dont exist")) // => , false + + // iterating pairs from oldest to newest: + for pair := om.Oldest(); pair != nil; pair = pair.Next() { + fmt.Printf("%s => %s\n", pair.Key, pair.Value) + } // prints: + // foo => bar + // bar => baz + // coucou => toi + + // iterating over the 2 newest pairs: + i := 0 + for pair := om.Newest(); pair != nil; pair = pair.Prev() { + fmt.Printf("%s => %s\n", pair.Key, pair.Value) + i++ + if i >= 2 { + break + } + } // prints: + // coucou => toi + // bar => baz +} +``` + +All of `OrderedMap`'s methods accept and return `interface{}`s, so you can use any type of keys that regular `map`s accept, as well pack/unpack arbitrary values, e.g.: +```go +type myStruct struct { + payload string +} + +func main() { + om := orderedmap.New() + + om.Set(12, &myStruct{"foo"}) + om.Set(1, &myStruct{"bar"}) + + value, present := om.Get(12) + if !present { + panic("should be there!") + } + fmt.Println(value.(*myStruct).payload) // => foo + + for pair := om.Oldest(); pair != nil; pair = pair.Next() { + fmt.Printf("%d => %s\n", pair.Key, pair.Value.(*myStruct).payload) + } // prints: + // 12 => foo + // 1 => bar +} +``` + +## Alternatives + +There are several other ordered map golang implementations out there, but I believe that at the time of writing none of them offer the same functionality as this library; more specifically: +* [iancoleman/orderedmap](https://github.com/iancoleman/orderedmap) only accepts `string` keys, its `Delete` operations are linear +* [cevaris/ordered_map](https://github.com/cevaris/ordered_map) uses a channel for iterations, and leaks goroutines if the iteration is interrupted before fully traversing the map +* [mantyr/iterator](https://github.com/mantyr/iterator) also uses a channel for iterations, and its `Delete` operations are linear +* [samdolan/go-ordered-map](https://github.com/samdolan/go-ordered-map) adds unnecessary locking (users should add their own locking instead if they need it), its `Delete` and `Get` operations are linear, iterations trigger a linear memory allocation diff --git a/vendor/github.com/wk8/go-ordered-map/orderedmap.go b/vendor/github.com/wk8/go-ordered-map/orderedmap.go new file mode 100644 index 0000000000..e5fd79a7da --- /dev/null +++ b/vendor/github.com/wk8/go-ordered-map/orderedmap.go @@ -0,0 +1,193 @@ +// Package orderedmap implements an ordered map, i.e. a map that also keeps track of +// the order in which keys were inserted. +// +// All operations are constant-time. +// +// Github repo: https://github.com/wk8/go-ordered-map +// +package orderedmap + +import ( + "container/list" + "fmt" +) + +type Pair struct { + Key interface{} + Value interface{} + + element *list.Element +} + +type OrderedMap struct { + pairs map[interface{}]*Pair + list *list.List +} + +// New creates a new OrderedMap. +func New() *OrderedMap { + return &OrderedMap{ + pairs: make(map[interface{}]*Pair), + list: list.New(), + } +} + +// Get looks for the given key, and returns the value associated with it, +// or nil if not found. The boolean it returns says whether the key is present in the map. +func (om *OrderedMap) Get(key interface{}) (interface{}, bool) { + if pair, present := om.pairs[key]; present { + return pair.Value, present + } + return nil, false +} + +// Load is an alias for Get, mostly to present an API similar to `sync.Map`'s. +func (om *OrderedMap) Load(key interface{}) (interface{}, bool) { + return om.Get(key) +} + +// GetPair looks for the given key, and returns the pair associated with it, +// or nil if not found. The Pair struct can then be used to iterate over the ordered map +// from that point, either forward or backward. +func (om *OrderedMap) GetPair(key interface{}) *Pair { + return om.pairs[key] +} + +// Set sets the key-value pair, and returns what `Get` would have returned +// on that key prior to the call to `Set`. +func (om *OrderedMap) Set(key interface{}, value interface{}) (interface{}, bool) { + if pair, present := om.pairs[key]; present { + oldValue := pair.Value + pair.Value = value + return oldValue, true + } + + pair := &Pair{ + Key: key, + Value: value, + } + pair.element = om.list.PushBack(pair) + om.pairs[key] = pair + + return nil, false +} + +// Store is an alias for Set, mostly to present an API similar to `sync.Map`'s. +func (om *OrderedMap) Store(key interface{}, value interface{}) (interface{}, bool) { + return om.Set(key, value) +} + +// Delete removes the key-value pair, and returns what `Get` would have returned +// on that key prior to the call to `Delete`. +func (om *OrderedMap) Delete(key interface{}) (interface{}, bool) { + if pair, present := om.pairs[key]; present { + om.list.Remove(pair.element) + delete(om.pairs, key) + return pair.Value, true + } + return nil, false +} + +// Len returns the length of the ordered map. +func (om *OrderedMap) Len() int { + return len(om.pairs) +} + +// Oldest returns a pointer to the oldest pair. It's meant to be used to iterate on the ordered map's +// pairs from the oldest to the newest, e.g.: +// for pair := orderedMap.Oldest(); pair != nil; pair = pair.Next() { fmt.Printf("%v => %v\n", pair.Key, pair.Value) } +func (om *OrderedMap) Oldest() *Pair { + return listElementToPair(om.list.Front()) +} + +// Newest returns a pointer to the newest pair. It's meant to be used to iterate on the ordered map's +// pairs from the newest to the oldest, e.g.: +// for pair := orderedMap.Oldest(); pair != nil; pair = pair.Next() { fmt.Printf("%v => %v\n", pair.Key, pair.Value) } +func (om *OrderedMap) Newest() *Pair { + return listElementToPair(om.list.Back()) +} + +// Next returns a pointer to the next pair. +func (p *Pair) Next() *Pair { + return listElementToPair(p.element.Next()) +} + +// Previous returns a pointer to the previous pair. +func (p *Pair) Prev() *Pair { + return listElementToPair(p.element.Prev()) +} + +func listElementToPair(element *list.Element) *Pair { + if element == nil { + return nil + } + return element.Value.(*Pair) +} + +// KeyNotFoundError may be returned by functions in this package when they're called with keys that are not present +// in the map. +type KeyNotFoundError struct { + MissingKey interface{} +} + +var _ error = &KeyNotFoundError{} + +func (e *KeyNotFoundError) Error() string { + return fmt.Sprintf("missing key: %v", e.MissingKey) +} + +// MoveAfter moves the value associated with key to its new position after the one associated with markKey. +// Returns an error iff key or markKey are not present in the map. +func (om *OrderedMap) MoveAfter(key, markKey interface{}) error { + elements, err := om.getElements(key, markKey) + if err != nil { + return err + } + om.list.MoveAfter(elements[0], elements[1]) + return nil +} + +// MoveBefore moves the value associated with key to its new position before the one associated with markKey. +// Returns an error iff key or markKey are not present in the map. +func (om *OrderedMap) MoveBefore(key, markKey interface{}) error { + elements, err := om.getElements(key, markKey) + if err != nil { + return err + } + om.list.MoveBefore(elements[0], elements[1]) + return nil +} + +func (om *OrderedMap) getElements(keys ...interface{}) ([]*list.Element, error) { + elements := make([]*list.Element, len(keys)) + for i, k := range keys { + pair, present := om.pairs[k] + if !present { + return nil, &KeyNotFoundError{k} + } + elements[i] = pair.element + } + return elements, nil +} + +// MoveToBack moves the value associated with key to the back of the ordered map. +// Returns an error iff key is not present in the map. +func (om *OrderedMap) MoveToBack(key interface{}) error { + pair, present := om.pairs[key] + if !present { + return &KeyNotFoundError{key} + } + om.list.MoveToBack(pair.element) + return nil +} + +// MoveToFront moves the value associated with key to the front of the ordered map. +// Returns an error iff key is not present in the map. +func (om *OrderedMap) MoveToFront(key interface{}) error { + pair, present := om.pairs[key] + if !present { + return &KeyNotFoundError{key} + } + om.list.MoveToFront(pair.element) + return nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 02d45d1a07..e1782ac6b3 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -414,6 +414,9 @@ github.com/stretchr/testify/require # github.com/vbatts/tar-split v0.11.2 ## explicit; go 1.15 github.com/vbatts/tar-split/archive/tar +# github.com/wk8/go-ordered-map v1.0.0 +## explicit; go 1.14 +github.com/wk8/go-ordered-map # github.com/xlab/treeprint v1.2.0 ## explicit; go 1.13 github.com/xlab/treeprint @@ -1160,7 +1163,7 @@ k8s.io/utils/pointer k8s.io/utils/ptr k8s.io/utils/strings/slices k8s.io/utils/trace -# sigs.k8s.io/cli-utils v0.35.1-0.20240504222723-227a03f4a7f9 +# sigs.k8s.io/cli-utils v0.36.1-0.20240525003310-87074c9799d2 ## explicit; go 1.20 sigs.k8s.io/cli-utils/pkg/apis/actuation sigs.k8s.io/cli-utils/pkg/apply diff --git a/vendor/sigs.k8s.io/cli-utils/pkg/apply/applier_builder.go b/vendor/sigs.k8s.io/cli-utils/pkg/apply/applier_builder.go index 09f3523902..e964d110f8 100644 --- a/vendor/sigs.k8s.io/cli-utils/pkg/apply/applier_builder.go +++ b/vendor/sigs.k8s.io/cli-utils/pkg/apply/applier_builder.go @@ -86,3 +86,8 @@ func (b *ApplierBuilder) WithStatusWatcher(statusWatcher watcher.StatusWatcher) b.statusWatcher = statusWatcher return b } + +func (b *ApplierBuilder) WithStatusWatcherFilters(filters *watcher.Filters) *ApplierBuilder { + b.statusWatcherFilters = filters + return b +} diff --git a/vendor/sigs.k8s.io/cli-utils/pkg/apply/builder.go b/vendor/sigs.k8s.io/cli-utils/pkg/apply/builder.go index e252902c9d..75602e74f0 100644 --- a/vendor/sigs.k8s.io/cli-utils/pkg/apply/builder.go +++ b/vendor/sigs.k8s.io/cli-utils/pkg/apply/builder.go @@ -27,6 +27,7 @@ type commonBuilder struct { restConfig *rest.Config unstructuredClientForMapping func(*meta.RESTMapping) (resource.RESTClient, error) statusWatcher watcher.StatusWatcher + statusWatcherFilters *watcher.Filters } func (cb *commonBuilder) finalize() (*commonBuilder, error) { @@ -78,7 +79,15 @@ func (cb *commonBuilder) finalize() (*commonBuilder, error) { cx.unstructuredClientForMapping = cx.factory.UnstructuredClientForMapping } if cx.statusWatcher == nil { - cx.statusWatcher = watcher.NewDefaultStatusWatcher(cx.client, cx.mapper) + statusWatcher := watcher.NewDefaultStatusWatcher(cx.client, cx.mapper) + if cx.statusWatcherFilters != nil { + statusWatcher.Filters = cx.statusWatcherFilters + } + cx.statusWatcher = statusWatcher + } else if cx.statusWatcherFilters != nil { + // If you want to use a custom status watcher with a label selector, + // configure it before injecting the status watcher. + return nil, errors.New("status watcher and status watcher filters must not both be provided") } return &cx, nil } diff --git a/vendor/sigs.k8s.io/cli-utils/pkg/apply/destroyer_builder.go b/vendor/sigs.k8s.io/cli-utils/pkg/apply/destroyer_builder.go index 870859fa1f..6927284dc4 100644 --- a/vendor/sigs.k8s.io/cli-utils/pkg/apply/destroyer_builder.go +++ b/vendor/sigs.k8s.io/cli-utils/pkg/apply/destroyer_builder.go @@ -86,3 +86,8 @@ func (b *DestroyerBuilder) WithStatusWatcher(statusWatcher watcher.StatusWatcher b.statusWatcher = statusWatcher return b } + +func (b *DestroyerBuilder) WithStatusWatcherFilters(filters *watcher.Filters) *DestroyerBuilder { + b.statusWatcherFilters = filters + return b +} diff --git a/vendor/sigs.k8s.io/cli-utils/pkg/kstatus/watcher/default_status_watcher.go b/vendor/sigs.k8s.io/cli-utils/pkg/kstatus/watcher/default_status_watcher.go index 08d3ea97a7..e732fbe4db 100644 --- a/vendor/sigs.k8s.io/cli-utils/pkg/kstatus/watcher/default_status_watcher.go +++ b/vendor/sigs.k8s.io/cli-utils/pkg/kstatus/watcher/default_status_watcher.go @@ -11,6 +11,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/dynamic" + "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" "sigs.k8s.io/cli-utils/pkg/kstatus/polling/clusterreader" "sigs.k8s.io/cli-utils/pkg/kstatus/polling/engine" @@ -43,6 +44,15 @@ type DefaultStatusWatcher struct { // required for computing parent object status, to compensate for // controllers that aren't following status conventions. ClusterReader engine.ClusterReader + + // Indexers control how the watch cache is indexed, allowing namespace + // filtering and field selectors. If you watch at namespace scope, you must + // provide the namespace indexer. If you specify a field selector filter, + // you must also provide an indexer for that field. + Indexers cache.Indexers + + // Filters allows filtering the objects being watched. + Filters *Filters } var _ StatusWatcher = &DefaultStatusWatcher{} @@ -60,6 +70,7 @@ func NewDefaultStatusWatcher(dynamicClient dynamic.Interface, mapper meta.RESTMa DynamicClient: dynamicClient, Mapper: mapper, }, + Indexers: DefaultIndexers(), } } @@ -88,13 +99,18 @@ func (w *DefaultStatusWatcher) Watch(ctx context.Context, ids object.ObjMetadata } informer := &ObjectStatusReporter{ - InformerFactory: NewDynamicInformerFactory(w.DynamicClient, w.ResyncPeriod), - Mapper: w.Mapper, - StatusReader: w.StatusReader, - ClusterReader: w.ClusterReader, - Targets: targets, - ObjectFilter: &AllowListObjectFilter{AllowList: ids}, - RESTScope: scope, + InformerFactory: &DynamicInformerFactory{ + Client: w.DynamicClient, + ResyncPeriod: w.ResyncPeriod, + Indexers: w.Indexers, + Filters: w.Filters, + }, + Mapper: w.Mapper, + StatusReader: w.StatusReader, + ClusterReader: w.ClusterReader, + Targets: targets, + ObjectFilter: &AllowListObjectFilter{AllowList: ids}, + RESTScope: scope, } return informer.Start(ctx) } diff --git a/vendor/sigs.k8s.io/cli-utils/pkg/kstatus/watcher/dynamic_informer_factory.go b/vendor/sigs.k8s.io/cli-utils/pkg/kstatus/watcher/dynamic_informer_factory.go index 27810d6050..1e0c95390b 100644 --- a/vendor/sigs.k8s.io/cli-utils/pkg/kstatus/watcher/dynamic_informer_factory.go +++ b/vendor/sigs.k8s.io/cli-utils/pkg/kstatus/watcher/dynamic_informer_factory.go @@ -5,12 +5,16 @@ package watcher import ( "context" + "fmt" "time" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/dynamic" "k8s.io/client-go/tools/cache" @@ -20,15 +24,14 @@ type DynamicInformerFactory struct { Client dynamic.Interface ResyncPeriod time.Duration Indexers cache.Indexers + Filters *Filters } func NewDynamicInformerFactory(client dynamic.Interface, resyncPeriod time.Duration) *DynamicInformerFactory { return &DynamicInformerFactory{ Client: client, ResyncPeriod: resyncPeriod, - Indexers: cache.Indexers{ - cache.NamespaceIndex: cache.MetaNamespaceIndexFunc, - }, + Indexers: DefaultIndexers(), } } @@ -36,22 +39,122 @@ func (f *DynamicInformerFactory) NewInformer(ctx context.Context, mapping *meta. // Unstructured example output need `"apiVersion"` and `"kind"` set. example := &unstructured.Unstructured{} example.SetGroupVersionKind(mapping.GroupVersionKind) - return cache.NewSharedIndexInformer( - &cache.ListWatch{ - ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return f.Client.Resource(mapping.Resource). - Namespace(namespace). - List(ctx, options) - }, - WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return f.Client.Resource(mapping.Resource). - Namespace(namespace). - Watch(ctx, options) - }, - }, + NewFilteredListWatchFromDynamicClient( + ctx, + f.Client, + mapping.Resource, + namespace, + f.Filters, + ), example, f.ResyncPeriod, f.Indexers, ) } + +// DefaultIndexers returns the default set of cache indexers, namely the +// namespace indexer. +func DefaultIndexers() cache.Indexers { + return cache.Indexers{ + cache.NamespaceIndex: cache.MetaNamespaceIndexFunc, + } +} + +// Filters are optional selectors for list and watch +type Filters struct { + Labels labels.Selector + Fields fields.Selector +} + +// NewFilteredListWatchFromDynamicClient creates a new ListWatch from the +// specified client, resource, namespace, and optional filters. +func NewFilteredListWatchFromDynamicClient( + ctx context.Context, + client dynamic.Interface, + resource schema.GroupVersionResource, + namespace string, + filters *Filters, +) *cache.ListWatch { + optionsModifier := func(options *metav1.ListOptions) error { + if filters == nil { + return nil + } + if filters.Labels != nil { + selector := filters.Labels + // Merge label selectors, if both were provided + if options.LabelSelector != "" { + var err error + selector, err = labels.Parse(options.LabelSelector) + if err != nil { + return fmt.Errorf("parsing label selector: %w", err) + } + selector = andLabelSelectors(selector, filters.Labels) + } + options.LabelSelector = selector.String() + } + if filters.Fields != nil { + selector := filters.Fields + // Merge field selectors, if both were provided + if options.FieldSelector != "" { + var err error + selector, err = fields.ParseSelector(options.FieldSelector) + if err != nil { + return fmt.Errorf("parsing field selector: %w", err) + } + selector = fields.AndSelectors(selector, filters.Fields) + } + options.FieldSelector = selector.String() + } + return nil + } + return NewModifiedListWatchFromDynamicClient(ctx, client, resource, namespace, optionsModifier) +} + +// NewModifiedListWatchFromDynamicClient creates a new ListWatch from the +// specified client, resource, namespace, and options modifier. +// Options modifier is a function takes a ListOptions and modifies the consumed +// ListOptions. Provide customized modifier function to apply modification to +// ListOptions with field selectors, label selectors, or any other desired options. +func NewModifiedListWatchFromDynamicClient( + ctx context.Context, + client dynamic.Interface, + resource schema.GroupVersionResource, + namespace string, + optionsModifier func(*metav1.ListOptions) error, +) *cache.ListWatch { + listFunc := func(options metav1.ListOptions) (runtime.Object, error) { + if err := optionsModifier(&options); err != nil { + return nil, fmt.Errorf("modifying list options: %w", err) + } + return client.Resource(resource). + Namespace(namespace). + List(ctx, options) + } + watchFunc := func(options metav1.ListOptions) (watch.Interface, error) { + options.Watch = true + if err := optionsModifier(&options); err != nil { + return nil, fmt.Errorf("modifying watch options: %w", err) + } + return client.Resource(resource). + Namespace(namespace). + Watch(ctx, options) + } + return &cache.ListWatch{ListFunc: listFunc, WatchFunc: watchFunc} +} + +func andLabelSelectors(selectors ...labels.Selector) labels.Selector { + var s labels.Selector + for _, item := range selectors { + if s == nil { + s = item + } else { + reqs, selectable := item.Requirements() + if !selectable { + return item // probably the nothing selector + } + s = s.Add(reqs...) + } + } + return s +}