From 8ca680fb153795c657817ec0819fb860a5108a72 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 21 May 2024 21:11:23 +0200 Subject: [PATCH] chore(warehouse): enrich discovery logs (#2035) Signed-off-by: Hidde Beydals --- internal/controller/warehouses/git.go | 54 +++++++++++++++++++++ internal/controller/warehouses/helm.go | 14 ++++-- internal/controller/warehouses/helm_test.go | 4 +- internal/controller/warehouses/images.go | 34 +++++++++++-- internal/image/newest_build_selector.go | 12 +++-- 5 files changed, 102 insertions(+), 16 deletions(-) diff --git a/internal/controller/warehouses/git.go b/internal/controller/warehouses/git.go index 555bdc0b5..518a2192c 100644 --- a/internal/controller/warehouses/git.go +++ b/internal/controller/warehouses/git.go @@ -7,8 +7,10 @@ import ( "regexp" "slices" "strings" + "time" "github.com/Masterminds/semver/v3" + log "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kargoapi "github.com/akuity/kargo/api/v1alpha1" @@ -25,6 +27,9 @@ const ( type pathSelector func(path string) (bool, error) +// discoverCommits discovers commits from the given Git repositories based on the +// given subscriptions. It returns a list of GitDiscoveryResult objects, each +// containing the discovered commits for the corresponding subscription. func (r *reconciler) discoverCommits( ctx context.Context, namespace string, @@ -41,6 +46,7 @@ func (r *reconciler) discoverCommits( logger := logging.LoggerFromContext(ctx).WithField("repo", sub.RepoURL) + // Obtain credentials for the Git repository. creds, ok, err := r.credentialsDB.Get(ctx, namespace, credentials.TypeGit, sub.RepoURL) if err != nil { return nil, fmt.Errorf( @@ -61,6 +67,7 @@ func (r *reconciler) discoverCommits( logger.Debug("found no credentials for git repo") } + // Clone the Git repository. cloneOpts := &git.CloneOptions{ Branch: sub.Branch, SingleBranch: true, @@ -78,6 +85,10 @@ func (r *reconciler) discoverCommits( return nil, fmt.Errorf("failed to clone git repo %q: %w", sub.RepoURL, err) } + // Enrich the logger with additional fields for this subscription. + logger = logger.WithFields(gitDiscoveryLogFields(sub)) + + // Discover commits based on the subscription's commit selection strategy. var discovered []kargoapi.DiscoveredCommit switch sub.CommitSelectionStrategy { case kargoapi.CommitSelectionStrategyLexical, @@ -101,6 +112,11 @@ func (r *reconciler) discoverCommits( Committer: meta.Committer, CreatorDate: &metav1.Time{Time: meta.CreatorDate}, }) + logger.WithFields(log.Fields{ + "tag": meta.Tag, + "commit": meta.CommitID, + "creatorDate": meta.CreatorDate.Format(time.RFC3339), + }).Trace("discovered commit from tag") } default: commits, err := r.discoverBranchHistoryFn(repo, sub) @@ -121,18 +137,36 @@ func (r *reconciler) discoverCommits( Committer: meta.Committer, CreatorDate: &metav1.Time{Time: meta.CommitDate}, }) + logger.WithFields(log.Fields{ + "commit": meta.ID, + "creatorDate": meta.CommitDate.Format(time.RFC3339), + }).Trace("discovered commit from branch") } } + if len(discovered) == 0 { + results = append(results, kargoapi.GitDiscoveryResult{ + RepoURL: sub.RepoURL, + }) + logger.Debug("discovered no commits") + continue + } + results = append(results, kargoapi.GitDiscoveryResult{ RepoURL: sub.RepoURL, Commits: discovered, }) + logger.Debugf("discovered %d commits", len(discovered)) } return results, nil } +// discoverBranchHistory returns a list of commits from the given Git repository +// that match the given subscription's branch selection criteria. It returns the +// list of commits that match the criteria, sorted in descending order. If the +// list contains more than 20 commits, it is clipped to the 20 most recent +// commits. func (r *reconciler) discoverBranchHistory(repo git.Repo, sub kargoapi.GitSubscription) ([]git.CommitMetadata, error) { limit := int(sub.DiscoveryLimit) var filteredCommits = make([]git.CommitMetadata, 0, limit) @@ -450,6 +484,26 @@ func (r *reconciler) getDiffPathsForCommitID(repo git.Repo, commitID string) ([] return repo.GetDiffPathsForCommitID(commitID) } +// gitDiscoveryLogFields returns a set of log fields for a Git subscription +// based on the subscription's configuration. +func gitDiscoveryLogFields(sub kargoapi.GitSubscription) log.Fields { + f := log.Fields{ + "selectionStrategy": sub.CommitSelectionStrategy, + "pathConstrained": sub.IncludePaths != nil || sub.ExcludePaths != nil, + } + if sub.Branch != "" { + f["branch"] = sub.Branch + } + switch sub.CommitSelectionStrategy { + case kargoapi.CommitSelectionStrategySemVer: + f["semverConstraint"] = sub.SemverConstraint + f["tagConstrained"] = sub.AllowTags != "" || len(sub.IgnoreTags) > 0 + case kargoapi.CommitSelectionStrategyLexical, kargoapi.CommitSelectionStrategyNewestTag: + f["tagConstrained"] = sub.AllowTags != "" || len(sub.IgnoreTags) > 0 + } + return f +} + // shortenString truncates the given string to the given length, appending an // ellipsis if the string is longer than the length. func shortenString(str string, length int) string { diff --git a/internal/controller/warehouses/helm.go b/internal/controller/warehouses/helm.go index d1d187f76..2a4565bed 100644 --- a/internal/controller/warehouses/helm.go +++ b/internal/controller/warehouses/helm.go @@ -48,17 +48,23 @@ func (r *reconciler) discoverCharts( logger.Debug("found no credentials for chart repo") } + // Enrich the logger with additional fields for this subscription. + if sub.SemverConstraint != "" { + logger = logger.WithField("semverConstraint", sub.SemverConstraint) + } + + // Discover versions of the chart based on the semver constraint. versions, err := r.discoverChartVersionsFn(ctx, sub.RepoURL, sub.Name, sub.SemverConstraint, helmCreds) if err != nil { if sub.Name == "" { return nil, fmt.Errorf( - "error discovering latest suitable chart versions in repository %q: %w", + "error discovering latest chart versions in repository %q: %w", sub.RepoURL, err, ) } return nil, fmt.Errorf( - "error discovering latest suitable chart versions for chart %q in repository %q: %w", + "error discovering latest chart versions for chart %q in repository %q: %w", sub.Name, sub.RepoURL, err, @@ -66,22 +72,22 @@ func (r *reconciler) discoverCharts( } if len(versions) == 0 { - logger.Debug("discovered no suitable chart versions") results = append(results, kargoapi.ChartDiscoveryResult{ RepoURL: sub.RepoURL, Name: sub.Name, SemverConstraint: sub.SemverConstraint, }) + logger.Debug("discovered no chart versions") continue } - logger.Debugf("discovered %d suitable chart versions", len(versions)) results = append(results, kargoapi.ChartDiscoveryResult{ RepoURL: sub.RepoURL, Name: sub.Name, SemverConstraint: sub.SemverConstraint, Versions: trimSlice(versions, int(sub.DiscoveryLimit)), }) + logger.Debugf("discovered %d chart versions", len(versions)) } return results, nil diff --git a/internal/controller/warehouses/helm_test.go b/internal/controller/warehouses/helm_test.go index f6bb2e7c4..0aeb5ae1b 100644 --- a/internal/controller/warehouses/helm_test.go +++ b/internal/controller/warehouses/helm_test.go @@ -131,7 +131,7 @@ func TestDiscoverCharts(t *testing.T) { {Chart: &kargoapi.ChartSubscription{}}, }, assertions: func(t *testing.T, results []kargoapi.ChartDiscoveryResult, err error) { - require.ErrorContains(t, err, "error discovering latest suitable chart versions") + require.ErrorContains(t, err, "error discovering latest chart versions") require.ErrorContains(t, err, "something went wrong") require.Empty(t, results) }, @@ -156,7 +156,7 @@ func TestDiscoverCharts(t *testing.T) { }}, }, assertions: func(t *testing.T, results []kargoapi.ChartDiscoveryResult, err error) { - require.ErrorContains(t, err, "error discovering latest suitable chart versions for chart") + require.ErrorContains(t, err, "error discovering latest chart versions for chart") require.ErrorContains(t, err, "something went wrong") require.Empty(t, results) }, diff --git a/internal/controller/warehouses/images.go b/internal/controller/warehouses/images.go index 7268deef7..0ff92f396 100644 --- a/internal/controller/warehouses/images.go +++ b/internal/controller/warehouses/images.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + log "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kargoapi "github.com/akuity/kargo/api/v1alpha1" @@ -14,6 +15,9 @@ import ( "github.com/akuity/kargo/internal/logging" ) +// discoverImages discovers the latest suitable images for the given image +// subscriptions. It returns a list of image discovery results, one for each +// subscription. func (r *reconciler) discoverImages( ctx context.Context, namespace string, @@ -25,10 +29,11 @@ func (r *reconciler) discoverImages( if s.Image == nil { continue } - sub := s.Image + sub := *s.Image logger := logging.LoggerFromContext(ctx).WithField("repo", sub.RepoURL) + // Obtain credentials for the image repository. creds, ok, err := r.credentialsDB.Get(ctx, namespace, credentials.TypeImage, sub.RepoURL) if err != nil { return nil, fmt.Errorf( @@ -48,24 +53,27 @@ func (r *reconciler) discoverImages( logger.Debug("found no credentials for image repo") } - images, err := r.discoverImageRefsFn(ctx, *sub, regCreds) + // Enrich the logger with additional fields for this subscription. + logger = logger.WithFields(imageDiscoveryLogFields(sub)) + + // Discover the latest suitable images. + images, err := r.discoverImageRefsFn(ctx, sub, regCreds) if err != nil { return nil, fmt.Errorf( - "error discovering latest suitable images %q: %w", + "error discovering latest images %q: %w", sub.RepoURL, err, ) } if len(images) == 0 { - logger.Debug("discovered no suitable images") results = append(results, kargoapi.ImageDiscoveryResult{ RepoURL: sub.RepoURL, Platform: sub.Platform, }) + logger.Debug("discovered no images") continue } - logger.Debugf("discovered %d suitable images", len(images)) discoveredImages := make([]kargoapi.DiscoveredImageReference, 0, len(images)) for _, img := range images { discovery := kargoapi.DiscoveredImageReference{ @@ -78,11 +86,13 @@ func (r *reconciler) discoverImages( } discoveredImages = append(discoveredImages, discovery) } + results = append(results, kargoapi.ImageDiscoveryResult{ RepoURL: sub.RepoURL, Platform: sub.Platform, References: discoveredImages, }) + logger.Debugf("discovered %d images", len(images)) } return results, nil @@ -126,6 +136,20 @@ func (r *reconciler) getImageSourceURL(gitRepoURL, tag string) string { return "" } +func imageDiscoveryLogFields(sub kargoapi.ImageSubscription) log.Fields { + f := log.Fields{ + "imageSelectionStrategy": sub.ImageSelectionStrategy, + "platformConstrained": sub.Platform != "", + } + switch sub.ImageSelectionStrategy { + case kargoapi.ImageSelectionStrategySemVer, kargoapi.ImageSelectionStrategyDigest: + f["semverConstraint"] = sub.SemverConstraint + case kargoapi.ImageSelectionStrategyLexical, kargoapi.ImageSelectionStrategyNewestBuild: + f["tagConstrained"] = sub.AllowTags != "" || len(sub.IgnoreTags) > 0 + } + return f +} + func imageSelectorForSubscription( sub kargoapi.ImageSubscription, creds *image.Credentials, diff --git a/internal/image/newest_build_selector.go b/internal/image/newest_build_selector.go index 24a0a6abe..e61ad8e9f 100644 --- a/internal/image/newest_build_selector.go +++ b/internal/image/newest_build_selector.go @@ -6,6 +6,7 @@ import ( "regexp" "sort" "sync" + "time" log "github.com/sirupsen/logrus" @@ -97,13 +98,14 @@ func (n *newestBuildSelector) Select(ctx context.Context) ([]Image, error) { continue } - logger.WithFields(log.Fields{ - "tag": image.Tag, - "digest": image.Digest, - }).Trace("discovered image") - discoveredImage.Tag = image.Tag discoveredImages = append(discoveredImages, *discoveredImage) + + logger.WithFields(log.Fields{ + "tag": discoveredImage.Tag, + "digest": discoveredImage.Digest, + "createdAt": discoveredImage.CreatedAt.Format(time.RFC3339), + }).Trace("discovered image") } if len(discoveredImages) == 0 {