chore: Provide condition and metrics for Manifests stuck in Deleting due to DefaultCR Warning

.golangci.yaml

@@ -7,6 +7,7 @@ linters:
     - exportloopref # deprecated (since v1.60.2), replaced by copyloopvar
     - gomnd # deprecated (since v1.58.0), renamed to mnd
     - lll
+    - execinquery # deprecated (since v1.58.0) due to: The repository of the linter has been archived by the owner.
     - nlreturn # too strict and mostly code is not more readable
     - sqlclosecheck # not needed for this project
     - paralleltest # should be enabled consciously for long running tests

api/v1beta2/manifest_types.go

@@ -17,6 +17,8 @@
 package v1beta2
 
 import (
+	"fmt"
+
 	apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	machineryruntime "k8s.io/apimachinery/pkg/runtime"
@@ -32,6 +34,8 @@
 	RawManifestLayer LayerName = "raw-manifest"
 )
 
+var ErrLabelNotFound = fmt.Errorf("label not found")
+
 // InstallInfo defines installation information.
 type InstallInfo struct {
 	// Source in the ImageSpec format
@@ -148,6 +152,22 @@
 	return manifest.GetLabels() != nil && manifest.GetLabels()[shared.SkipReconcileLabel] == shared.EnableLabelValue
 }
 
+func (manifest *Manifest) GetKymaName() (string, error) {
+	kymaName, found := manifest.GetLabels()[shared.KymaName]
+	if !found {
+		return "", ErrLabelNotFound
+	}
+	return kymaName, nil
+}
+
+func (manifest *Manifest) GetModuleName() (string, error) {
+	moduleName, found := manifest.GetLabels()[shared.ModuleName]
+	if !found {
+		return "", ErrLabelNotFound
+	}
+	return moduleName, nil
+}
+
 func (manifest *Manifest) GetChannel() (string, bool) {
 	channel, found := manifest.Labels[shared.ChannelLabel]
 	if !found {

cmd/main.go

@@ -181,10 +181,12 @@ func setupManager(flagVar *flags.FlagVar, cacheOptions cache.Options, scheme *ma
 	descriptorProvider := provider.NewCachedDescriptorProvider()
 	kymaMetrics := metrics.NewKymaMetrics(sharedMetrics)
 	mandatoryModulesMetrics := metrics.NewMandatoryModulesMetrics()
+	moduleMetrics := metrics.NewModuleMetrics()
 	setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager,
 		kymaMetrics,
 		setupLog)
-	setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics, setupLog, eventRecorder)
+	setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics, moduleMetrics, setupLog,
+		eventRecorder)
 	setupMandatoryModuleReconciler(mgr, descriptorProvider, flagVar, options, mandatoryModulesMetrics, setupLog)
 	setupMandatoryModuleDeletionReconciler(mgr, descriptorProvider, eventRecorder, flagVar, options, setupLog)
 	if flagVar.EnablePurgeFinalizer {
@@ -382,8 +384,7 @@ 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,
+	moduleMetrics *metrics.ModuleMetrics, setupLog logr.Logger, event event.Event,
 ) {
 	options.MaxConcurrentReconciles = flagVar.MaxConcurrentManifestReconciles
 	options.RateLimiter = internal.ManifestRateLimiter(flagVar.FailureBaseDelay,
@@ -402,7 +403,7 @@ func setupManifestReconciler(mgr ctrl.Manager, flagVar *flags.FlagVar, options c
 		}, manifest.SetupOptions{
 			ListenerAddr:                 flagVar.ManifestListenerAddr,
 			EnableDomainNameVerification: flagVar.EnableDomainNameVerification,
-		}, metrics.NewManifestMetrics(sharedMetrics), mandatoryModulesMetrics,
+		}, metrics.NewManifestMetrics(sharedMetrics), mandatoryModulesMetrics, moduleMetrics,
 		manifestClient,
 	); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "Manifest")

internal/controller/manifest/controller.go

