Skip to content

Commit

Permalink
chore(warehouse): enrich discovery logs (#2035)
Browse files Browse the repository at this point in the history 
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
  • Loading branch information
@hiddeco
hiddeco authored May 21, 2024
1 parent 473eacd commit 8ca680f
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 16 deletions.
54 changes: 54 additions & 0 deletions internal/controller/warehouses/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand All @@ -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(
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 10 additions & 4 deletions internal/controller/warehouses/helm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,40 +48,46 @@ 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,
)
}

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
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/warehouses/helm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
Expand All @@ -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)
},
Expand Down
34 changes: 29 additions & 5 deletions internal/controller/warehouses/images.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand All @@ -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(
Expand All @@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 7 additions & 5 deletions internal/image/newest_build_selector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"regexp"
"sort"
"sync"
"time"

log "github.com/sirupsen/logrus"

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 8ca680f

Please sign in to comment.