Skip to content

Commit

Permalink
fixed code after review
Browse files Browse the repository at this point in the history
  • Loading branch information
@zugao
zugao committed Jan 9, 2020
1 parent 0fcb6f0 commit 8a052dc
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 124 deletions.
57 changes: 41 additions & 16 deletions cmd/imagestream.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func NewImageStreamCleanupCommand() *cobra.Command {
cmd.Flags().StringVarP(&o.Namespace, "namespace", "n", "", "Kubernetes namespace")
cmd.Flags().IntVarP(&o.Keep, "keep", "k", 10, "keep most current <n> images")
cmd.Flags().BoolVarP(&o.Tag, "tag", "t", false, "use tags instead of commit hashes")
cmd.Flags().StringVar(&o.Sorted, "sort-by", git.Version.String(), "sort tags either by version or in alphabetical order. The sort can be version or alphabetic")
cmd.Flags().StringVar(&o.Sorted, "sort", string(git.SortOptionVersion), "sort tags by criteria. Allowed values: [version, alphabetical]")
return cmd
}

Expand All @@ -47,6 +47,10 @@ func (o *ImageStreamCleanupOptions) cleanupImageStreamTags(cmd *cobra.Command, a
o.ImageStream = args[0]
}

if o.Tag && !git.IsValidSortValue(o.Sorted) {
log.WithField("sort_criteria", o.Sorted).Fatal("Invalid sort criteria")
}

