From 65744502979ba45f2864bee758a628e7bd0f1bcf Mon Sep 17 00:00:00 2001 From: Nesma Badr Date: Fri, 11 Oct 2024 13:55:32 +0200 Subject: [PATCH] feat: Remove managed-by label when module is unmanaged (#1913) * Add label removal finalizer * Handle labels deletion from resources and default CR * Add unit tests * Fix linting issues * Add E2E test * Fix linting * Fix test * Fix test * Fix test * Fix test * Extract ManifestApiClient and label removal into new services * Remove watched by label since we no longer have it on the resources * fix unit test * review comments --- cmd/main.go | 8 +- internal/controller/manifest/controller.go | 3 +- internal/controller/manifest/setup.go | 5 +- internal/declarative/v2/reconciler.go | 162 ++++++---- internal/declarative/v2/reconciler_test.go | 74 ----- .../manifest/labelsremoval/labels_removal.go | 100 ++++++ .../labelsremoval/labels_removal_test.go | 296 ++++++++++++++++++ .../manifestclient/manifest_client.go | 67 ++++ .../manifestclient/manifest_client_test.go | 86 +++++ internal/pkg/metrics/manifest.go | 1 + pkg/testutils/builder/manifest.go | 6 + pkg/testutils/unstructured.go | 28 ++ tests/e2e/manifest_reconciliation_test.go | 4 +- tests/e2e/module_unmanage_test.go | 49 ++- tests/e2e/utils_test.go | 8 +- .../custom_resource_check/suite_test.go | 6 + .../controller/manifest/suite_test.go | 6 + unit-test-coverage.yaml | 1 + 18 files changed, 759 insertions(+), 151 deletions(-) create mode 100644 internal/manifest/labelsremoval/labels_removal.go create mode 100644 internal/manifest/labelsremoval/labels_removal_test.go create mode 100644 internal/manifest/manifestclient/manifest_client.go create mode 100644 internal/manifest/manifestclient/manifest_client_test.go diff --git a/cmd/main.go b/cmd/main.go index bae30b1bc26..274f158f92c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -57,6 +57,7 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/crd" "github.com/kyma-project/lifecycle-manager/internal/descriptor/provider" "github.com/kyma-project/lifecycle-manager/internal/event" + "github.com/kyma-project/lifecycle-manager/internal/manifest/manifestclient" "github.com/kyma-project/lifecycle-manager/internal/pkg/flags" "github.com/kyma-project/lifecycle-manager/internal/pkg/metrics" "github.com/kyma-project/lifecycle-manager/internal/remote" @@ -68,7 +69,6 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" _ "ocm.software/ocm/api/ocm" //nolint:gci // kubebuilder's scaffold imports must be appended here. - // +kubebuilder:scaffold:imports ) const ( @@ -184,7 +184,7 @@ func setupManager(flagVar *flags.FlagVar, cacheOptions cache.Options, scheme *ma setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager, kymaMetrics, setupLog) - setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics, setupLog) + setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics, setupLog, eventRecorder) setupMandatoryModuleReconciler(mgr, descriptorProvider, flagVar, options, mandatoryModulesMetrics, setupLog) setupMandatoryModuleDeletionReconciler(mgr, descriptorProvider, eventRecorder, flagVar, options, setupLog) if flagVar.EnablePurgeFinalizer { @@ -383,11 +383,14 @@ func setupPurgeReconciler(mgr ctrl.Manager, func setupManifestReconciler(mgr ctrl.Manager, flagVar *flags.FlagVar, options ctrlruntime.Options, sharedMetrics *metrics.SharedMetrics, mandatoryModulesMetrics *metrics.MandatoryModulesMetrics, setupLog logr.Logger, + event event.Event, ) { options.MaxConcurrentReconciles = flagVar.MaxConcurrentManifestReconciles options.RateLimiter = internal.ManifestRateLimiter(flagVar.FailureBaseDelay, flagVar.FailureMaxDelay, flagVar.RateLimiterFrequency, flagVar.RateLimiterBurst) + manifestClient := manifestclient.NewManifestClient(event, mgr.GetClient()) + if err := manifest.SetupWithManager( mgr, options, queue.RequeueIntervals{ Success: flagVar.ManifestRequeueSuccessInterval, @@ -400,6 +403,7 @@ func setupManifestReconciler(mgr ctrl.Manager, flagVar *flags.FlagVar, options c ListenerAddr: flagVar.ManifestListenerAddr, EnableDomainNameVerification: flagVar.EnableDomainNameVerification, }, metrics.NewManifestMetrics(sharedMetrics), mandatoryModulesMetrics, + manifestClient, ); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Manifest") os.Exit(bootstrapFailedExitCode) diff --git a/internal/controller/manifest/controller.go b/internal/controller/manifest/controller.go index 74b94f35053..ba5fca9e6f7 100644 --- a/internal/controller/manifest/controller.go +++ b/internal/controller/manifest/controller.go @@ -19,6 +19,7 @@ func NewReconciler(mgr manager.Manager, requeueIntervals queue.RequeueIntervals, manifestMetrics *metrics.ManifestMetrics, mandatoryModulesMetrics *metrics.MandatoryModulesMetrics, + manifestClient declarativev2.ManifestAPIClient, ) *declarativev2.Reconciler { kcp := &declarativev2.ClusterInfo{ Client: mgr.GetClient(), @@ -30,7 +31,7 @@ func NewReconciler(mgr manager.Manager, statefulChecker := statecheck.NewStatefulSetStateCheck() deploymentChecker := statecheck.NewDeploymentStateCheck() return declarativev2.NewFromManager( - mgr, requeueIntervals, manifestMetrics, mandatoryModulesMetrics, + mgr, requeueIntervals, manifestMetrics, mandatoryModulesMetrics, manifestClient, manifest.NewSpecResolver(keyChainLookup, extractor), declarativev2.WithCustomStateCheck(statecheck.NewManagerStateCheck(statefulChecker, deploymentChecker)), declarativev2.WithRemoteTargetCluster(lookup.ConfigResolver), diff --git a/internal/controller/manifest/setup.go b/internal/controller/manifest/setup.go index a5b7a83b4f3..2858d8aed73 100644 --- a/internal/controller/manifest/setup.go +++ b/internal/controller/manifest/setup.go @@ -22,6 +22,7 @@ import ( "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" + declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" "github.com/kyma-project/lifecycle-manager/internal/pkg/metrics" "github.com/kyma-project/lifecycle-manager/pkg/queue" "github.com/kyma-project/lifecycle-manager/pkg/security" @@ -40,6 +41,7 @@ func SetupWithManager(mgr manager.Manager, settings SetupOptions, manifestMetrics *metrics.ManifestMetrics, mandatoryModulesMetrics *metrics.MandatoryModulesMetrics, + manifestClient declarativev2.ManifestAPIClient, ) error { var verifyFunc watcherevent.Verify if settings.EnableDomainNameVerification { @@ -84,7 +86,8 @@ func SetupWithManager(mgr manager.Manager, predicate.LabelChangedPredicate{}))). WatchesRawSource(skrEventChannel). WithOptions(opts). - Complete(NewReconciler(mgr, requeueIntervals, manifestMetrics, mandatoryModulesMetrics)); err != nil { + Complete(NewReconciler(mgr, requeueIntervals, manifestMetrics, mandatoryModulesMetrics, + manifestClient)); err != nil { return fmt.Errorf("failed to setup manager for manifest controller: %w", err) } diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index d85bd32b27c..6eae00eea84 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -20,6 +20,7 @@ import ( "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal" + "github.com/kyma-project/lifecycle-manager/internal/manifest/labelsremoval" "github.com/kyma-project/lifecycle-manager/internal/pkg/metrics" "github.com/kyma-project/lifecycle-manager/internal/pkg/resources" "github.com/kyma-project/lifecycle-manager/pkg/common" @@ -49,6 +50,7 @@ func NewFromManager(mgr manager.Manager, requeueIntervals queue.RequeueIntervals, metrics *metrics.ManifestMetrics, mandatoryModulesMetrics *metrics.MandatoryModulesMetrics, + manifestAPIClient ManifestAPIClient, specResolver SpecResolver, options ...Option, ) *Reconciler { @@ -57,16 +59,33 @@ func NewFromManager(mgr manager.Manager, reconciler.MandatoryModuleMetrics = mandatoryModulesMetrics reconciler.RequeueIntervals = requeueIntervals reconciler.specResolver = specResolver + reconciler.manifestClient = manifestAPIClient + reconciler.managedLabelRemovalService = labelsremoval.NewManagedLabelRemovalService(manifestAPIClient) reconciler.Options = DefaultOptions().Apply(WithManager(mgr)).Apply(options...) return reconciler } +type ManagedLabelRemoval interface { + RemoveManagedLabel(ctx context.Context, + manifest *v1beta2.Manifest, skrClient client.Client, defaultCR *unstructured.Unstructured, + ) error +} + +type ManifestAPIClient interface { + UpdateManifest(ctx context.Context, manifest *v1beta2.Manifest) error + PatchStatusIfDiffExist(ctx context.Context, manifest *v1beta2.Manifest, + previousStatus shared.Status) error + SsaSpec(ctx context.Context, obj client.Object) error +} + type Reconciler struct { queue.RequeueIntervals *Options - ManifestMetrics *metrics.ManifestMetrics - MandatoryModuleMetrics *metrics.MandatoryModulesMetrics - specResolver SpecResolver + ManifestMetrics *metrics.ManifestMetrics + MandatoryModuleMetrics *metrics.MandatoryModulesMetrics + specResolver SpecResolver + manifestClient ManifestAPIClient + managedLabelRemovalService ManagedLabelRemoval } const waitingForResourcesMsg = "waiting for resources to become ready" @@ -138,10 +157,26 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu r.MandatoryModuleMetrics.RecordMandatoryModuleState(kymaName, moduleName, state) } + skrClient, err := r.getTargetClient(ctx, manifest) + if err != nil { + if !manifest.GetDeletionTimestamp().IsZero() && errors.Is(err, ErrAccessSecretNotFound) { + return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestClientInit, + err) + } + + manifest.SetStatus(manifest.GetStatus().WithState(shared.StateError).WithErr(err)) + return r.finishReconcile(ctx, manifest, metrics.ManifestClientInit, manifestStatus, err) + } + if manifest.IsUnmanaged() { if !manifest.GetDeletionTimestamp().IsZero() { return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestUnmanagedUpdate, nil) } + + if controllerutil.ContainsFinalizer(manifest, labelsremoval.LabelRemovalFinalizer) { + return r.handleLabelsRemovalFinalizer(ctx, skrClient, manifest) + } + if err := r.Delete(ctx, manifest); err != nil { return ctrl.Result{}, fmt.Errorf("manifestController: %w", err) } @@ -150,7 +185,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if manifest.GetDeletionTimestamp().IsZero() { partialMeta := r.partialObjectMetadata(manifest) - if controllerutil.AddFinalizer(partialMeta, defaultFinalizer) { + defaultFinalizerAdded := controllerutil.AddFinalizer(partialMeta, defaultFinalizer) + labelRemovalFinalizerAdded := controllerutil.AddFinalizer(partialMeta, labelsremoval.LabelRemovalFinalizer) + if defaultFinalizerAdded || labelRemovalFinalizerAdded { return r.ssaSpec(ctx, partialMeta, metrics.ManifestAddFinalizer) } } @@ -169,17 +206,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return r.updateManifest(ctx, manifest, metrics.ManifestInitSyncedOCIRef) } - skrClient, err := r.getTargetClient(ctx, manifest) - if err != nil { - if !manifest.GetDeletionTimestamp().IsZero() && errors.Is(err, ErrAccessSecretNotFound) { - return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestClientInit, - err) - } - - manifest.SetStatus(manifest.GetStatus().WithState(shared.StateError).WithErr(err)) - return r.finishReconcile(ctx, manifest, metrics.ManifestClientInit, manifestStatus, err) - } - target, current, err := r.renderResources(ctx, skrClient, manifest, spec) if err != nil { if util.IsConnectionRelatedError(err) { @@ -231,12 +257,29 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return r.finishReconcile(ctx, manifest, metrics.ManifestReconcileFinished, manifestStatus, nil) } +func (r *Reconciler) handleLabelsRemovalFinalizer(ctx context.Context, skrClient client.Client, + manifest *v1beta2.Manifest, +) (ctrl.Result, error) { + defaultCR, err := getCR(ctx, skrClient, manifest) + if err != nil { + return ctrl.Result{}, err + } + + if err := r.managedLabelRemovalService.RemoveManagedLabel(ctx, manifest, skrClient, + defaultCR); err != nil { + return ctrl.Result{}, err + } + + r.ManifestMetrics.RecordRequeueReason(metrics.ManifestResourcesLabelRemoval, queue.IntendedRequeue) + return ctrl.Result{Requeue: true}, nil +} + func (r *Reconciler) cleanupManifest(ctx context.Context, req ctrl.Request, manifest *v1beta2.Manifest, manifestStatus shared.Status, requeueReason metrics.ManifestRequeueReason, originalErr error, ) (ctrl.Result, error) { r.ManifestMetrics.RemoveManifestDuration(req.Name) r.cleanUpMandatoryModuleMetrics(manifest) - finalizersToRemove := []string{defaultFinalizer} + finalizersToRemove := []string{defaultFinalizer, labelsremoval.LabelRemovalFinalizer} if errors.Is(originalErr, ErrAccessSecretNotFound) || manifest.IsUnmanaged() { finalizersToRemove = manifest.GetFinalizers() } @@ -483,30 +526,22 @@ func (r *Reconciler) renderTargetResources(ctx context.Context, skrClient client return target, nil } -func checkCRDeletion(ctx context.Context, skrClient client.Client, manifest *v1beta2.Manifest) (bool, error) { - if manifest.Spec.Resource == nil { +func checkCRDeletion(ctx context.Context, skrClient client.Client, manifestCR *v1beta2.Manifest) (bool, + error, +) { + if manifestCR.Spec.Resource == nil { return true, nil } - name := manifest.Spec.Resource.GetName() - namespace := manifest.Spec.Resource.GetNamespace() - gvk := manifest.Spec.Resource.GroupVersionKind() - - resourceCR := &unstructured.Unstructured{} - resourceCR.SetGroupVersionKind(schema.GroupVersionKind{ - Group: gvk.Group, - Version: gvk.Version, - Kind: gvk.Kind, - }) - - if err := skrClient.Get(ctx, - client.ObjectKey{Name: name, Namespace: namespace}, resourceCR); err != nil { + resourceCR, err := getCR(ctx, skrClient, manifestCR) + if err != nil { if util.IsNotFound(err) { return true, nil } return false, fmt.Errorf("%w: failed to fetch default resource CR", err) } - return false, nil + + return resourceCR == nil, nil } func (r *Reconciler) pruneDiff(ctx context.Context, clnt Client, manifest *v1beta2.Manifest, @@ -633,9 +668,8 @@ func (r *Reconciler) configClient(ctx context.Context, manifest *v1beta2.Manifes func (r *Reconciler) finishReconcile(ctx context.Context, manifest *v1beta2.Manifest, requeueReason metrics.ManifestRequeueReason, previousStatus shared.Status, originalErr error, ) (ctrl.Result, error) { - if err := r.patchStatusIfDiffExist(ctx, manifest, previousStatus); err != nil { - r.Event(manifest, "Warning", "PatchStatus", err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to patch status: %w", err) + if err := r.manifestClient.PatchStatusIfDiffExist(ctx, manifest, previousStatus); err != nil { + return ctrl.Result{}, err } if originalErr != nil { r.ManifestMetrics.RecordRequeueReason(requeueReason, queue.UnexpectedRequeue) @@ -646,49 +680,25 @@ func (r *Reconciler) finishReconcile(ctx context.Context, manifest *v1beta2.Mani return ctrl.Result{RequeueAfter: requeueAfter}, nil } -func (r *Reconciler) patchStatusIfDiffExist(ctx context.Context, manifest *v1beta2.Manifest, - previousStatus shared.Status, -) error { - if hasStatusDiff(manifest.GetStatus(), previousStatus) { - resetNonPatchableField(manifest) - if err := r.Status().Patch(ctx, manifest, client.Apply, client.ForceOwnership, defaultFieldOwner); err != nil { - return fmt.Errorf("failed to patch status: %w", err) - } - } - - return nil -} - -func hasStatusDiff(first, second shared.Status) bool { - return first.State != second.State || first.LastOperation.Operation != second.LastOperation.Operation -} - func (r *Reconciler) ssaSpec(ctx context.Context, obj client.Object, requeueReason metrics.ManifestRequeueReason, ) (ctrl.Result, error) { - resetNonPatchableField(obj) r.ManifestMetrics.RecordRequeueReason(requeueReason, queue.IntendedRequeue) - if err := r.Patch(ctx, obj, client.Apply, client.ForceOwnership, defaultFieldOwner); err != nil { - r.Event(obj, "Warning", "PatchObject", err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to patch object: %w", err) + if err := r.manifestClient.SsaSpec(ctx, obj); err != nil { + return ctrl.Result{}, err } return ctrl.Result{Requeue: true}, nil } -func resetNonPatchableField(obj client.Object) { - obj.SetUID("") - obj.SetManagedFields(nil) - obj.SetResourceVersion("") -} - func (r *Reconciler) updateManifest(ctx context.Context, manifest *v1beta2.Manifest, requeueReason metrics.ManifestRequeueReason, ) (ctrl.Result, error) { r.ManifestMetrics.RecordRequeueReason(requeueReason, queue.IntendedRequeue) - if err := r.Update(ctx, manifest); err != nil { - r.Event(manifest, "Warning", "UpdateObject", err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to update object: %w", err) + + if err := r.manifestClient.UpdateManifest(ctx, manifest); err != nil { + return ctrl.Result{}, err } + return ctrl.Result{Requeue: true}, nil } @@ -708,3 +718,25 @@ func (r *Reconciler) cleanUpMandatoryModuleMetrics(manifest *v1beta2.Manifest) { r.MandatoryModuleMetrics.CleanupMetrics(kymaName, moduleName) } } + +func getCR(ctx context.Context, skrClient client.Client, manifest *v1beta2.Manifest) (*unstructured.Unstructured, + error, +) { + resourceCR := &unstructured.Unstructured{} + name := manifest.Spec.Resource.GetName() + namespace := manifest.Spec.Resource.GetNamespace() + gvk := manifest.Spec.Resource.GroupVersionKind() + + resourceCR.SetGroupVersionKind(schema.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + }) + + if err := skrClient.Get(ctx, + client.ObjectKey{Name: name, Namespace: namespace}, resourceCR); err != nil { + return nil, fmt.Errorf("%w: failed to fetch default resource CR", err) + } + + return resourceCR, nil +} diff --git a/internal/declarative/v2/reconciler_test.go b/internal/declarative/v2/reconciler_test.go index 0d1e4ddee14..6f706ddf94a 100644 --- a/internal/declarative/v2/reconciler_test.go +++ b/internal/declarative/v2/reconciler_test.go @@ -3,7 +3,6 @@ package v2 import ( "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -157,76 +156,3 @@ func Test_hasDiff(t *testing.T) { }) } } - -func Test_hasStatusDiff(t *testing.T) { - type args struct { - first shared.Status - second shared.Status - } - tests := []struct { - name string - args args - want bool - }{ - { - name: "Different Status", - args: args{ - first: shared.Status{ - State: shared.StateReady, - LastOperation: shared.LastOperation{ - Operation: "resources are ready", - LastUpdateTime: apimetav1.Now(), - }, - }, - second: shared.Status{ - State: shared.StateProcessing, - LastOperation: shared.LastOperation{ - Operation: "installing resources", - LastUpdateTime: apimetav1.Now(), - }, - }, - }, - want: true, - }, - { - name: "Same Status", - args: args{ - first: shared.Status{ - State: shared.StateReady, - LastOperation: shared.LastOperation{ - Operation: "resources are ready", - LastUpdateTime: apimetav1.Now(), - }, - }, - second: shared.Status{ - State: shared.StateReady, - LastOperation: shared.LastOperation{ - Operation: "resources are ready", - LastUpdateTime: apimetav1.NewTime(time.Now().Add(time.Hour)), - }, - }, - }, - want: false, - }, - { - name: "Empty Status", - args: args{ - first: shared.Status{}, - second: shared.Status{ - State: shared.StateReady, - LastOperation: shared.LastOperation{ - Operation: "resources are ready", - LastUpdateTime: apimetav1.NewTime(time.Now().Add(time.Hour)), - }, - }, - }, - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require.Equalf(t, tt.want, hasStatusDiff(tt.args.first, tt.args.second), "hasStatusDiff(%v, %v)", - tt.args.first, tt.args.second) - }) - } -} diff --git a/internal/manifest/labelsremoval/labels_removal.go b/internal/manifest/labelsremoval/labels_removal.go new file mode 100644 index 00000000000..5e397c1cb97 --- /dev/null +++ b/internal/manifest/labelsremoval/labels_removal.go @@ -0,0 +1,100 @@ +package labelsremoval + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/api/v1beta2" +) + +const LabelRemovalFinalizer = "label-removal-finalizer" + +type ManifestAPIClient interface { + UpdateManifest(ctx context.Context, manifest *v1beta2.Manifest) error +} + +type ManagedLabelRemovalService struct { + manifestClient ManifestAPIClient +} + +func NewManagedLabelRemovalService(manifestClient ManifestAPIClient) *ManagedLabelRemovalService { + return &ManagedLabelRemovalService{ + manifestClient: manifestClient, + } +} + +func (l *ManagedLabelRemovalService) RemoveManagedLabel(ctx context.Context, + manifest *v1beta2.Manifest, skrClient client.Client, defaultCR *unstructured.Unstructured, +) error { + if err := HandleLabelsRemovalFromResources(ctx, manifest, skrClient, defaultCR); err != nil { + return err + } + + controllerutil.RemoveFinalizer(manifest, LabelRemovalFinalizer) + return l.manifestClient.UpdateManifest(ctx, manifest) +} + +func HandleLabelsRemovalFromResources(ctx context.Context, manifestCR *v1beta2.Manifest, + skrClient client.Client, defaultCR *unstructured.Unstructured, +) error { + for _, res := range manifestCR.Status.Synced { + objectKey := client.ObjectKey{ + Name: res.Name, + Namespace: res.Namespace, + } + + obj := constructResource(res) + if err := skrClient.Get(ctx, objectKey, obj); err != nil { + return fmt.Errorf("failed to get resource, %w", err) + } + + if IsManagedLabelRemoved(obj) { + if err := skrClient.Update(ctx, obj); err != nil { + return fmt.Errorf("failed to update object: %w", err) + } + } + } + + if defaultCR == nil { + return nil + } + + if IsManagedLabelRemoved(defaultCR) { + if err := skrClient.Update(ctx, defaultCR); err != nil { + return fmt.Errorf("failed to update object: %w", err) + } + } + + return nil +} + +func constructResource(resource shared.Resource) *unstructured.Unstructured { + gvk := schema.GroupVersionKind{ + Group: resource.Group, + Version: resource.Version, + Kind: resource.Kind, + } + + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + + return obj +} + +func IsManagedLabelRemoved(resource *unstructured.Unstructured) bool { + labels := resource.GetLabels() + _, managedByLabelExists := labels[shared.ManagedBy] + if managedByLabelExists { + delete(labels, shared.ManagedBy) + } + + resource.SetLabels(labels) + + return managedByLabelExists +} diff --git a/internal/manifest/labelsremoval/labels_removal_test.go b/internal/manifest/labelsremoval/labels_removal_test.go new file mode 100644 index 00000000000..ec0f4588186 --- /dev/null +++ b/internal/manifest/labelsremoval/labels_removal_test.go @@ -0,0 +1,296 @@ +package labelsremoval_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + machineryruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/internal/manifest/labelsremoval" + "github.com/kyma-project/lifecycle-manager/internal/manifest/manifestclient" + "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" +) + +func Test_needsUpdateAfterLabelRemoval_WhenLabelsAreEmpty(t *testing.T) { + emptyLabels := map[string]string{} + res := &unstructured.Unstructured{} + res.SetLabels(emptyLabels) + actual := labelsremoval.IsManagedLabelRemoved(res) + + require.False(t, actual) + require.Equal(t, emptyLabels, res.GetLabels()) +} + +func Test_needsUpdateAfterLabelRemoval_WhenManagedByLabel(t *testing.T) { + labels := map[string]string{ + shared.ManagedBy: shared.ManagedByLabelValue, + } + expectedLabels := map[string]string{} + res := &unstructured.Unstructured{} + res.SetLabels(labels) + actual := labelsremoval.IsManagedLabelRemoved(res) + + require.True(t, actual) + require.Equal(t, expectedLabels, res.GetLabels()) +} + +func Test_handleLabelsRemovalFromResources_WhenManifestResourcesHaveLabels(t *testing.T) { + gvk := schema.GroupVersionKind{ + Group: "test-group", + Version: "v1", + Kind: "TestKind", + } + + status := shared.Status{ + Synced: []shared.Resource{ + { + Name: "test-resource-1", + Namespace: "test-1", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + }, + }, + { + Name: "test-resource-2", + Namespace: "test-2", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + }, + }, + }, + } + manifest := builder.NewManifestBuilder().WithStatus(status).Build() + + objs := []client.Object{ + &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "version": gvk.Version, + }, + }, + &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "version": gvk.Version, + }, + }, + } + objs[0].SetName("test-resource-1") + objs[0].SetNamespace("test-1") + objs[0].SetLabels(map[string]string{ + "operator.kyma-project.io/managed-by": "kyma", + }) + + objs[1].SetName("test-resource-2") + objs[1].SetNamespace("test-2") + objs[1].SetLabels(map[string]string{ + "operator.kyma-project.io/managed-by": "kyma", + }) + + scheme := machineryruntime.NewScheme() + err := v1beta2.AddToScheme(scheme) + require.NoError(t, err) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + + err = labelsremoval.HandleLabelsRemovalFromResources(context.TODO(), manifest, fakeClient, nil) + require.NoError(t, err) + + firstObj, secondObj := &unstructured.Unstructured{}, &unstructured.Unstructured{} + firstObj.SetGroupVersionKind(gvk) + err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: "test-resource-1", Namespace: "test-1"}, + firstObj) + require.NoError(t, err) + require.Empty(t, firstObj.GetLabels()) + + secondObj.SetGroupVersionKind(gvk) + err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: "test-resource-2", Namespace: "test-2"}, + secondObj) + require.NoError(t, err) + require.Empty(t, secondObj.GetLabels()) +} + +func Test_handleLabelsRemovalFromResources_WhenManifestResourcesAreNilAndNoDefaultCR(t *testing.T) { + manifest := builder.NewManifestBuilder().Build() + + scheme := machineryruntime.NewScheme() + err := v1beta2.AddToScheme(scheme) + require.NoError(t, err) + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + err = labelsremoval.HandleLabelsRemovalFromResources(context.TODO(), manifest, fakeClient, nil) + + require.NoError(t, err) +} + +func Test_handleLabelsRemovalFromResources_WhenManifestResourcesAndDefaultCRHaveLabels(t *testing.T) { + gvk := schema.GroupVersionKind{ + Group: "test-group", + Version: "v1", + Kind: "TestKind", + } + + status := shared.Status{ + Synced: []shared.Resource{ + { + Name: "test-resource-1", + Namespace: "test-1", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + }, + }, + }, + } + manifest := builder.NewManifestBuilder().WithStatus(status).Build() + + objs := []client.Object{ + &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "version": gvk.Version, + }, + }, + } + objs[0].SetName("test-resource-1") + objs[0].SetNamespace("test-1") + objs[0].SetLabels(map[string]string{ + "operator.kyma-project.io/managed-by": "kyma", + }) + + defaultCR := &unstructured.Unstructured{} + defaultCR.SetName("default-cr") + defaultCR.SetNamespace("default-ns") + defaultCR.SetGroupVersionKind(gvk) + defaultCR.SetLabels(map[string]string{ + "operator.kyma-project.io/managed-by": "kyma", + }) + + objs = append(objs, defaultCR) + + scheme := machineryruntime.NewScheme() + err := v1beta2.AddToScheme(scheme) + require.NoError(t, err) + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + + err = labelsremoval.HandleLabelsRemovalFromResources(context.TODO(), manifest, fakeClient, + defaultCR) + + require.NoError(t, err) + + firstObj := &unstructured.Unstructured{} + firstObj.SetGroupVersionKind(gvk) + err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: "test-resource-1", Namespace: "test-1"}, + firstObj) + require.NoError(t, err) + require.Empty(t, firstObj.GetLabels()) + + require.NoError(t, err) + require.Empty(t, defaultCR.GetLabels()) +} + +func Test_RemoveManagedLabel_WhenErrorIsReturned(t *testing.T) { + scheme := machineryruntime.NewScheme() + err := v1beta2.AddToScheme(scheme) + require.NoError(t, err) + + gvk := schema.GroupVersionKind{ + Group: "test-group", + Version: "v1", + Kind: "TestKind", + } + status := shared.Status{ + Synced: []shared.Resource{ + { + Name: "test-resource-1", + Namespace: "test-1", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + }, + }, + }, + } + manifest := builder.NewManifestBuilder().WithStatus(status).Build() + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + manifestClnt := manifestclient.NewManifestClient(nil, fakeClient) + svc := labelsremoval.NewManagedLabelRemovalService(manifestClnt) + + err = svc.RemoveManagedLabel(context.TODO(), manifest, fakeClient, nil) + require.ErrorContains(t, err, "failed to get resource") +} + +func Test_RemoveManagedLabel_WhenFinalizerIsRemoved(t *testing.T) { + scheme := machineryruntime.NewScheme() + err := v1beta2.AddToScheme(scheme) + require.NoError(t, err) + + gvk := schema.GroupVersionKind{ + Group: "test-group", + Version: "v1", + Kind: "TestKind", + } + status := shared.Status{ + Synced: []shared.Resource{ + { + Name: "test-resource-1", + Namespace: "test-1", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + }, + }, + }, + } + finalizers := []string{"label-removal-finalizer"} + manifest := builder.NewManifestBuilder().WithFinalizers(finalizers).WithStatus(status).Build() + + objs := []client.Object{ + &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "version": gvk.Version, + }, + }, + } + objs[0].SetName("test-resource-1") + objs[0].SetNamespace("test-1") + objs[0].SetLabels(map[string]string{ + "operator.kyma-project.io/managed-by": "kyma", + }) + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(manifest).WithObjects(objs...).Build() + + manifestClnt := manifestclient.NewManifestClient(nil, fakeClient) + svc := labelsremoval.NewManagedLabelRemovalService(manifestClnt) + + err = svc.RemoveManagedLabel(context.TODO(), manifest, fakeClient, nil) + require.NoError(t, err) + + err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: manifest.GetName(), Namespace: manifest.GetNamespace()}, + manifest) + require.NoError(t, err) + require.Empty(t, manifest.GetFinalizers()) +} diff --git a/internal/manifest/manifestclient/manifest_client.go b/internal/manifest/manifestclient/manifest_client.go new file mode 100644 index 00000000000..4eddf5a03e0 --- /dev/null +++ b/internal/manifest/manifestclient/manifest_client.go @@ -0,0 +1,67 @@ +package manifestclient + +import ( + "context" + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/internal/event" +) + +const defaultFieldOwner client.FieldOwner = "declarative.kyma-project.io/applier" + +type ManifestClient struct { + client.Client + event.Event +} + +func NewManifestClient(event event.Event, kcpClient client.Client) *ManifestClient { + return &ManifestClient{ + Event: event, + Client: kcpClient, + } +} + +func (m *ManifestClient) UpdateManifest(ctx context.Context, manifest *v1beta2.Manifest) error { + if err := m.Update(ctx, manifest); err != nil { + m.Warning(manifest, "UpdateObject", err) + return fmt.Errorf("failed to update object: %w", err) + } + return nil +} + +func (m *ManifestClient) PatchStatusIfDiffExist(ctx context.Context, manifest *v1beta2.Manifest, + previousStatus shared.Status, +) error { + if HasStatusDiff(manifest.GetStatus(), previousStatus) { + resetNonPatchableField(manifest) + if err := m.Status().Patch(ctx, manifest, client.Apply, client.ForceOwnership, defaultFieldOwner); err != nil { + m.Warning(manifest, "PatchStatus", err) + return fmt.Errorf("failed to patch status: %w", err) + } + } + + return nil +} + +func (m *ManifestClient) SsaSpec(ctx context.Context, obj client.Object) error { + resetNonPatchableField(obj) + if err := m.Patch(ctx, obj, client.Apply, client.ForceOwnership, defaultFieldOwner); err != nil { + m.Warning(obj, "PatchObject", err) + return fmt.Errorf("failed to patch object: %w", err) + } + return nil +} + +func HasStatusDiff(first, second shared.Status) bool { + return first.State != second.State || first.LastOperation.Operation != second.LastOperation.Operation +} + +func resetNonPatchableField(obj client.Object) { + obj.SetUID("") + obj.SetManagedFields(nil) + obj.SetResourceVersion("") +} diff --git a/internal/manifest/manifestclient/manifest_client_test.go b/internal/manifest/manifestclient/manifest_client_test.go new file mode 100644 index 00000000000..45bf1705610 --- /dev/null +++ b/internal/manifest/manifestclient/manifest_client_test.go @@ -0,0 +1,86 @@ +package manifestclient_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/internal/manifest/manifestclient" +) + +func Test_hasStatusDiff(t *testing.T) { + type args struct { + first shared.Status + second shared.Status + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Different Status", + args: args{ + first: shared.Status{ + State: shared.StateReady, + LastOperation: shared.LastOperation{ + Operation: "resources are ready", + LastUpdateTime: apimetav1.Now(), + }, + }, + second: shared.Status{ + State: shared.StateProcessing, + LastOperation: shared.LastOperation{ + Operation: "installing resources", + LastUpdateTime: apimetav1.Now(), + }, + }, + }, + want: true, + }, + { + name: "Same Status", + args: args{ + first: shared.Status{ + State: shared.StateReady, + LastOperation: shared.LastOperation{ + Operation: "resources are ready", + LastUpdateTime: apimetav1.Now(), + }, + }, + second: shared.Status{ + State: shared.StateReady, + LastOperation: shared.LastOperation{ + Operation: "resources are ready", + LastUpdateTime: apimetav1.NewTime(time.Now().Add(time.Hour)), + }, + }, + }, + want: false, + }, + { + name: "Empty Status", + args: args{ + first: shared.Status{}, + second: shared.Status{ + State: shared.StateReady, + LastOperation: shared.LastOperation{ + Operation: "resources are ready", + LastUpdateTime: apimetav1.NewTime(time.Now().Add(time.Hour)), + }, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equalf(t, tt.want, manifestclient.HasStatusDiff(tt.args.first, tt.args.second), + "hasStatusDiff(%v, %v)", + tt.args.first, tt.args.second) + }) + } +} diff --git a/internal/pkg/metrics/manifest.go b/internal/pkg/metrics/manifest.go index b358a75b457..149501bd690 100644 --- a/internal/pkg/metrics/manifest.go +++ b/internal/pkg/metrics/manifest.go @@ -32,6 +32,7 @@ const ( ManifestUnauthorized ManifestRequeueReason = "manifest_unauthorized" ManifestReconcileFinished ManifestRequeueReason = "manifest_reconcile_finished" ManifestUnmanagedUpdate ManifestRequeueReason = "manifest_unmanaged_update" + ManifestResourcesLabelRemoval ManifestRequeueReason = "manifest_labels_removal" ) type ManifestMetrics struct { diff --git a/pkg/testutils/builder/manifest.go b/pkg/testutils/builder/manifest.go index 25ca91f2941..e7a49626898 100644 --- a/pkg/testutils/builder/manifest.go +++ b/pkg/testutils/builder/manifest.go @@ -85,6 +85,12 @@ func (mb ManifestBuilder) WithStatus(status shared.Status) ManifestBuilder { return mb } +// WithFinalizers sets v1beta2.Manifest.Finalizers. +func (mb ManifestBuilder) WithFinalizers(finalizers []string) ManifestBuilder { + mb.manifest.Finalizers = finalizers + return mb +} + // Build returns the built v1beta2.Manifest. func (mb ManifestBuilder) Build() *v1beta2.Manifest { return mb.manifest diff --git a/pkg/testutils/unstructured.go b/pkg/testutils/unstructured.go index 8a8d36a7b76..308345e85a6 100644 --- a/pkg/testutils/unstructured.go +++ b/pkg/testutils/unstructured.go @@ -2,6 +2,7 @@ package testutils import ( "context" + "errors" "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -14,6 +15,11 @@ import ( "github.com/kyma-project/lifecycle-manager/pkg/util" ) +var ( + ErrLabelNotFound = errors.New("label is not found") + ErrLabelValueNotCorrect = errors.New("label value is not as expected") +) + func DeleteCR(ctx context.Context, clnt client.Client, obj client.Object) error { err := clnt.Delete(ctx, obj) if err != nil && util.IsNotFound(err) { @@ -69,6 +75,28 @@ func IsResourceVersionSame(ctx context.Context, clnt client.Client, return false, nil } +func HasExpectedLabel(ctx context.Context, clnt client.Client, + objectKey client.ObjectKey, + gvk schema.GroupVersionKind, expectedLabelKey string, expectedLabelValue string, +) error { + obj, err := GetCR(ctx, clnt, objectKey, gvk) + if err != nil { + return err + } + + labels := obj.GetLabels() + value, exists := labels[expectedLabelKey] + if !exists { + return ErrLabelNotFound + } + + if value != expectedLabelValue { + return ErrLabelValueNotCorrect + } + + return nil +} + func CreateCR(ctx context.Context, clnt client.Client, obj client.Object) error { err := clnt.Create(ctx, obj) if !apierrors.IsAlreadyExists(err) { diff --git a/tests/e2e/manifest_reconciliation_test.go b/tests/e2e/manifest_reconciliation_test.go index d18ae4dfdaa..5b907eba9dd 100644 --- a/tests/e2e/manifest_reconciliation_test.go +++ b/tests/e2e/manifest_reconciliation_test.go @@ -1,10 +1,9 @@ package e2e_test import ( - templatev1alpha1 "github.com/kyma-project/template-operator/api/v1alpha1" - "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" + templatev1alpha1 "github.com/kyma-project/template-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -43,6 +42,7 @@ var _ = Describe("Manifest Skip Reconciliation Label", Ordered, func() { WithArguments(TestModuleCRName, RemoteNamespace, skrClient, shared.ManagedBy, shared.ManagedByLabelValue). Should(Succeed()) + By("And the KCP Kyma CR is in a \"Ready\" State") Eventually(KymaIsInState). WithContext(ctx). diff --git a/tests/e2e/module_unmanage_test.go b/tests/e2e/module_unmanage_test.go index a55566d8bd6..406b59b0103 100644 --- a/tests/e2e/module_unmanage_test.go +++ b/tests/e2e/module_unmanage_test.go @@ -2,14 +2,15 @@ package e2e_test import ( templatev1alpha1 "github.com/kyma-project/template-operator/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" + . "github.com/kyma-project/lifecycle-manager/pkg/testutils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - . "github.com/kyma-project/lifecycle-manager/pkg/testutils" ) var _ = Describe("Unmanaging Kyma Module", Ordered, func() { @@ -20,6 +21,7 @@ var _ = Describe("Unmanaging Kyma Module", Ordered, func() { module := NewTemplateOperator(v1beta2.DefaultChannel) Context("Given SKR Cluster", func() { + var manifestResources []shared.Resource It("When Kyma Module is enabled on SKR Kyma CR", func() { Eventually(EnableModule). WithContext(ctx). @@ -38,6 +40,29 @@ var _ = Describe("Unmanaging Kyma Module", Ordered, func() { WithContext(ctx). WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient, shared.StateReady). Should(Succeed()) + By("And all manifest resources and default CR have managed-by label") + manifest, err := GetManifest(ctx, kcpClient, kyma.GetName(), kyma.GetNamespace(), + module.Name) + Expect(err).Should(Succeed()) + manifestResources = manifest.Status.Synced + for _, resource := range manifestResources { + objectKey := client.ObjectKey{Name: resource.Name, Namespace: resource.Namespace} + gvk := schema.GroupVersionKind{ + Group: resource.Group, + Version: resource.Version, + Kind: resource.Kind, + } + Eventually(HasExpectedLabel). + WithContext(ctx). + WithArguments(skrClient, objectKey, gvk, + shared.ManagedBy, shared.ManagedByLabelValue).Should(Succeed()) + + } + Eventually(CheckSampleCRHasExpectedLabel). + WithContext(ctx). + WithArguments(TestModuleCRName, RemoteNamespace, skrClient, shared.ManagedBy, + shared.ManagedByLabelValue). + Should(Succeed()) }) It("When Kyma Module is unmanaged", func() { @@ -66,6 +91,26 @@ var _ = Describe("Unmanaging Kyma Module", Ordered, func() { "apps", "v1", "Deployment", skrClient). Should(Succeed()) + By("And all manifest resources and default CR no longer have managed-by labels") + for _, resource := range manifestResources { + objectKey := client.ObjectKey{Name: resource.Name, Namespace: resource.Namespace} + gvk := schema.GroupVersionKind{ + Group: resource.Group, + Version: resource.Version, + Kind: resource.Kind, + } + Eventually(HasExpectedLabel). + WithContext(ctx). + WithArguments(skrClient, objectKey, gvk, + shared.ManagedBy, shared.ManagedByLabelValue).Should(Equal(ErrLabelNotFound)) + } + + Eventually(CheckSampleCRHasExpectedLabel). + WithContext(ctx). + WithArguments(TestModuleCRName, RemoteNamespace, skrClient, shared.ManagedBy, + shared.ManagedByLabelValue). + Should(Equal(ErrLabelNotExistOnCR)) + By("And KCP Kyma CR is in \"Ready\" State") Eventually(KymaIsInState). WithContext(ctx). diff --git a/tests/e2e/utils_test.go b/tests/e2e/utils_test.go index ee79bb00951..7eafb38a336 100644 --- a/tests/e2e/utils_test.go +++ b/tests/e2e/utils_test.go @@ -28,7 +28,7 @@ var ( errKymaNotInExpectedState = errors.New("kyma CR not in expected state") errModuleNotExisting = errors.New("module does not exists in KymaCR") errLabelNotExistOnNamespace = errors.New("label does not exist on namespace") - errLabelNotExistOnSampleCR = errors.New("label does not exist on CustomResource") + ErrLabelNotExistOnCR = errors.New("label does not exist on CustomResource") ) const ( @@ -226,8 +226,8 @@ func CheckSampleCRHasExpectedLabel(ctx context.Context, name, namespace string, labelKey, labelValue string, ) error { customResource, err := GetCR(ctx, clnt, client.ObjectKey{Name: name, Namespace: namespace}, schema.GroupVersionKind{ - Group: "operator.kyma-project.io", - Version: "v1alpha1", + Group: templatev1alpha1.GroupVersion.Group, + Version: templatev1alpha1.GroupVersion.Version, Kind: string(templatev1alpha1.SampleKind), }) if err != nil { @@ -236,7 +236,7 @@ func CheckSampleCRHasExpectedLabel(ctx context.Context, name, namespace string, labels := customResource.GetLabels() if labels == nil || labels[labelKey] != labelValue { - return errLabelNotExistOnSampleCR + return ErrLabelNotExistOnCR } return nil diff --git a/tests/integration/controller/manifest/custom_resource_check/suite_test.go b/tests/integration/controller/manifest/custom_resource_check/suite_test.go index b3e52e8511c..93f2a1166f5 100644 --- a/tests/integration/controller/manifest/custom_resource_check/suite_test.go +++ b/tests/integration/controller/manifest/custom_resource_check/suite_test.go @@ -37,11 +37,14 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "github.com/kyma-project/lifecycle-manager/api" + "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal" declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" + "github.com/kyma-project/lifecycle-manager/internal/event" "github.com/kyma-project/lifecycle-manager/internal/manifest" "github.com/kyma-project/lifecycle-manager/internal/manifest/img" + "github.com/kyma-project/lifecycle-manager/internal/manifest/manifestclient" "github.com/kyma-project/lifecycle-manager/internal/manifest/statecheck" "github.com/kyma-project/lifecycle-manager/internal/pkg/metrics" "github.com/kyma-project/lifecycle-manager/pkg/log" @@ -141,6 +144,8 @@ var _ = BeforeSuite(func() { statefulChecker := statecheck.NewStatefulSetStateCheck() deploymentChecker := statecheck.NewDeploymentStateCheck() extractor := img.NewPathExtractor() + testEventRec := event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName)) + manifestClient := manifestclient.NewManifestClient(testEventRec, kcpClient) reconciler = declarativev2.NewFromManager(mgr, queue.RequeueIntervals{ Success: 1 * time.Second, Busy: 1 * time.Second, @@ -148,6 +153,7 @@ var _ = BeforeSuite(func() { Warning: 1 * time.Second, }, metrics.NewManifestMetrics(metrics.NewSharedMetrics()), metrics.NewMandatoryModulesMetrics(), + manifestClient, manifest.NewSpecResolver(keyChainLookup, extractor), declarativev2.WithRemoteTargetCluster( func(_ context.Context, _ declarativev2.Object) (*declarativev2.ClusterInfo, error) { diff --git a/tests/integration/controller/manifest/suite_test.go b/tests/integration/controller/manifest/suite_test.go index 5df7ec83b75..3ee5e72dfc2 100644 --- a/tests/integration/controller/manifest/suite_test.go +++ b/tests/integration/controller/manifest/suite_test.go @@ -37,11 +37,14 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "github.com/kyma-project/lifecycle-manager/api" + "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal" declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" + "github.com/kyma-project/lifecycle-manager/internal/event" "github.com/kyma-project/lifecycle-manager/internal/manifest" "github.com/kyma-project/lifecycle-manager/internal/manifest/img" + "github.com/kyma-project/lifecycle-manager/internal/manifest/manifestclient" "github.com/kyma-project/lifecycle-manager/internal/pkg/metrics" "github.com/kyma-project/lifecycle-manager/pkg/log" "github.com/kyma-project/lifecycle-manager/pkg/queue" @@ -138,6 +141,8 @@ var _ = BeforeSuite(func() { kcp := &declarativev2.ClusterInfo{Config: cfg, Client: kcpClient} keyChainLookup := manifest.NewKeyChainProvider(kcp.Client) extractor := img.NewPathExtractor() + testEventRec := event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName)) + manifestClient := manifestclient.NewManifestClient(testEventRec, kcpClient) reconciler = declarativev2.NewFromManager(mgr, queue.RequeueIntervals{ Success: 1 * time.Second, Busy: 1 * time.Second, @@ -145,6 +150,7 @@ var _ = BeforeSuite(func() { Warning: 1 * time.Second, }, metrics.NewManifestMetrics(metrics.NewSharedMetrics()), metrics.NewMandatoryModulesMetrics(), + manifestClient, manifest.NewSpecResolver(keyChainLookup, extractor), declarativev2.WithRemoteTargetCluster( func(_ context.Context, _ declarativev2.Object) (*declarativev2.ClusterInfo, error) { diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index 50122cafa0f..0f142680e32 100644 --- a/unit-test-coverage.yaml +++ b/unit-test-coverage.yaml @@ -6,6 +6,7 @@ packages: internal/manifest/statecheck: 71 internal/manifest/filemutex: 100 internal/manifest: 10 + internal/manifest/labelsremoval: 93 internal/istio: 63.3 internal/pkg/resources: 91.7 internal/remote: 6.4