Skip to content

Commit

Permalink
rename unpacker result's lastTransitionTime to unpackTime, set observ…
Browse files Browse the repository at this point in the history
…edGeneration in conditions
  • Loading branch information
ankitathomas committed Oct 1, 2024
1 parent 2da5929 commit ce16296
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 31 deletions.
35 changes: 19 additions & 16 deletions internal/controllers/core/clustercatalog_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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,
})
}

Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions internal/source/containers_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

//////////////////////////////////////////////////////
Expand Down Expand Up @@ -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{
Expand All @@ -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,
}
}

Expand Down
10 changes: 5 additions & 5 deletions internal/source/containers_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions internal/source/unpacker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit ce16296

Please sign in to comment.