if len(o.Namespace) == 0 {
namespace, err := kubernetes.Namespace()
if err != nil {
Expand All @@ -59,56 +63,77 @@ func (o *ImageStreamCleanupOptions) cleanupImageStreamTags(cmd *cobra.Command, a
if len(o.ImageStream) == 0 {
imageStreams, err := openshift.GetImageStreams(o.Namespace)
if err != nil {
log.WithError(err).WithField("namespace", o.Namespace).Fatal("Could not retrieve image streams.")
log.WithError(err).
WithField("namespace", o.Namespace).
Fatal("Could not retrieve image streams.")
}

log.Printf("No image stream provided as argument. Available image streams for namespace %s: %s", o.Namespace, imageStreams)

return
}

var commits []string
var matchValues []string
if o.Tag {
if !git.IsValidSortValue(o.Sorted) {
log.WithField("namespace", o.Namespace).Fatal("Invalid sort flag")
}
var err error
commits, err = git.GetCommitTags(o.RepoPath, o.CommitLimit, git.SortTagBy(o.Sorted))
matchValues, err = git.GetTags(o.RepoPath, o.CommitLimit, git.SortOption(o.Sorted))
if err != nil {
log.WithError(err).WithField("RepoPath", o.RepoPath).WithField("CommitLimit", o.CommitLimit).Fatal("Retrieving commit tags failed.")
log.WithError(err).
WithFields(log.Fields{
"RepoPath": o.RepoPath,
"CommitLimit": o.CommitLimit}).
Fatal("Retrieving commit tags failed.")
}
} else {
var err error
commits, err = git.GetCommitHashes(o.RepoPath, o.CommitLimit)
matchValues, err = git.GetCommitHashes(o.RepoPath, o.CommitLimit)
if err != nil {
log.WithError(err).WithField("RepoPath", o.RepoPath).WithField("CommitLimit", o.CommitLimit).Fatal("Retrieving commit hashes failed.")
log.WithError(err).
WithFields(log.Fields{
"RepoPath": o.RepoPath,
"CommitLimit": o.CommitLimit}).
Fatal("Retrieving commit hashes failed.")
}
}

imageStreamTags, err := openshift.GetImageStreamTags(o.Namespace, o.ImageStream)
if err != nil {
log.WithError(err).WithField("Namespace", o.Namespace).WithField("ImageStream", o.ImageStream).Fatal("Could not retrieve image stream.")
log.WithError(err).
WithFields(log.Fields{
"Namespace": o.Namespace,
"ImageStream": o.ImageStream}).
Fatal("Could not retrieve image stream.")
}

var matchOption cleanup.MatchOption
if o.Tag {
matchOption = cleanup.MatchOptionExact
}

matchingTags := cleanup.GetTagsMatchingPrefixes(&commits, &imageStreamTags, o.Tag)
matchingTags := cleanup.GetMatchingTags(&matchValues, &imageStreamTags, matchOption)

activeImageStreamTags, err := openshift.GetActiveImageStreamTags(o.Namespace, o.ImageStream, imageStreamTags)
if err != nil {
log.WithError(err).WithField("Namespace", o.Namespace).WithField("ImageStream", o.ImageStream).WithField("imageStreamTags", imageStreamTags).Fatal("Could not retrieve active image stream tags.")
log.WithError(err).
WithFields(log.Fields{
"Namespace": o.Namespace,
"ImageStream": o.ImageStream,
"imageStreamTags": imageStreamTags}).
Fatal("Could not retrieve active image stream tags.")
}

inactiveTags := cleanup.GetInactiveTags(&activeImageStreamTags, &matchingTags)

inactiveTags = cleanup.LimitTags(&inactiveTags, o.Keep)

log.Printf("Tags for deletion: %s", inactiveTags)
log.Info("Tags for deletion: %s", inactiveTags)

if o.Force {
for _, inactiveTag := range inactiveTags {
openshift.DeleteImageStreamTag(o.Namespace, openshift.BuildImageStreamTagName(o.ImageStream, inactiveTag))
log.Printf("Deleted image stream tag: %s", inactiveTag)
log.Info("Deleted image stream tag: %s", inactiveTag)
}
} else {
log.Println("--force was not specified. Nothing has been deleted.")
log.Info("--force was not specified. Nothing has been deleted.")
}
}
46 changes: 30 additions & 16 deletions pkg/cleanup/cleanup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,31 @@ import (
log "github.com/sirupsen/logrus"
)

// GetTagsMatchingPrefixes returns all tags matching one of the provided prefixes
func GetTagsMatchingPrefixes(prefixes, tags *[]string, commitTag bool) []string {
// MatchOption type defines how the tags should be matched
type MatchOption int8

const (
MatchOptionDefault MatchOption = iota
MatchOptionExact
MatchOptionPrefix
)

// GetMatchingTags returns all tags matching one of the provided prefixes
func GetMatchingTags(values, tags *[]string, matchOption MatchOption) []string {
var matchingTags []string

log.Debugf("GetTagsMatchingPrefixes | Prefixes: %s", prefixes)
log.Debugf("GetTagsMatchingPrefixes | Tags: %s", tags)
log.WithField("values", values).Debug("values")
log.WithField("tags", tags).Debug("tags")

if len(*prefixes) > 0 && len(*tags) > 0 {
for _, prefix := range *prefixes {
if len(*values) > 0 && len(*tags) > 0 {
for _, value := range *values {
for _, tag := range *tags {
if match(tag, prefix, commitTag) {
if match(tag, value, matchOption) {
matchingTags = append(matchingTags, tag)
log.Debugf("GetTagsMatchingPrefixes | Tag %s matched with %s", tag, prefix)
log.WithFields(log.Fields{
"tag": tag,
"value": value}).
Debug("Tag matched")
}
}
}
Expand All @@ -30,15 +42,15 @@ func GetTagsMatchingPrefixes(prefixes, tags *[]string, commitTag bool) []string
func GetInactiveTags(activeTags, tags *[]string) []string {
var inactiveTags []string

log.Debugf("GetInactiveTags | Active tags: %s", activeTags)
log.Debugf("GetInactiveTags | Tags: %s", tags)
log.WithField("activeTags", activeTags).Debug("Active tags")
log.WithField("tags", tags).Debug("Tags")

for _, tag := range *tags {
active := false
for _, activeTag := range *activeTags {
if tag == activeTag {
active = true
log.Debugf("GetInactiveTags | Tag %s is part of the active tags", tag)
log.WithField("tag", tag).Debug("The tag is part of the active tags")
break
}
}
Expand All @@ -61,10 +73,12 @@ func LimitTags(tags *[]string, keep int) []string {
return []string{}
}

//Depending on commit type (hash or tag) use different matching logic
func match(tag, prefix string, commitTag bool) bool {
if commitTag {
return tag == prefix
func match(tag, value string, matchOption MatchOption) bool {
switch matchOption {
case MatchOptionDefault, MatchOptionPrefix:
return strings.HasPrefix(tag, value)
case MatchOptionExact:
return tag == value
}
return strings.HasPrefix(tag, prefix)
return false
}
29 changes: 11 additions & 18 deletions pkg/cleanup/cleanup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import (
"github.com/stretchr/testify/assert"
)

type GetTagsMatchingPrefixesTestCase struct {
prefixes, tags, expected []string
type GetMatchingTagsTestCase struct {
matchValues, tags, expected []string
matchOption MatchOption
}

type GetInactiveTagsTestCase struct {
Expand All @@ -19,10 +20,10 @@ type LimitTagsTestCase struct {
limit int
}

func Test_GetTagsMatchingPrefixesCommitHashes(t *testing.T) {
testcases := []GetTagsMatchingPrefixesTestCase{
GetTagsMatchingPrefixesTestCase{
prefixes: []string{
func Test_GetMatchingTags(t *testing.T) {
testcases := []GetMatchingTagsTestCase{
GetMatchingTagsTestCase{
matchValues: []string{
"0b81a958f590ed7ed8",
"108f2be974f8e1e5fec8bc759ecf824e81565747",
"4cb7de27c985216b8888ff6049294dae02f3282e",
Expand All @@ -45,17 +46,8 @@ func Test_GetTagsMatchingPrefixesCommitHashes(t *testing.T) {
"c8a693ad89e7069674eda512c553ff56d3ca2ffd-debug",
},
},
}

for _, testcase := range testcases {
assert.Equal(t, testcase.expected, GetTagsMatchingPrefixes(&testcase.prefixes, &testcase.tags, false))
}
}

func Test_GetTagsMatchingPrefixesCommitTags(t *testing.T) {
testcases := []GetTagsMatchingPrefixesTestCase{
GetTagsMatchingPrefixesTestCase{
prefixes: []string{
GetMatchingTagsTestCase{
matchValues: []string{
"v1.0.2",
"2.3",
"1.0",
Expand All @@ -70,6 +62,7 @@ func Test_GetTagsMatchingPrefixesCommitTags(t *testing.T) {
"0.0.2",
"v2.3.0",
},
matchOption: MatchOptionExact,
expected: []string{
"v1.0.2",
"1.0",
Expand All @@ -78,7 +71,7 @@ func Test_GetTagsMatchingPrefixesCommitTags(t *testing.T) {
}

for _, testcase := range testcases {
assert.Equal(t, testcase.expected, GetTagsMatchingPrefixes(&testcase.prefixes, &testcase.tags, true))
assert.Equal(t, testcase.expected, GetMatchingTags(&testcase.matchValues, &testcase.tags, testcase.matchOption))
}
}

Expand Down
43 changes: 3 additions & 40 deletions pkg/git/git.go
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
package git

import (
"errors"
"io"
"sort"
"strings"

"github.com/hashicorp/go-version"
log "github.com/sirupsen/logrus"
"gopkg.in/src-d/go-git.v4"
)

Expand Down Expand Up @@ -40,8 +36,8 @@ func GetCommitHashes(repoPath string, commitLimit int) ([]string, error) {
return commitHashes, nil
}

// GetCommitTags returns the commit tags of a given repository ordered alphabetically or by version. If `commitLimit` is -1 all tags will be returned.
func GetCommitTags(repoPath string, tagLimit int, sortTagBy SortTagBy) ([]string, error) {
// GetTags returns the commit tags of a given repository ordered alphabetically or by version. If `commitLimit` is -1 all tags will be returned.
func GetTags(repoPath string, tagLimit int, sortTagBy SortOption) ([]string, error) {
var commitTags []string

repository, err := git.PlainOpen(repoPath)
Expand Down Expand Up @@ -71,38 +67,5 @@ func GetCommitTags(repoPath string, tagLimit int, sortTagBy SortTagBy) ([]string
commitTags = append(commitTags, tagName)
}

return Sort(commitTags, sortTagBy)
}

// Sort function sorts the slice according to the sort type
func Sort(tags []string, sortTagBy SortTagBy) ([]string, error) {
switch sortTagBy {

case Version:
var versionTags []*version.Version
for _, raw := range tags {
version, err := version.NewVersion(raw)
if err != nil {
log.WithError(err).WithField("tag", raw).Warn("Skipped invalid version")
} else {
versionTags = append(versionTags, version)
}
}

sort.Sort(sort.Reverse(version.Collection(versionTags)))

sortedTags := make([]string, len(versionTags))
for i, sortedVersion := range versionTags {
sortedTags[i] = sortedVersion.Original()
}

return sortedTags, nil

case Alphabetic:
sort.Strings(tags)
return tags, nil

default:
return nil, errors.New("Undefined sort type")
}
return sortTags(commitTags, sortTagBy)
}
20 changes: 10 additions & 10 deletions pkg/git/git_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ func Test_GetCommitHashesFail(t *testing.T) {
assert.Error(t, err)
}

func Test_GetCommitTagsSortedInAlphabeticalOrder(t *testing.T) {
func Test_GetTagsSortedInAlphabeticalOrder(t *testing.T) {
commitLimit := 2
commitHashes, err := GetCommitTags("../..", commitLimit, "alphabetic") // Open repository from root dir
commitHashes, err := GetTags("../..", commitLimit, "alphabetic") // Open repository from root dir

expectInOrder := []string{"0.0.1", "v0.1.0"}

Expand All @@ -39,9 +39,9 @@ func Test_GetCommitTagsSortedInAlphabeticalOrder(t *testing.T) {
assert.EqualValues(t, commitHashes, expectInOrder)
}

func Test_GetCommitTagsSortedByVersion(t *testing.T) {
func Test_GetTagsSortedByVersion(t *testing.T) {
commitLimit := 2
commitHashes, err := GetCommitTags("../..", commitLimit, "version") // Open repository from root dir
commitHashes, err := GetTags("../..", commitLimit, "version") // Open repository from root dir

expectInOrder := []string{"v0.1.0", "0.0.1"}

Expand All @@ -50,16 +50,16 @@ func Test_GetCommitTagsSortedByVersion(t *testing.T) {
assert.EqualValues(t, commitHashes, expectInOrder)
}

func Test_GetCommitAllTags(t *testing.T) {
func Test_GetAllTags(t *testing.T) {
commitLimit := -1
_, err := GetCommitTags("../..", commitLimit, "version") // Open repository from root dir
_, err := GetTags("../..", commitLimit, "version") // Open repository from root dir

assert.NoError(t, err)
}

func Test_GetCommitTagsFail(t *testing.T) {
func Test_GetTagsFail(t *testing.T) {
commitLimit := 2
_, err := GetCommitTags("not-a-repo", commitLimit, "version")
_, err := GetTags("not-a-repo", commitLimit, "version")

assert.Error(t, err)
}
Expand All @@ -69,7 +69,7 @@ func Test_SortByVersion(t *testing.T) {
unsortedTags := []string{"v3.0.1", "0.3", "v2.1.1", "0.0.1", "v5.0.2", "4.0.1-beta", "v3.0.0-alpha", "v3", "0.0.2", "v0.2.0", "3.0.0", "random"}
expectedSortedTags := []string{"v5.0.2", "4.0.1-beta", "v3.0.1", "v3", "3.0.0", "v3.0.0-alpha", "v2.1.1", "0.3", "v0.2.0", "0.0.2", "0.0.1"}

sortedTags, err := Sort(unsortedTags, Version)
sortedTags, err := sortTags(unsortedTags, SortOptionVersion)

assert.NoError(t, err)
assert.EqualValues(t, expectedSortedTags, sortedTags)
Expand All @@ -80,7 +80,7 @@ func Test_SortBInAlphabeticalOrder(t *testing.T) {
unsortedTags := []string{"v3.0.1", "0.3", "v2.1.1", "0.0.1", "v5.0.2", "4.0.1-beta", "v3.0.0-alpha", "v3", "0.0.2", "v0.2.0", "3.0.0", "random"}
expectedSortedTags := []string{"0.0.1", "0.0.2", "0.3", "3.0.0", "4.0.1-beta", "random", "v0.2.0", "v2.1.1", "v3", "v3.0.0-alpha", "v3.0.1", "v5.0.2"}

sortedTags, err := Sort(unsortedTags, Alphabetic)
sortedTags, err := sortTags(unsortedTags, SortOptionAlphabetic)

assert.NoError(t, err)
assert.EqualValues(t, expectedSortedTags, sortedTags)
Expand Down
Loading

0 comments on commit 8a052dc

Please sign in to comment.