gitrepo: Use commit msg in NewArtifact message

Use commit message in the NewArtifact event message to make it more user
friendly.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
This commit is contained in:
Sunny 2022-02-15 17:20:13 +05:30 committed by Hidde Beydals
parent 45df2d76c8
commit acda998150
2 changed files with 42 additions and 34 deletions

View File

@ -107,7 +107,7 @@ type GitRepositoryReconcilerOptions struct {
// gitRepoReconcilerFunc is the function type for all the Git repository
// reconciler functions.
type gitRepoReconcilerFunc func(ctx context.Context, obj *sourcev1.GitRepository, artifact *sourcev1.Artifact, includes *artifactSet, dir string) (sreconcile.Result, error)
type gitRepoReconcilerFunc func(ctx context.Context, obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error)
func (r *GitRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error {
return r.SetupWithManagerAndOptions(mgr, GitRepositoryReconcilerOptions{})
@ -207,7 +207,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.G
conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation)
}
var artifact sourcev1.Artifact
var commit git.Commit
var includes artifactSet
// Create temp dir for Git clone
@ -224,7 +224,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.G
var res sreconcile.Result
var resErr error
for _, rec := range reconcilers {
recResult, err := rec(ctx, obj, &artifact, &includes, tmpDir)
recResult, err := rec(ctx, obj, &commit, &includes, tmpDir)
// Exit immediately on ResultRequeue.
if recResult == sreconcile.ResultRequeue {
return sreconcile.ResultRequeue, nil
@ -248,7 +248,8 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.G
// If the artifact in the Status object of the resource disappeared from storage, it is removed from the object.
// If the object does not have an artifact in its Status object, a v1beta1.ArtifactUnavailableCondition is set.
// If the hostname of any of the URLs on the object do not match the current storage server hostname, they are updated.
func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, obj *sourcev1.GitRepository, artifact *sourcev1.Artifact, includes *artifactSet, dir string) (sreconcile.Result, error) {
func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context,
obj *sourcev1.GitRepository, _ *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
// Garbage collect previous advertised artifact(s) from storage
_ = r.garbageCollect(ctx, obj)
@ -284,7 +285,7 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, obj *sou
// If both the checkout and signature verification are successful, the given artifact pointer is set to a new artifact
// with the available metadata.
func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
obj *sourcev1.GitRepository, artifact *sourcev1.Artifact, _ *artifactSet, dir string) (sreconcile.Result, error) {
obj *sourcev1.GitRepository, commit *git.Commit, _ *artifactSet, dir string) (sreconcile.Result, error) {
// Configure authentication strategy to access the source
var authOpts *git.AuthOptions
var err error
@ -344,7 +345,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
// Checkout HEAD of reference in object
gitCtx, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
defer cancel()
commit, err := checkoutStrategy.Checkout(gitCtx, dir, obj.Spec.URL, authOpts)
c, err := checkoutStrategy.Checkout(gitCtx, dir, obj.Spec.URL, authOpts)
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to checkout and determine revision: %w", err),
@ -354,6 +355,8 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
// Coin flip on transient or persistent error, return error and hope for the best
return sreconcile.ResultEmpty, e
}
// Assign the commit to the shared commit reference.
*commit = *c
ctrl.LoggerFrom(ctx).V(logger.DebugLevel).Info("git repository checked out", "url", obj.Spec.URL, "revision", commit.String())
conditions.Delete(obj, sourcev1.FetchFailedCondition)
@ -362,9 +365,6 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
return result, err
}
// Create potential new artifact with current available metadata
*artifact = r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), commit.String(), fmt.Sprintf("%s.tar.gz", commit.Hash.String()))
// Mark observations about the revision on the object
if !obj.GetArtifact().HasRevision(commit.String()) {
message := fmt.Sprintf("new upstream revision '%s'", commit.String())
@ -383,7 +383,11 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
// Source ignore patterns are loaded, and the given directory is archived.
// On a successful archive, the artifact and includes in the status of the given object are set, and the symlink in the
// storage is updated to its path.
func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.GitRepository, artifact *sourcev1.Artifact, includes *artifactSet, dir string) (sreconcile.Result, error) {
func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
// Create potential new artifact with current available metadata
artifact := r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), commit.String(), fmt.Sprintf("%s.tar.gz", commit.Hash.String()))
// Always restore the Ready condition in case it got removed due to a transient error
defer func() {
if obj.GetArtifact().HasRevision(artifact.Revision) && !includes.Diff(obj.Status.IncludedArtifacts) {
@ -419,14 +423,14 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
}
// Ensure artifact directory exists and acquire lock
if err := r.Storage.MkdirAll(*artifact); err != nil {
if err := r.Storage.MkdirAll(artifact); err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to create artifact directory: %w", err),
Reason: sourcev1.StorageOperationFailedReason,
}
return sreconcile.ResultEmpty, e
}
unlock, err := r.Storage.Lock(*artifact)
unlock, err := r.Storage.Lock(artifact)
if err != nil {
return sreconcile.ResultEmpty, &serror.Event{
Err: fmt.Errorf("failed to acquire lock for artifact: %w", err),
@ -448,7 +452,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
}
// Archive directory to storage
if err := r.Storage.Archive(artifact, dir, SourceIgnoreFilter(ps, nil)); err != nil {
if err := r.Storage.Archive(&artifact, dir, SourceIgnoreFilter(ps, nil)); err != nil {
return sreconcile.ResultEmpty, &serror.Event{
Err: fmt.Errorf("unable to archive artifact to storage: %w", err),
Reason: sourcev1.StorageOperationFailedReason,
@ -457,14 +461,14 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
r.AnnotatedEventf(obj, map[string]string{
"revision": artifact.Revision,
"checksum": artifact.Checksum,
}, corev1.EventTypeNormal, "NewArtifact", "stored artifact for revision '%s'", artifact.Revision)
}, corev1.EventTypeNormal, "NewArtifact", "stored artifact for commit '%s'", commit.ShortMessage())
// Record it on the object
obj.Status.Artifact = artifact.DeepCopy()
obj.Status.IncludedArtifacts = *includes
// Update symlink on a "best effort" basis
url, err := r.Storage.Symlink(*artifact, "latest.tar.gz")
url, err := r.Storage.Symlink(artifact, "latest.tar.gz")
if err != nil {
r.eventLogf(ctx, obj, corev1.EventTypeWarning, sourcev1.StorageOperationFailedReason,
"failed to update status URL symlink: %s", err)
@ -481,7 +485,8 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
// If an include is unavailable, it marks the object with v1beta1.IncludeUnavailableCondition and returns early.
// If the copy operations are successful, it deletes the v1beta1.IncludeUnavailableCondition from the object.
// If the artifactSet differs from the current set, it marks the object with v1beta1.ArtifactOutdatedCondition.
func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, obj *sourcev1.GitRepository, _ *sourcev1.Artifact, includes *artifactSet, dir string) (sreconcile.Result, error) {
func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
obj *sourcev1.GitRepository, _ *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
artifacts := make(artifactSet, len(obj.Spec.Include))
for i, incl := range obj.Spec.Include {
// Do this first as it is much cheaper than copy operations

View File

@ -511,14 +511,14 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<url>", obj.Spec.URL)
}
var artifact sourcev1.Artifact
var commit git.Commit
var includes artifactSet
got, err := r.reconcileSource(context.TODO(), obj, &artifact, &includes, tmpDir)
got, err := r.reconcileSource(context.TODO(), obj, &commit, &includes, tmpDir)
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
g.Expect(artifact).ToNot(BeNil())
g.Expect(commit).ToNot(BeNil())
})
}
})
@ -666,9 +666,9 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
obj := obj.DeepCopy()
obj.Spec.GitImplementation = i
var artifact sourcev1.Artifact
var commit git.Commit
var includes artifactSet
got, err := r.reconcileSource(ctx, obj, &artifact, &includes, tmpDir)
got, err := r.reconcileSource(ctx, obj, &commit, &includes, tmpDir)
if err != nil {
println(err.Error())
}
@ -676,7 +676,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
g.Expect(got).To(Equal(tt.want))
if tt.wantRevision != "" {
revision := strings.ReplaceAll(tt.wantRevision, "<commit>", headRef.Hash().String())
g.Expect(artifact.Revision).To(Equal(revision))
g.Expect(commit.String()).To(Equal(revision))
g.Expect(conditions.IsTrue(obj, sourcev1.ArtifactOutdatedCondition)).To(BeTrue())
}
})
@ -691,7 +691,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
dir string
includes artifactSet
beforeFunc func(obj *sourcev1.GitRepository)
afterFunc func(t *WithT, obj *sourcev1.GitRepository, artifact sourcev1.Artifact)
afterFunc func(t *WithT, obj *sourcev1.GitRepository)
want sreconcile.Result
wantErr bool
assertConditions []metav1.Condition
@ -702,7 +702,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
beforeFunc: func(obj *sourcev1.GitRepository) {
obj.Spec.Interval = metav1.Duration{Duration: interval}
},
afterFunc: func(t *WithT, obj *sourcev1.GitRepository, artifact sourcev1.Artifact) {
afterFunc: func(t *WithT, obj *sourcev1.GitRepository) {
t.Expect(obj.GetArtifact()).ToNot(BeNil())
t.Expect(obj.Status.URL).ToNot(BeEmpty())
},
@ -719,7 +719,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
beforeFunc: func(obj *sourcev1.GitRepository) {
obj.Spec.Interval = metav1.Duration{Duration: interval}
},
afterFunc: func(t *WithT, obj *sourcev1.GitRepository, artifact sourcev1.Artifact) {
afterFunc: func(t *WithT, obj *sourcev1.GitRepository) {
t.Expect(obj.GetArtifact()).ToNot(BeNil())
t.Expect(obj.GetArtifact().Checksum).To(Equal("60a3bf69f337cb5ec9ebd00abefbb6e7f2a2cf27158ecf438d52b2035b184172"))
t.Expect(obj.Status.IncludedArtifacts).ToNot(BeEmpty())
@ -740,7 +740,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
obj.Status.Artifact = &sourcev1.Artifact{Revision: "main/revision"}
obj.Status.IncludedArtifacts = []*sourcev1.Artifact{{Revision: "main/revision"}}
},
afterFunc: func(t *WithT, obj *sourcev1.GitRepository, artifact sourcev1.Artifact) {
afterFunc: func(t *WithT, obj *sourcev1.GitRepository) {
t.Expect(obj.Status.URL).To(BeEmpty())
},
want: sreconcile.ResultSuccess,
@ -755,7 +755,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
obj.Spec.Interval = metav1.Duration{Duration: interval}
obj.Spec.Ignore = pointer.StringPtr("!**.txt\n")
},
afterFunc: func(t *WithT, obj *sourcev1.GitRepository, artifact sourcev1.Artifact) {
afterFunc: func(t *WithT, obj *sourcev1.GitRepository) {
t.Expect(obj.GetArtifact()).ToNot(BeNil())
t.Expect(obj.GetArtifact().Checksum).To(Equal("11f7f007dce5619bd79e6c57688261058d09f5271e802463ac39f2b9ead7cabd"))
},
@ -772,7 +772,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
obj.Spec.Interval = metav1.Duration{Duration: interval}
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "")
},
afterFunc: func(t *WithT, obj *sourcev1.GitRepository, artifact sourcev1.Artifact) {
afterFunc: func(t *WithT, obj *sourcev1.GitRepository) {
t.Expect(obj.GetArtifact()).ToNot(BeNil())
t.Expect(obj.GetArtifact().Checksum).To(Equal("60a3bf69f337cb5ec9ebd00abefbb6e7f2a2cf27158ecf438d52b2035b184172"))
t.Expect(obj.Status.URL).ToNot(BeEmpty())
@ -789,7 +789,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
beforeFunc: func(obj *sourcev1.GitRepository) {
obj.Spec.Interval = metav1.Duration{Duration: interval}
},
afterFunc: func(t *WithT, obj *sourcev1.GitRepository, artifact sourcev1.Artifact) {
afterFunc: func(t *WithT, obj *sourcev1.GitRepository) {
t.Expect(obj.GetArtifact()).ToNot(BeNil())
localPath := testStorage.LocalPath(*obj.GetArtifact())
@ -843,15 +843,18 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
tt.beforeFunc(obj)
}
artifact := testStorage.NewArtifactFor(obj.Kind, obj, "main/revision", "checksum.tar.gz")
commit := git.Commit{
Hash: []byte("revision"),
Reference: "refs/heads/main",
}
got, err := r.reconcileArtifact(ctx, obj, &artifact, &tt.includes, tt.dir)
got, err := r.reconcileArtifact(ctx, obj, &commit, &tt.includes, tt.dir)
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
if tt.afterFunc != nil {
tt.afterFunc(g, obj, artifact)
tt.afterFunc(g, obj)
}
})
}
@ -1036,10 +1039,10 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
g.Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(tmpDir)
var artifact sourcev1.Artifact
var commit git.Commit
var includes artifactSet
got, err := r.reconcileInclude(ctx, obj, &artifact, &includes, tmpDir)
got, err := r.reconcileInclude(ctx, obj, &commit, &includes, tmpDir)
g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions))
g.Expect(err != nil).To(Equal(tt.wantErr))
if err == nil {