Skip to content

Commit

Permalink
fix: Update lastOperation.operation when manifest is back to ready (#…
Browse files Browse the repository at this point in the history
…1474)

* Update lastOperation

* Adapt integration test
  • Loading branch information
nesmabadr authored Apr 18, 2024
1 parent fb6f058 commit 3947c84
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 5 deletions.
23 changes: 19 additions & 4 deletions internal/declarative/v2/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
r.ManifestMetrics.RemoveManifestDuration(req.Name)
return r.removeFinalizers(ctx, obj, []string{r.Finalizer}, metrics.ManifestRemoveFinalizerInDeleting)
}

if err = r.handleStatusPatch(ctx, obj, currentObjStatus); err != nil {
r.Event(obj, "Warning", "PatchStatus", err.Error())
return ctrl.Result{}, fmt.Errorf("failed to patch status: %w", err)
}

return ctrl.Result{RequeueAfter: r.Success}, nil
}

Expand Down Expand Up @@ -634,19 +640,28 @@ func (r *Reconciler) configClient(ctx context.Context, obj Object) (Client, erro
}

func (r *Reconciler) ssaStatusIfDiffExist(ctx context.Context, obj Object,
requeueReason metrics.ManifestRequeueReason, previous shared.Status,
requeueReason metrics.ManifestRequeueReason, previousStatus shared.Status,
) (ctrl.Result, error) {
r.ManifestMetrics.RecordRequeueReason(requeueReason, queue.UnexpectedRequeue)

if hasStatusDiff(obj.GetStatus(), previous) {
if err := r.handleStatusPatch(ctx, obj, previousStatus); err != nil {
r.Event(obj, "Warning", "PatchStatus", err.Error())
return ctrl.Result{}, fmt.Errorf("failed to patch status: %w", err)
}

return ctrl.Result{RequeueAfter: r.RequeueIntervals.Busy}, nil
}

func (r *Reconciler) handleStatusPatch(ctx context.Context, obj Object, previousStatus shared.Status) error {
if hasStatusDiff(obj.GetStatus(), previousStatus) {
resetNonPatchableField(obj)
if err := r.Status().Patch(ctx, obj, client.Apply, client.ForceOwnership, r.FieldOwner); err != nil {
r.Event(obj, "Warning", "PatchStatus", err.Error())
return ctrl.Result{}, fmt.Errorf("failed to patch status: %w", err)
return fmt.Errorf("failed to patch status: %w", err)
}
}

return ctrl.Result{RequeueAfter: r.RequeueIntervals.Busy}, nil
return nil
}

func hasStatusDiff(first, second shared.Status) bool {
Expand Down
28 changes: 28 additions & 0 deletions pkg/testutils/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,19 @@ func WithValidInstallImageSpec(ctx context.Context, clnt client.Client, name, ma
}
}

func UpdateManifestSpec(cxt context.Context, clnt client.Client, manifestName string, spec v1beta2.ManifestSpec) error {
manifest, err := GetManifestWithName(cxt, clnt, manifestName)
if err != nil {
return err
}
manifest.Spec = spec
if err = clnt.Update(cxt, manifest); err != nil {
return fmt.Errorf("error updating Manifest: %w", err)
}

return nil
}

func InstallManifest(ctx context.Context, clnt client.Client, manifest *v1beta2.Manifest, installSpecByte []byte,
enableResource bool,
) error {
Expand Down Expand Up @@ -522,6 +535,21 @@ func ExpectManifestStateIn(ctx context.Context, clnt client.Client,
}
}

func ExpectManifestLastOperationMessageContains(ctx context.Context, clnt client.Client,
manifestName, message string,
) error {
manifest, err := GetManifestWithName(ctx, clnt, manifestName)
if err != nil {
return err
}

if !strings.Contains(manifest.Status.Operation, message) {
return errManifestOperationNotContainMessage
}

return nil
}

func ExpectOCISyncRefAnnotationExists(ctx context.Context, clnt client.Client,
mustExist bool,
) func(manifestName string) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/kyma-project/lifecycle-manager/api/shared"
"github.com/kyma-project/lifecycle-manager/api/v1beta2"
"github.com/kyma-project/lifecycle-manager/pkg/ocmextensions"
"github.com/kyma-project/lifecycle-manager/pkg/testutils"

Expand All @@ -29,7 +30,7 @@ var _ = Describe(
"Rendering manifest install layer", Ordered, func() {
ociTempDir := "main-dir"
installName := filepath.Join(ociTempDir, "installs")

var validManifest *v1beta2.Manifest
setupTestEnvironment(ociTempDir, installName)

Context("Given a Manifest CR", func() {
Expand Down Expand Up @@ -69,6 +70,12 @@ var _ = Describe(
serverAddress, true, false), standardTimeout, standardInterval).
WithArguments(manifest).
Should(Succeed())
Eventually(func() error {
var err error
validManifest, err = testutils.GetManifestWithName(ctx, controlPlaneClient, manifest.GetName())
return err
}).Should(Succeed())

By("Then Manifest CR is in Ready State", func() {
Eventually(testutils.ExpectManifestStateIn(ctx, controlPlaneClient, shared.StateReady),
standardTimeout,
Expand Down Expand Up @@ -107,6 +114,39 @@ var _ = Describe(
standardInterval).
WithArguments(manifest.GetName()).Should(Succeed())
})
By("And Manifest LastOperation is updated with error message", func() {
Eventually(testutils.ExpectManifestLastOperationMessageContains,
standardTimeout,
standardInterval).
WithContext(ctx).
WithArguments(controlPlaneClient, manifest.GetName(),
"failed to extract raw manifest from layer digest").
Should(Succeed())
})

By("When OCI Image is corrected", func() {
Eventually(testutils.UpdateManifestSpec, standardTimeout, standardInterval).
WithContext(ctx).
WithArguments(controlPlaneClient, manifest.GetName(), validManifest.Spec).
Should(Succeed())
})

By("The Manifest CR is in Ready State and lastOperation is updated correctly", func() {
Eventually(testutils.ExpectManifestStateIn(ctx, controlPlaneClient, shared.StateError),
standardTimeout,
standardInterval).
WithArguments(manifest.GetName()).Should(Succeed())
})
By("And Manifest LastOperation is updated correctly", func() {
Eventually(testutils.ExpectManifestLastOperationMessageContains,
standardTimeout,
standardInterval).
WithContext(ctx).
WithArguments(controlPlaneClient, manifest.GetName(),
"installation is ready and resources can be used").
Should(Succeed())
})

Eventually(testutils.DeleteManifestAndVerify(ctx, controlPlaneClient, manifest), standardTimeout,
standardInterval).Should(Succeed())
},
Expand Down

0 comments on commit 3947c84

Please sign in to comment.