From adf1ebf2b80fa0b818aa42521b4eb43c695689d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20R=C3=BCegg?= Date: Tue, 31 Mar 2020 11:50:42 +0200 Subject: [PATCH] Improve log messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Various improvements around log messages to make them more clear. Signed-off-by: Simon Rüegg --- cmd/common.go | 29 ++++++++++++---- cmd/history.go | 17 +++++----- cmd/orphans.go | 19 ++++++----- cmd/root.go | 4 +-- pkg/cleanup/imagetags.go | 62 ++++++++++++++++++++--------------- pkg/cleanup/imagetags_test.go | 9 ++--- pkg/git/git.go | 12 +++---- pkg/openshift/imagestream.go | 5 +++ 8 files changed, 94 insertions(+), 63 deletions(-) diff --git a/cmd/common.go b/cmd/common.go index 7c12a56..c303c99 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -11,13 +11,25 @@ import ( "github.com/spf13/cobra" ) -func DeleteImages(imageTags []string, imageName string, namespace string) { +// DeleteImages deletes a list of image tags +func DeleteImages(imageTags []string, imageName string, namespace string, force bool) { + if !force { + log.Warn("Force mode not enabled, nothing will be deleted") + } for _, inactiveTag := range imageTags { - err := openshift.DeleteImageStreamTag(namespace, openshift.BuildImageStreamTagName(imageName, inactiveTag)) - if err == nil { - log.WithField("imageTag", inactiveTag).Info("Deleted image tag") + logEvent := log.WithFields(log.Fields{ + "namespace": namespace, + "image": imageName, + "imageTag": inactiveTag, + }) + if force { + if err := openshift.DeleteImageStreamTag(namespace, openshift.BuildImageStreamTagName(imageName, inactiveTag)); err != nil { + logEvent.Info("Deleted image tag") + } else { + logEvent.Error("Could not delete image tag") + } } else { - log.WithError(err).WithField("imageTag", inactiveTag).Error("Could not delete image") + logEvent.Info("Would delete image tag") } } } @@ -31,7 +43,7 @@ func PrintImageTags(imageTags []string) { } } else { for _, tag := range imageTags { - log.WithField("imageTag", tag).Info("Found image tag candidate.") + log.WithField("imageTag", tag).Info("Found image tag candidate") } } } @@ -62,6 +74,9 @@ func listImages() error { for _, image := range imageStreams { imageNames = append(imageNames, image.Name) } - log.WithField("project", ns).WithField("images", imageNames).Info("Please select an image. The following images are available") + log.WithFields(log.Fields{ + "project": ns, + "images": imageNames, + }).Info("Please select an image. The following images are available") return nil } diff --git a/cmd/history.go b/cmd/history.go index 13c92ea..23ec8b8 100644 --- a/cmd/history.go +++ b/cmd/history.go @@ -70,7 +70,7 @@ func ExecuteHistoryCleanupCommand(args []string) error { imageStreamTags = append(imageStreamTags, imageTag.Tag) } - matchOption := cleanup.MatchOptionDefault + matchOption := cleanup.MatchOptionPrefix if config.Git.Tag { matchOption = cleanup.MatchOptionExact } @@ -88,13 +88,14 @@ func ExecuteHistoryCleanupCommand(args []string) error { inactiveTags := cleanup.GetInactiveImageTags(&activeImageStreamTags, &matchingTags) inactiveTags = cleanup.LimitTags(&inactiveTags, c.Keep) - - PrintImageTags(inactiveTags) - - if config.Force { - DeleteImages(inactiveTags, image, namespace) - } else { - log.Info("--force was not specified. Nothing has been deleted.") + if len(inactiveTags) == 0 { + log.WithFields(log.Fields{ + "namespace": namespace, + "imageName": image, + }).Info("No inactive image stream tags found") + return nil } + PrintImageTags(inactiveTags) + DeleteImages(inactiveTags, image, namespace, config.Force) return nil } diff --git a/cmd/orphans.go b/cmd/orphans.go index f11240c..7249110 100644 --- a/cmd/orphans.go +++ b/cmd/orphans.go @@ -30,7 +30,7 @@ var ( Use: "orphans [PROJECT/IMAGE]", Short: "Clean up unknown image tags", Long: orphanCommandLongDescription, - Aliases: []string{"orph"}, + Aliases: []string{"orph", "orphan"}, Args: cobra.MaximumNArgs(1), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { @@ -92,7 +92,7 @@ func ExecuteOrphanCleanupCommand(args []string) error { cutOffDateTime, _ := parseCutOffDateTime(o.OlderThan) orphanIncludeRegex, _ := parseOrphanDeletionRegex(o.OrphanDeletionRegex) - matchOption := cleanup.MatchOptionDefault + matchOption := cleanup.MatchOptionPrefix if config.Git.Tag { matchOption = cleanup.MatchOptionExact } @@ -108,14 +108,15 @@ func ExecuteOrphanCleanupCommand(args []string) error { if err != nil { return err } - - PrintImageTags(imageTagList) - - if config.Force { - DeleteImages(imageTagList, imageName, namespace) - } else { - log.Info("--force was not specified. Nothing has been deleted.") + if len(imageTagList) == 0 { + log.WithFields(log.Fields{ + "namespace": namespace, + "imageName": imageName, + }).Info("No orphaned image stream tags found") + return nil } + PrintImageTags(imageTagList) + DeleteImages(imageTagList, imageName, namespace, config.Force) return nil } diff --git a/cmd/root.go b/cmd/root.go index 362bcae..cce3010 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -62,13 +62,11 @@ func parseConfig(cmd *cobra.Command, args []string) { } level, err := log.ParseLevel(config.Log.LogLevel) if err != nil { - log.WithField("error", err).Warn("Using info level.") + log.WithError(err).Warn("Could not parse log level, fallback to info level") log.SetLevel(log.InfoLevel) } else { log.SetLevel(level) } - - log.WithField("config", config).Debug("Parsed configuration.") } func bindFlags(flagSet *pflag.FlagSet) { diff --git a/pkg/cleanup/imagetags.go b/pkg/cleanup/imagetags.go index e3e3663..01584f3 100644 --- a/pkg/cleanup/imagetags.go +++ b/pkg/cleanup/imagetags.go @@ -13,34 +13,41 @@ import ( ) // MatchOption type defines how the tags should be matched -type MatchOption int8 +type MatchOption string const ( - MatchOptionDefault MatchOption = iota - MatchOptionExact - MatchOptionPrefix + // MatchOptionExact for exact matches + MatchOptionExact MatchOption = "exact" + // MatchOptionPrefix for prefix matches + MatchOptionPrefix MatchOption = "prefix" ) -// GetMatchingTags returns all tags matching one of the provided prefixes -func GetMatchingTags(values, tags *[]string, matchOption MatchOption) []string { +// GetMatchingTags returns all image tags matching one of the provided git tags +func GetMatchingTags(gitTags, imageTags *[]string, matchOption MatchOption) []string { var matchingTags []string - log.WithField("values", values).Debug("values") - log.WithField("tags", tags).Debug("tags") - - if len(*values) > 0 && len(*tags) > 0 { - for _, value := range *values { - for _, tag := range *tags { - if match(tag, value, matchOption) { - matchingTags = append(matchingTags, tag) - log.WithFields(log.Fields{ - "tag": tag, - "value": value}). - Debug("Tag matched") - } + log.WithFields(log.Fields{ + "match": matchOption, + "imageTags": imageTags, + "gitTags": gitTags, + }).Debug("Matching imageTags with gitTags") + + if len(*gitTags) == 0 || len(*imageTags) == 0 { + return matchingTags + } + + for _, gitTag := range *gitTags { + for _, imageTag := range *imageTags { + if match(imageTag, gitTag, matchOption) { + matchingTags = append(matchingTags, imageTag) + log.WithFields(log.Fields{ + "gitTag": gitTag, + "imageTag": imageTag, + }).Debug("Found matching tag") } } } + return matchingTags } @@ -61,8 +68,8 @@ func FilterOrphanImageTags(gitValues, imageTags *[]string, matchOption MatchOpti }).Debug("Filtering image tags by commits...") orphans := funk.FilterString(*imageTags, func(imageTag string) bool { - for _, value := range *gitValues { - if match(imageTag, value, matchOption) { + for _, gitValue := range *gitValues { + if match(imageTag, gitValue, matchOption) { return false } } @@ -79,7 +86,10 @@ func FilterByRegex(imageTags *[]string, regexp *regexp.Regexp) []string { for _, tag := range *imageTags { imageTagMatched := regexp.MatchString(tag) - log.WithField("imageTag:", tag).WithField("match:", imageTagMatched).Debug("Matching image tag") + log.WithFields(log.Fields{ + "imageTag:": tag, + "match:": imageTagMatched, + }).Debug("Matching image tag") if imageTagMatched { matchedTags = append(matchedTags, tag) } @@ -118,12 +128,12 @@ func FilterImageTagsByTime(imageStreamObjectTags *[]imagev1.NamedTagEventList, o return imageStreamTags } -func match(tag, value string, matchOption MatchOption) bool { +func match(imageTag, value string, matchOption MatchOption) bool { switch matchOption { - case MatchOptionDefault, MatchOptionPrefix: - return strings.HasPrefix(tag, value) + case MatchOptionPrefix: + return strings.HasPrefix(imageTag, value) case MatchOptionExact: - return tag == value + return imageTag == value } return false } diff --git a/pkg/cleanup/imagetags_test.go b/pkg/cleanup/imagetags_test.go index 7d114fd..aa6acb8 100644 --- a/pkg/cleanup/imagetags_test.go +++ b/pkg/cleanup/imagetags_test.go @@ -42,6 +42,7 @@ type TagsOlderThanTestCase struct { func Test_GetMatchingTags(t *testing.T) { testcases := []GetMatchingTagsTestCase{ { + matchOption: MatchOptionPrefix, matchValues: []string{ "0b81a958f590ed7ed8", "108f2be974f8e1e5fec8bc759ecf824e81565747", @@ -383,7 +384,7 @@ func TestFilterOrphanImageTags(t *testing.T) { args: args{ gitValues: &[]string{}, imageTags: &[]string{"a1"}, - matchOption: MatchOptionDefault, + matchOption: MatchOptionPrefix, }, want: []string{"a1"}, }, @@ -392,7 +393,7 @@ func TestFilterOrphanImageTags(t *testing.T) { args: args{ gitValues: &[]string{"a1", "a2", "a3"}, imageTags: &[]string{"a1", "b2", "b3"}, - matchOption: MatchOptionDefault, + matchOption: MatchOptionPrefix, }, want: []string{"b2", "b3"}, }, @@ -401,7 +402,7 @@ func TestFilterOrphanImageTags(t *testing.T) { args: args{ gitValues: &[]string{"a1", "a2", "a3"}, imageTags: &[]string{"a1", "a2", "a3"}, - matchOption: MatchOptionDefault, + matchOption: MatchOptionPrefix, }, want: []string{}, }, @@ -410,7 +411,7 @@ func TestFilterOrphanImageTags(t *testing.T) { args: args{ gitValues: &[]string{"a1", "a2", "a3"}, imageTags: &[]string{}, - matchOption: MatchOptionDefault, + matchOption: MatchOptionPrefix, }, want: []string{}, }, diff --git a/pkg/git/git.go b/pkg/git/git.go index 09c4dc4..2f1fcfd 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -81,11 +81,11 @@ func GetGitCandidateList(o *cfg.GitConfig) ([]string, error) { return []string{}, fmt.Errorf("retrieving commit tags failed: %w", err) } return candidates, nil - } else { - candidates, err := GetCommitHashes(o.RepoPath, o.CommitLimit) - if err != nil { - return []string{}, fmt.Errorf("retrieving commit hashes failed: %w", err) - } - return candidates, nil } + candidates, err := GetCommitHashes(o.RepoPath, o.CommitLimit) + if err != nil { + return []string{}, fmt.Errorf("retrieving commit hashes failed: %w", err) + } + return candidates, nil + } diff --git a/pkg/openshift/imagestream.go b/pkg/openshift/imagestream.go index 0f250e2..00b992f 100644 --- a/pkg/openshift/imagestream.go +++ b/pkg/openshift/imagestream.go @@ -25,6 +25,11 @@ var ( // GetActiveImageStreamTags retrieves the image streams tags referenced in some Kubernetes resources func GetActiveImageStreamTags(namespace, imageStream string, imageStreamTags []string) (activeImageStreamTags []string, funcError error) { + log.WithFields(log.Fields{ + "namespace": namespace, + "imageName": imageStream, + "imageTags": imageStreamTags, + }).Debug("Looking for active images") if len(imageStreamTags) == 0 { return []string{}, nil }