From ce1629660c9494270cc7d057c44877cc2f07c63d Mon Sep 17 00:00:00 2001 From: Ankita Thomas Date: Thu, 26 Sep 2024 15:36:37 -0400 Subject: [PATCH] rename unpacker result's lastTransitionTime to unpackTime, set observedGeneration in conditions --- .../core/clustercatalog_controller.go | 35 ++++++++++--------- internal/source/containers_image.go | 12 +++---- internal/source/containers_image_test.go | 10 +++--- internal/source/unpacker.go | 7 ++-- 4 files changed, 33 insertions(+), 31 deletions(-) diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go index 887314e3..8ab03ef6 100644 --- a/internal/controllers/core/clustercatalog_controller.go +++ b/internal/controllers/core/clustercatalog_controller.go @@ -160,7 +160,7 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp contentURL = r.Storage.ContentURL(catalog.Name) updateStatusProgressing(catalog, nil) - updateStatusServing(&catalog.Status, unpackResult, contentURL) + updateStatusServing(&catalog.Status, unpackResult, contentURL, catalog.GetGeneration()) var requeueAfter time.Duration switch catalog.Spec.Source.Type { @@ -178,10 +178,11 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) { progressingCond := metav1.Condition{ - Type: v1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonSucceeded, - Message: "Successfully unpacked and stored content from resolved source", + Type: v1alpha1.TypeProgressing, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonSucceeded, + Message: "Successfully unpacked and stored content from resolved source", + ObservedGeneration: catalog.GetGeneration(), } if err != nil { @@ -198,26 +199,28 @@ func updateStatusProgressing(catalog *v1alpha1.ClusterCatalog, err error) { meta.SetStatusCondition(&catalog.Status.Conditions, progressingCond) } -func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string) { +func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result *source.Result, contentURL string, generation int64) { status.ResolvedSource = result.ResolvedSource status.ContentURL = contentURL - status.LastUnpacked = result.LastTransitionTime + status.LastUnpacked = metav1.NewTime(result.UnpackTime) meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeServing, - Status: metav1.ConditionTrue, - Reason: v1alpha1.ReasonAvailable, - Message: "Serving desired content from resolved source", + Type: v1alpha1.TypeServing, + Status: metav1.ConditionTrue, + Reason: v1alpha1.ReasonAvailable, + Message: "Serving desired content from resolved source", + ObservedGeneration: generation, }) } -func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus) { +func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus, generation int64) { status.ResolvedSource = nil status.ContentURL = "" status.LastUnpacked = metav1.Time{} meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: v1alpha1.TypeServing, - Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonUnavailable, + Type: v1alpha1.TypeServing, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonUnavailable, + ObservedGeneration: generation, }) } @@ -277,7 +280,7 @@ func NewFinalizers(localStorage storage.Instance, unpacker source.Unpacker) (crf updateStatusProgressing(catalog, err) return crfinalizer.Result{StatusUpdated: true}, err } - updateStatusNotServing(&catalog.Status) + updateStatusNotServing(&catalog.Status, catalog.GetGeneration()) if err := unpacker.Cleanup(ctx, catalog); err != nil { updateStatusProgressing(catalog, err) return crfinalizer.Result{StatusUpdated: true}, err diff --git a/internal/source/containers_image.go b/internal/source/containers_image.go index 925bd936..a95c0a2b 100644 --- a/internal/source/containers_image.go +++ b/internal/source/containers_image.go @@ -71,7 +71,7 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv panic(fmt.Sprintf("unexpected file at unpack path %q: expected a directory", unpackPath)) } l.Info("image already unpacked", "ref", imgRef.String(), "digest", canonicalRef.Digest().String()) - return successResult(unpackPath, canonicalRef, metav1.NewTime(unpackStat.ModTime())), nil + return successResult(unpackPath, canonicalRef, unpackStat.ModTime()), nil } ////////////////////////////////////////////////////// @@ -153,10 +153,10 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv return nil, fmt.Errorf("error deleting old images: %w", err) } - return successResult(unpackPath, canonicalRef, metav1.Now()), nil + return successResult(unpackPath, canonicalRef, time.Now()), nil } -func successResult(unpackPath string, canonicalRef reference.Canonical, lastUnpacked metav1.Time) *Result { +func successResult(unpackPath string, canonicalRef reference.Canonical, lastUnpacked time.Time) *Result { return &Result{ FS: os.DirFS(unpackPath), ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ @@ -166,9 +166,9 @@ func successResult(unpackPath string, canonicalRef reference.Canonical, lastUnpa LastSuccessfulPollAttempt: metav1.NewTime(time.Now()), }, }, - State: StateUnpacked, - Message: fmt.Sprintf("unpacked %q successfully", canonicalRef), - LastTransitionTime: lastUnpacked, + State: StateUnpacked, + Message: fmt.Sprintf("unpacked %q successfully", canonicalRef), + UnpackTime: lastUnpacked, } } diff --git a/internal/source/containers_image_test.go b/internal/source/containers_image_test.go index 290fe289..d35c2c90 100644 --- a/internal/source/containers_image_test.go +++ b/internal/source/containers_image_test.go @@ -381,7 +381,7 @@ func TestImageRegistry(t *testing.T) { rs, err := imgReg.Unpack(ctx, tt.catalog) if !tt.wantErr { - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, fmt.Sprintf("%s@sha256:%s", imgName.Context().Name(), digest.Hex), rs.ResolvedSource.Image.Ref) assert.Equal(t, source.StateUnpacked, rs.State) @@ -396,15 +396,15 @@ func TestImageRegistry(t *testing.T) { // If the digest should already exist check that we actually hit it if tt.digestAlreadyExists { assert.Contains(t, buf.String(), "image already unpacked") - assert.Equal(t, rs.LastTransitionTime.Time, unpackDirStat.ModTime()) + assert.Equal(t, rs.UnpackTime, unpackDirStat.ModTime()) } else if tt.oldDigestExists { assert.NotContains(t, buf.String(), "image already unpacked") - assert.NotEqual(t, rs.LastTransitionTime, oldDigestModTime) + assert.NotEqual(t, rs.UnpackTime, oldDigestModTime) assert.NoDirExists(t, oldDigestDir) } else { - require.NotNil(t, rs.LastTransitionTime) + require.NotNil(t, rs.UnpackTime) require.NotNil(t, rs.ResolvedSource.Image) - assert.False(t, rs.LastTransitionTime.IsZero()) + assert.False(t, rs.UnpackTime.IsZero()) } } else { assert.Error(t, err) diff --git a/internal/source/unpacker.go b/internal/source/unpacker.go index c595e6b8..db6ffc57 100644 --- a/internal/source/unpacker.go +++ b/internal/source/unpacker.go @@ -3,8 +3,7 @@ package source import ( "context" "io/fs" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "time" catalogdv1alpha1 "github.com/operator-framework/catalogd/api/core/v1alpha1" ) @@ -57,8 +56,8 @@ type Result struct { // catalog content. Message string - // LastTraansitionTime is the timestamp when the transition to the current State happened - LastTransitionTime metav1.Time + // UnpackTime is the timestamp when the transition to the current State happened + UnpackTime time.Time } type State string