Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Image filtering and ordering with semver #1266

Merged
merged 4 commits into from
Aug 6, 2018
Merged

Image filtering and ordering with semver #1266

merged 4 commits into from
Aug 6, 2018

Conversation

rndstr
Copy link
Contributor

@rndstr rndstr commented Aug 1, 2018

Currently, containers can be tagged in manifests to filter what image
tags are considered when doing automated releases. Filtering is done by
specifying a wildcard glob. An optional prefix glob: can be used.

This PR adds support for tag filters based on semantic versioning
by using the prefix semver: instead. Version constraints can be
specified that filter images. Since versions have an implicit ordering
this also changes the way images are sorted when trying to determine the
newest image. For glob filtering this falls back to image creation date.

Closes #706

@rndstr rndstr requested review from squaremo and aaron7 August 1, 2018 23:14
@rndstr rndstr force-pushed the issue/706-semver-filtering-ordering branch from 54caaba to 7091c03 Compare August 1, 2018 23:18
@stefanprodan
Copy link
Member

Does this supports the v0.1.2-alpha1 format? Many images have a v prefix for sem ver, that's because GitHub encourages this convention in the release UI.

Some examples:

@rndstr
Copy link
Contributor Author

rndstr commented Aug 1, 2018

@stefanprodan it does recognize versions with a v prefix. As for pre-releases, I do think that per spec, the range constraints do ignore them unless mentioned explicitly.
So semver:~1 will ignore 1.2.3-alpha.1 but pick 1.2.2.

@rndstr rndstr force-pushed the issue/706-semver-filtering-ordering branch from 7091c03 to 67ff574 Compare August 2, 2018 00:52
squaremo
squaremo suggested changes Aug 2, 2018
View reviewed changes
Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a thorough job with some nice refactorings and tidies 💯

I made some comments. Most just offer advice or invite an quick explanation or rebuttal; the only thing I think definitely needs a code change is the accidental change in meaning in policy_cmd.go.

Gopkg.toml Outdated
@@ -47,5 +47,5 @@ required = ["k8s.io/code-generator/cmd/client-gen"]
version = "v1.0.0"

[[constraint]]
branch = "master"

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

cmd/fluxctl/policy_cmd.go Outdated
@@ -142,7 +142,7 @@ func calculatePolicyChanges(opts *controllerPolicyOpts) (policy.Update, error) {
Add(policy.LockedUser)
}
if opts.tagAll != "" {
add = add.Set(policy.TagAll, "glob:"+opts.tagAll)
add = add.Set(policy.TagAll, policy.PatternAll.String())

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

policy/pattern_test.go Outdated
name: "semver",
pattern: "semver:~1",
true: []string{"v1", "1", "1.2", "1.2.3"},
false: []string{"", "latest", "2.0.0", ""},

This comment was marked as abuse.

Sign in to view

policy/pattern_test.go
name: "all",
pattern: "semver:*",
true: []string{"1", "1.0", "v1.0.3"},
false: []string{"", "latest", "2.0.1-alpha.1"},

This comment was marked as abuse.

Sign in to view

policy/policy.go Outdated
if services == nil {
return "*"
return PatternAll
}
policies := services[service]

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

update/release.go Outdated
@@ -231,7 +231,7 @@ func (s ReleaseSpec) calculateImageUpdates(rc ReleaseContext, candidates []*Cont
for _, container := range containers {
currentImageID := container.Image

filteredImages := imageRepos.GetRepoImages(currentImageID.Name).Filter("*")
filteredImages := imageRepos.GetRepoImages(currentImageID.Name).Filter(policy.PatternAll)

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

daemon/daemon.go Outdated
return cs.controllers[i].ContainersOrNil()
}

func (cs clusterContainers) Pattern(i int, container string) policy.Pattern {

This comment was marked as abuse.

Sign in to view

daemon/daemon.go Outdated
}

policyResourceMap, _, err := d.getPolicyResourceMap(ctx)
imageRepos, err := update.FetchImageRepos(d.Registry, clusterContainers{controllers: services, policies: policyResourceMap}, d.Logger)

This comment was marked as abuse.

Sign in to view

policy/pattern.go Outdated
// String returns the prefixed string representation.
String() string
// ImageNewerFunc returns a function to compare image newness.
ImageNewerFunc() image.LessFunc

This comment was marked as abuse.

Sign in to view

update/images.go
@@ -38,24 +38,30 @@ func (r ImageRepos) GetRepoImages(repo image.Name) ImageInfos {
// ImageInfos is a list of image.Info which can be filtered.
type ImageInfos []image.Info

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@rndstr rndstr force-pushed the issue/706-semver-filtering-ordering branch 9 times, most recently from 440c710 to 3c7570f Compare August 2, 2018 21:21
squaremo
squaremo approved these changes Aug 3, 2018
View reviewed changes
Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great PR, thanks Roli. Nits/suggestions, but if this all compiles/passes/works in practice, I'm happy with it.

cmd/fluxctl/policy_cmd.go Outdated
@@ -142,7 +142,7 @@ func calculatePolicyChanges(opts *controllerPolicyOpts) (policy.Update, error) {
Add(policy.LockedUser)
}
if opts.tagAll != "" {
add = add.Set(policy.TagAll, policy.PatternAll.String())
add = add.Set(policy.TagAll, policy.NewPattern(opts.tagAll).String())

This comment was marked as abuse.

Sign in to view

policy/pattern_test.go Outdated
false: []string{"", "latest", "2.0.0"},
},
{
name: "semver",

This comment was marked as abuse.

Sign in to view

update/images.go Outdated
}
}
filtered.Sort(pattern)
return filtered
}

func (ii ImageInfos) Sort(pattern policy.Pattern) {
image.Sort(ii, pattern.ImageNewerFunc())
image.Sort(ii, pattern.Newer)

This comment was marked as abuse.

Sign in to view

update/images.go
}
return filtered
// SortedImageInfos is a list of sorted image.Info
type SortedImageInfos []image.Info

This comment was marked as abuse.

Sign in to view

rndstr added 2 commits August 3, 2018 13:34
@rndstr
Support semver in container filter tag
f7a99c3 
Currently, containers can be tagged in manifests to filter what image
tags are considered when doing automated releases. Filtering is done by
specifying a wildcard glob. An optional prefix `glob:` can be used.

This PR adds support for tag filters based on [semantic versioning][0]
by using the prefix `semver:` instead. Version constraints can be
specified that filter images. Since versions have an implicit ordering
this also changes the way images are sorted when trying to determine the
newest image. For glob filtering this falls back to image creation date.

[0]: https://semver.org
@rndstr
Split off SortedImageInfos to enforce constraint
4c8fabb
@rndstr rndstr force-pushed the issue/706-semver-filtering-ordering branch from 3c7570f to 4c8fabb Compare August 3, 2018 20:35
@rndstr
Copy link
Contributor Author

rndstr commented Aug 3, 2018

thanks for the thorough review @squaremo! I fixed up the commit history and will merge once #1263 makes it in as this one depends on it.

@rndstr rndstr changed the base branch from fix/test-mock-image-order to master August 6, 2018 21:52
@rndstr rndstr merged commit be0aa43 into master Aug 6, 2018
@rndstr rndstr mentioned this pull request Aug 7, 2018
Merged
@hiddeco hiddeco deleted the issue/706-semver-filtering-ordering branch April 11, 2019 13:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@squaremo squaremo squaremo approved these changes

@aaron7 aaron7 Awaiting requested review from aaron7

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

semver filtering and ordering
3 participants
@rndstr @stefanprodan @squaremo