@@ -15,11 +15,9 @@ import (
 // +kubebuilder:rbac:groups=operator.kyma-project.io,resources=manifests/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=operator.kyma-project.io,resources=manifests/finalizers,verbs=update
 
-func NewReconciler(mgr manager.Manager,
-	requeueIntervals queue.RequeueIntervals,
-	manifestMetrics *metrics.ManifestMetrics,
-	mandatoryModulesMetrics *metrics.MandatoryModulesMetrics,
-	manifestClient declarativev2.ManifestAPIClient,
+func NewReconciler(mgr manager.Manager, requeueIntervals queue.RequeueIntervals,
+	manifestMetrics *metrics.ManifestMetrics, mandatoryModulesMetrics *metrics.MandatoryModulesMetrics,
+	moduleMetrics *metrics.ModuleMetrics, manifestClient declarativev2.ManifestAPIClient,
 ) *declarativev2.Reconciler {
 	kcp := &declarativev2.ClusterInfo{
 		Client: mgr.GetClient(),
@@ -31,7 +29,7 @@ func NewReconciler(mgr manager.Manager,
 	statefulChecker := statecheck.NewStatefulSetStateCheck()
 	deploymentChecker := statecheck.NewDeploymentStateCheck()
 	return declarativev2.NewFromManager(
-		mgr, requeueIntervals, manifestMetrics, mandatoryModulesMetrics, manifestClient,
+		mgr, requeueIntervals, manifestMetrics, mandatoryModulesMetrics, moduleMetrics, manifestClient,
 		manifest.NewSpecResolver(keyChainLookup, extractor),
 		declarativev2.WithCustomStateCheck(statecheck.NewManagerStateCheck(statefulChecker, deploymentChecker)),
 		declarativev2.WithRemoteTargetCluster(lookup.ConfigResolver),

internal/controller/manifest/setup.go

@@ -35,12 +35,9 @@ type SetupOptions struct {
 	EnableDomainNameVerification bool
 }
 
-func SetupWithManager(mgr manager.Manager,
-	opts ctrlruntime.Options,
-	requeueIntervals queue.RequeueIntervals,
-	settings SetupOptions,
-	manifestMetrics *metrics.ManifestMetrics,
-	mandatoryModulesMetrics *metrics.MandatoryModulesMetrics,
+func SetupWithManager(mgr manager.Manager, opts ctrlruntime.Options, requeueIntervals queue.RequeueIntervals,
+	settings SetupOptions, manifestMetrics *metrics.ManifestMetrics,
+	mandatoryModulesMetrics *metrics.MandatoryModulesMetrics, moduleMetrics *metrics.ModuleMetrics,
 	manifestClient declarativev2.ManifestAPIClient,
 ) error {
 	var verifyFunc watcherevent.Verify
@@ -86,7 +83,7 @@ func SetupWithManager(mgr manager.Manager,
 				predicate.LabelChangedPredicate{}))).
 		WatchesRawSource(skrEventChannel).
 		WithOptions(opts).
-		Complete(NewReconciler(mgr, requeueIntervals, manifestMetrics, mandatoryModulesMetrics,
+		Complete(NewReconciler(mgr, requeueIntervals, manifestMetrics, mandatoryModulesMetrics, moduleMetrics,
 			manifestClient)); err != nil {
 		return fmt.Errorf("failed to setup manager for manifest controller: %w", err)
 	}

internal/declarative/v2/reconciler.go

@@ -41,17 +41,14 @@ const (
 	defaultFieldOwner      client.FieldOwner = "declarative.kyma-project.io/applier"
 )
 
-func NewFromManager(mgr manager.Manager,
-	requeueIntervals queue.RequeueIntervals,
-	metrics *metrics.ManifestMetrics,
-	mandatoryModulesMetrics *metrics.MandatoryModulesMetrics,
-	manifestAPIClient ManifestAPIClient,
-	specResolver SpecResolver,
-	options ...Option,
+func NewFromManager(mgr manager.Manager, requeueIntervals queue.RequeueIntervals, metrics *metrics.ManifestMetrics,
+	mandatoryModulesMetrics *metrics.MandatoryModulesMetrics, moduleMetrics *metrics.ModuleMetrics,
+	manifestAPIClient ManifestAPIClient, specResolver SpecResolver, options ...Option,
 ) *Reconciler {
 	reconciler := &Reconciler{}
 	reconciler.ManifestMetrics = metrics
 	reconciler.MandatoryModuleMetrics = mandatoryModulesMetrics
+	reconciler.ModuleMetrics = moduleMetrics
 	reconciler.RequeueIntervals = requeueIntervals
 	reconciler.specResolver = specResolver
 	reconciler.manifestClient = manifestAPIClient
@@ -78,6 +75,7 @@ type Reconciler struct {
 	*Options
 	ManifestMetrics            *metrics.ManifestMetrics
 	MandatoryModuleMetrics     *metrics.MandatoryModulesMetrics
+	ModuleMetrics              *metrics.ModuleMetrics
 	specResolver               SpecResolver
 	manifestClient             ManifestAPIClient
 	managedLabelRemovalService ManagedLabelRemoval
@@ -119,8 +117,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	skrClient, err := r.getTargetClient(ctx, manifest)
 	if err != nil {
 		if !manifest.GetDeletionTimestamp().IsZero() && errors.Is(err, common.ErrAccessSecretNotFound) {
-			return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestClientInit,
-				err)
+			return r.cleanupManifest(ctx, manifest, manifestStatus, metrics.ManifestClientInit, err)
 		}
 
 		manifest.SetStatus(manifest.GetStatus().WithState(shared.StateError).WithErr(err))
@@ -129,7 +126,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	if manifest.IsUnmanaged() {
 		if !manifest.GetDeletionTimestamp().IsZero() {
-			return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestUnmanagedUpdate, nil)
+			return r.cleanupManifest(ctx, manifest, manifestStatus, metrics.ManifestUnmanagedUpdate, nil)
 		}
 
 		if controllerutil.ContainsFinalizer(manifest, labelsremoval.LabelRemovalFinalizer) {
@@ -143,7 +140,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 
 	if manifest.GetDeletionTimestamp().IsZero() {
-		if finalizer.AddFinalizers(manifest) {
+		if finalizer.FinalizersUpdateRequired(manifest) {
 			return r.ssaSpec(ctx, manifest, metrics.ManifestAddFinalizer)
 		}
 	}
@@ -152,7 +149,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	if err != nil {
 		manifest.SetStatus(manifest.GetStatus().WithState(shared.StateError).WithErr(err))
 		if !manifest.GetDeletionTimestamp().IsZero() {
-			return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestParseSpec, err)
+			return r.cleanupManifest(ctx, manifest, manifestStatus, metrics.ManifestParseSpec, err)
 		}
 		return r.finishReconcile(ctx, manifest, metrics.ManifestParseSpec, manifestStatus, err)
 	}
@@ -181,24 +178,27 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 
 	if err := modulecr.NewClient(skrClient).RemoveModuleCR(ctx, r.Client, manifest); err != nil {
-		if errors.Is(err, modulecr.ErrRequeueRequired) {
+		if errors.Is(err, finalizer.ErrRequeueRequired) {
 			r.ManifestMetrics.RecordRequeueReason(metrics.ManifestPreDeleteEnqueueRequired, queue.IntendedRequeue)
 			return ctrl.Result{Requeue: true}, nil
 		}
 		return r.finishReconcile(ctx, manifest, metrics.ManifestPreDelete, manifestStatus, err)
 	}
 
-	if err = r.syncResources(ctx, skrClient, manifest, target); err != nil {
-		if errors.Is(err, modulecr.ErrRequeueRequired) {
-			r.ManifestMetrics.RecordRequeueReason(metrics.ManifestSyncResourcesEnqueueRequired, queue.IntendedRequeue)
-			return ctrl.Result{Requeue: true}, nil
-		}
+	if err := r.syncResources(ctx, skrClient, manifest, target); err != nil {
 		if errors.Is(err, ErrClientUnauthorized) {
 			r.invalidateClientCache(ctx, manifest)
 		}
 		return r.finishReconcile(ctx, manifest, metrics.ManifestSyncResources, manifestStatus, err)
 	}
 
+	if err := r.syncManifestState(ctx, skrClient, manifest, target); err != nil {
+		if errors.Is(err, finalizer.ErrRequeueRequired) {
+			r.ManifestMetrics.RecordRequeueReason(metrics.ManifestSyncResourcesEnqueueRequired, queue.IntendedRequeue)
+			return ctrl.Result{Requeue: true}, nil
+		}
+		return r.finishReconcile(ctx, manifest, metrics.ManifestSyncState, manifestStatus, err)
+	}
 	// This situation happens when manifest get new installation layer to update resources,
 	// we need to make sure all updates successfully before we can update synced oci ref
 	if requireUpdateSyncedOCIRefAnnotation(manifest, spec.OCIRef) {
@@ -207,7 +207,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 
 	if !manifest.GetDeletionTimestamp().IsZero() {
-		return r.cleanupManifest(ctx, req, manifest, manifestStatus, metrics.ManifestReconcileFinished, nil)
+		return r.cleanupManifest(ctx, manifest, manifestStatus, metrics.ManifestReconcileFinished, nil)
 	}
 
 	return r.finishReconcile(ctx, manifest, metrics.ManifestReconcileFinished, manifestStatus, nil)
@@ -230,13 +230,20 @@ func (r *Reconciler) handleLabelsRemovalFinalizer(ctx context.Context, skrClient
 	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,
+func (r *Reconciler) cleanupManifest(ctx context.Context, manifest *v1beta2.Manifest, manifestStatus shared.Status,
+	requeueReason metrics.ManifestRequeueReason, originalErr error,
 ) (ctrl.Result, error) {
-	r.ManifestMetrics.RemoveManifestDuration(req.Name)
-	r.cleanUpMandatoryModuleMetrics(manifest)
-
-	if finalizer.RemoveFinalizers(manifest, originalErr) {
+	err := r.cleanupMetrics(manifest)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+	var finalizerRemoved bool
+	if errors.Is(originalErr, common.ErrAccessSecretNotFound) || manifest.IsUnmanaged() {
+		finalizerRemoved = finalizer.RemoveAllFinalizers(manifest)
+	} else {
+		finalizerRemoved = finalizer.RemoveRequiredFinalizers(manifest)
+	}
+	if finalizerRemoved {
 		return r.updateManifest(ctx, manifest, requeueReason)
 	}
 	if manifest.GetStatus().State != shared.StateWarning {
@@ -246,6 +253,25 @@ func (r *Reconciler) cleanupManifest(ctx context.Context, req ctrl.Request, mani
 	return r.finishReconcile(ctx, manifest, requeueReason, manifestStatus, originalErr)
 }
 
+func (r *Reconciler) cleanupMetrics(manifest *v1beta2.Manifest) error {
+	kymaName, err := manifest.GetKymaName()
+	if err != nil {
+		return fmt.Errorf("failed to get kyma name: %w", err)
+	}
+	moduleName, err := manifest.GetModuleName()
+	if err != nil {
+		return fmt.Errorf("failed to get module name: %w", err)
+	}
+	r.ManifestMetrics.CleanupMetrics(manifest.GetName())
+
+	if manifest.IsMandatoryModule() {
+		r.MandatoryModuleMetrics.CleanupMetrics(kymaName, moduleName)
+	}
+
+	r.ModuleMetrics.CleanupMetrics(kymaName, moduleName)
+	return nil
+}
+
 func (r *Reconciler) invalidateClientCache(ctx context.Context, manifest *v1beta2.Manifest) {
 	if r.ClientCacheKeyFn != nil {
 		clientsCacheKey, ok := r.ClientCacheKeyFn(ctx, manifest)
@@ -277,7 +303,7 @@ func (r *Reconciler) renderResources(ctx context.Context, skrClient Client, mani
 		manifest.SetStatus(manifestStatus.WithState(shared.StateError).WithErr(err))
 		return nil, nil, err
 	}
-	status.UpdateResourcesCondition(manifest)
+	status.ConfirmResourcesCondition(manifest)
 	return target, current, nil
 }
 
@@ -303,13 +329,31 @@ func (r *Reconciler) syncResources(ctx context.Context, skrClient Client, manife
 		}
 		return ErrWarningResourceSyncStateDiff
 	}
+	return nil
+}
 
-	if err := modulecr.NewClient(skrClient).CreateCR(ctx, r.Client, manifest); err != nil {
+func (r *Reconciler) syncManifestState(ctx context.Context, skrClient Client, manifest *v1beta2.Manifest,
+	target []*resource.Info,
+) error {
+	manifestStatus := manifest.GetStatus()
+
+	moduleCRState, err := modulecr.NewClient(skrClient).SyncModuleCR(ctx, manifest)
+	if err != nil {
 		manifest.SetStatus(manifestStatus.WithState(shared.StateError).WithErr(err))
 		return err
 	}
-
+	if err := finalizer.EnsureCRFinalizer(ctx, r.Client, manifest); err != nil {
+		return err
+	}
 	if !manifest.GetDeletionTimestamp().IsZero() {
+		if moduleCRState == shared.StateWarning {
+			if err := r.RecordModuleCRWarningCondition(manifest); err != nil {
+				return err
+			}
+			status.ConfirmModuleCRCondition(manifest)
+		} else if err := r.RemoveModuleCRWarningCondition(manifest); err != nil {
+			return err
+		}
 		return status.SetManifestState(manifest, shared.StateDeleting)
 	}
 
@@ -321,6 +365,32 @@ func (r *Reconciler) syncResources(ctx context.Context, skrClient Client, manife
 	return status.SetManifestState(manifest, managerState)
 }
 
+func (r *Reconciler) RecordModuleCRWarningCondition(manifest *v1beta2.Manifest) error {
+	kymaName, err := manifest.GetKymaName()
+	if err != nil {
+		return fmt.Errorf("failed to get kyma name: %w", err)
+	}
+	moduleName, err := manifest.GetModuleName()
+	if err != nil {
+		return fmt.Errorf("failed to get module name: %w", err)
+	}
+	r.ModuleMetrics.SetModuleCRWarningCondition(kymaName, moduleName)
+	return nil
+}
+
+func (r *Reconciler) RemoveModuleCRWarningCondition(manifest *v1beta2.Manifest) error {
+	kymaName, err := manifest.GetKymaName()
+	if err != nil {
+		return fmt.Errorf("failed to get kyma name: %w", err)
+	}
+	moduleName, err := manifest.GetModuleName()
+	if err != nil {
+		return fmt.Errorf("failed to get module name: %w", err)
+	}
+	r.ModuleMetrics.RemoveModuleCRWarningCondition(kymaName, moduleName)
+	return nil
+}
+
 func hasDiff(oldResources []shared.Resource, newResources []shared.Resource) bool {
 	if len(oldResources) != len(newResources) {
 		return true
@@ -557,11 +627,3 @@ func (r *Reconciler) recordReconciliationDuration(startTime time.Time, name stri
 		r.ManifestMetrics.RemoveManifestDuration(name)
 	}
 }
-
-func (r *Reconciler) cleanUpMandatoryModuleMetrics(manifest *v1beta2.Manifest) {
-	if manifest.IsMandatoryModule() {
-		kymaName := manifest.GetLabels()[shared.KymaName]
-		moduleName := manifest.GetLabels()[shared.ModuleName]
-		r.MandatoryModuleMetrics.CleanupMetrics(kymaName, moduleName)
-	}
-}