Improve log messages

Various improvements around log messages to make them more clear.

Signed-off-by: Simon Rüegg <simon@rueggs.ch>

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
 }

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
 }

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
 }
 

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) {

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
 }

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{},
 		},

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
+
 }

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
 	}