Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Imagestream tag exclude from pruning - using object selection #17511

Closed
wants to merge 4 commits into from
Closed

Imagestream tag exclude from pruning - using object selection #17511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
70d88f8
add label selection for image prune
deads2k Nov 29, 2017
30a2a0e
generated
deads2k Dec 18, 2017
74337de
debug
deads2k Dec 19, 2017
7f60fe8
debug
deads2k Dec 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions contrib/completions/bash/oc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5327,6 +5327,9 @@ _oc_adm_prune_images()
local_nonpersistent_flags+=("--prune-registry")
flags+=("--registry-url=")
local_nonpersistent_flags+=("--registry-url=")
flags+=("--selector=")
two_word_flags+=("-l")
local_nonpersistent_flags+=("--selector=")
flags+=("--as=")
flags+=("--as-group=")
flags+=("--cache-dir=")
Expand Down
3 changes: 3 additions & 0 deletions contrib/completions/zsh/oc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5469,6 +5469,9 @@ _oc_adm_prune_images()
local_nonpersistent_flags+=("--prune-registry")
flags+=("--registry-url=")
local_nonpersistent_flags+=("--registry-url=")
flags+=("--selector=")
two_word_flags+=("-l")
local_nonpersistent_flags+=("--selector=")
flags+=("--as=")
flags+=("--as-group=")
flags+=("--cache-dir=")
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/bootstrappolicy/policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ func GetOpenshiftBootstrapClusterRoles() []rbac.ClusterRole {
Name: ImagePrunerRoleName,
},
Rules: []rbac.PolicyRule{
rbac.NewRule("get", "list").Groups(kapiGroup).Resources("pods", "replicationcontrollers").RuleOrDie(),
rbac.NewRule("get", "list").Groups(kapiGroup).Resources("namespaces", "pods", "replicationcontrollers").RuleOrDie(),
rbac.NewRule("list").Groups(kapiGroup).Resources("limitranges").RuleOrDie(),
rbac.NewRule("get", "list").Groups(buildGroup, legacyBuildGroup).Resources("buildconfigs", "builds").RuleOrDie(),
rbac.NewRule("get", "list").Groups(deployGroup, legacyDeployGroup).Resources("deploymentconfigs").RuleOrDie(),
Expand Down
32 changes: 28 additions & 4 deletions pkg/image/prune/prune.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ const (
// ReferencedImageLayerEdgeKind defines an edge from an ImageStreamNode or an
// ImageNode to an ImageComponentNode.
ReferencedImageLayerEdgeKind = "ReferencedImageLayer"

// TODO choose final name and promote to API
protectionAnnotation = "image.openshift.io/prune-immunity"
Copy link
Contributor

Choose a reason for hiding this comment

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

@openshift/api-review ptal (yes David I know it's you, but also others 😉)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@openshift/api-review ptal (yes David I know it's you, but also others )

Fair. :)

)

// pruneAlgorithm contains the various settings to use when evaluating images
Expand Down Expand Up @@ -126,6 +129,12 @@ type PrunerOptions struct {
PruneRegistry *bool
// Namespace to be pruned, if specified it should never remove Images.
Namespace string

// NamespacesToPrune is the set of namespaces which are eligible to prune
NamespacesToPrune sets.String
// NamespacesToAllowImmunity is the set of namespaces which allow imagestreamtags to be excluded
NamespacesToAllowImmunity sets.String

// Images is the entire list of images in OpenShift. An image must be in this
// list to be a candidate for pruning.
Images *imageapi.ImageList
Expand Down Expand Up @@ -176,6 +185,9 @@ type pruner struct {
algorithm pruneAlgorithm
registryClient *http.Client
registryURL *url.URL

namespacesToPrune sets.String
namespacesToAllowImmunity sets.String
}

var _ Pruner = &pruner{}
Expand Down Expand Up @@ -252,9 +264,11 @@ func NewPruner(options PrunerOptions) (Pruner, kerrors.Aggregate) {
algorithm.namespace = options.Namespace

p := &pruner{
algorithm: algorithm,
registryClient: options.RegistryClient,
registryURL: options.RegistryURL,
algorithm: algorithm,
registryClient: options.RegistryClient,
registryURL: options.RegistryURL,
namespacesToPrune: options.NamespacesToPrune,
namespacesToAllowImmunity: options.NamespacesToAllowImmunity,
}

if err := p.buildGraph(options); err != nil {
Expand Down Expand Up @@ -361,8 +375,18 @@ func (p *pruner) addImageStreamsToGraph(streams *imageapi.ImageStreamList, limit
continue
}

isNamespaceEligible := p.namespacesToPrune.Has(stream.Namespace)
doesNamespaceAllowImmunity := p.namespacesToAllowImmunity.Has(stream.Namespace)
isISTagImmune := stream.Spec.Tags[tag].Annotations[protectionAnnotation] == "true"
Copy link

Choose a reason for hiding this comment

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

I'd welcome per-imagestream control as well. Where istag's annotation overrides is' annotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd welcome per-imagestream control as well. Where istag's annotation overrides is' annotation.

It's easy enough to add, but the initial use-case didn't request it. @miminar could it be a followup?

@soltysh @fabianofranz @juanvallejo add it or no?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for adding it (can be followup). IIRC discussions in the topic it was about both IS and ISTag to be excluded from pruning.

Copy link

Choose a reason for hiding this comment

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

👍 for a follow-up


kind := oldImageRevisionReferenceKind
if p.algorithm.pruneOverSizeLimit {
if !isNamespaceEligible {
glog.V(4).Infof("Namespace %q is not included", istNode.Namespace)
kind = ReferencedImageEdgeKind
} else if doesNamespaceAllowImmunity && isISTagImmune {
glog.V(4).Infof("%v/%v is protected from pruning", istNode.Namespace, istNode.Name)
kind = ReferencedImageEdgeKind
} else if p.algorithm.pruneOverSizeLimit {
if exceedsLimits(stream, imageNode.Image, limits) {
kind = WeakReferencedImageEdgeKind
} else {
Expand Down
151 changes: 91 additions & 60 deletions pkg/image/prune/prune_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,30 @@ func TestImagePruning(t *testing.T) {
registryURL := "https://" + registryHost

tests := []struct {
name string
pruneOverSizeLimit *bool
allImages *bool
pruneRegistry *bool
keepTagRevisions *int
namespace string
images imageapi.ImageList
pods kapi.PodList
streams imageapi.ImageStreamList
rcs kapi.ReplicationControllerList
bcs buildapi.BuildConfigList
builds buildapi.BuildList
dss kapisext.DaemonSetList
deployments kapisext.DeploymentList
dcs appsapi.DeploymentConfigList
rss kapisext.ReplicaSetList
limits map[string][]*kapi.LimitRange
expectedImageDeletions []string
expectedStreamUpdates []string
expectedLayerLinkDeletions []string
expectedBlobDeletions []string
expectedErrorString string
name string
pruneOverSizeLimit *bool
allImages *bool
pruneRegistry *bool
keepTagRevisions *int
namespace string
namespacesToPrune []string
namespacesToAllowProtection []string
images imageapi.ImageList
pods kapi.PodList
streams imageapi.ImageStreamList
rcs kapi.ReplicationControllerList
bcs buildapi.BuildConfigList
builds buildapi.BuildList
dss kapisext.DaemonSetList
deployments kapisext.DeploymentList
dcs appsapi.DeploymentConfigList
rss kapisext.ReplicaSetList
limits map[string][]*kapi.LimitRange
expectedImageDeletions []string
expectedStreamUpdates []string
expectedLayerLinkDeletions []string
expectedBlobDeletions []string
expectedErrorString string
}{
{
name: "1 pod - phase pending - don't prune",
Expand Down Expand Up @@ -374,6 +376,29 @@ func TestImagePruning(t *testing.T) {
expectedStreamUpdates: []string{"foo/bar|sha256:0000000000000000000000000000000000000000000000000000000000000004"},
},

{
name: "image stream - keep most recent n images - only prune in bar, so nothing is removed",
images: testutil.ImageList(
testutil.UnmanagedImage("sha256:0000000000000000000000000000000000000000000000000000000000000000", "otherregistry/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000000", false, "", ""),
testutil.Image("sha256:0000000000000000000000000000000000000000000000000000000000000002", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000002"),
testutil.Image("sha256:0000000000000000000000000000000000000000000000000000000000000003", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000003"),
testutil.Image("sha256:0000000000000000000000000000000000000000000000000000000000000004", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000004"),
),
streams: testutil.StreamList(
testutil.Stream(registryHost, "foo", "bar", testutil.Tags(
testutil.Tag("latest",
testutil.TagEvent("sha256:0000000000000000000000000000000000000000000000000000000000000000", "otherregistry/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000000"),
testutil.TagEvent("sha256:0000000000000000000000000000000000000000000000000000000000000002", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000002"),
testutil.TagEvent("sha256:0000000000000000000000000000000000000000000000000000000000000003", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000003"),
testutil.TagEvent("sha256:0000000000000000000000000000000000000000000000000000000000000004", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000004"),
),
)),
),
namespacesToPrune: []string{"bar"},
expectedImageDeletions: []string{},
expectedStreamUpdates: []string{},
},

{
name: "image stream - same manifest listed multiple times in tag history",
images: testutil.ImageList(
Expand Down Expand Up @@ -879,20 +904,24 @@ func TestImagePruning(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
options := PrunerOptions{
Namespace: test.namespace,
AllImages: test.allImages,
Images: &test.images,
Streams: &test.streams,
Pods: &test.pods,
RCs: &test.rcs,
BCs: &test.bcs,
Builds: &test.builds,
DSs: &test.dss,
Deployments: &test.deployments,
DCs: &test.dcs,
RSs: &test.rss,
LimitRanges: test.limits,
RegistryURL: &url.URL{Scheme: "https", Host: registryHost},
Namespace: test.namespace,
NamespacesToPrune: sets.NewString("foo", "bar"),
AllImages: test.allImages,
Images: &test.images,
Streams: &test.streams,
Pods: &test.pods,
RCs: &test.rcs,
BCs: &test.bcs,
Builds: &test.builds,
DSs: &test.dss,
Deployments: &test.deployments,
DCs: &test.dcs,
RSs: &test.rss,
LimitRanges: test.limits,
RegistryURL: &url.URL{Scheme: "https", Host: registryHost},
}
if len(test.namespacesToPrune) > 0 {
options.NamespacesToPrune = sets.NewString(test.namespacesToPrune...)
}
if test.pruneOverSizeLimit != nil {
options.PruneOverSizeLimit = test.pruneOverSizeLimit
Expand Down Expand Up @@ -1181,20 +1210,21 @@ func TestRegistryPruning(t *testing.T) {
keepYoungerThan := 60 * time.Minute
keepTagRevisions := 1
options := PrunerOptions{
KeepYoungerThan: &keepYoungerThan,
KeepTagRevisions: &keepTagRevisions,
PruneRegistry: &test.pruneRegistry,
Images: &test.images,
Streams: &test.streams,
Pods: &kapi.PodList{},
RCs: &kapi.ReplicationControllerList{},
BCs: &buildapi.BuildConfigList{},
Builds: &buildapi.BuildList{},
DSs: &kapisext.DaemonSetList{},
Deployments: &kapisext.DeploymentList{},
DCs: &appsapi.DeploymentConfigList{},
RSs: &kapisext.ReplicaSetList{},
RegistryURL: &url.URL{Scheme: "https", Host: "registry1.io"},
NamespacesToPrune: sets.NewString("foo", "bar"),
KeepYoungerThan: &keepYoungerThan,
KeepTagRevisions: &keepTagRevisions,
PruneRegistry: &test.pruneRegistry,
Images: &test.images,
Streams: &test.streams,
Pods: &kapi.PodList{},
RCs: &kapi.ReplicationControllerList{},
BCs: &buildapi.BuildConfigList{},
Builds: &buildapi.BuildList{},
DSs: &kapisext.DaemonSetList{},
Deployments: &kapisext.DeploymentList{},
DCs: &appsapi.DeploymentConfigList{},
RSs: &kapisext.ReplicaSetList{},
RegistryURL: &url.URL{Scheme: "https", Host: "registry1.io"},
}
p, err := NewPruner(options)
if err != nil {
Expand Down Expand Up @@ -1258,16 +1288,17 @@ func TestImageWithStrongAndWeakRefsIsNotPruned(t *testing.T) {
rss := testutil.RSList()

options := PrunerOptions{
Images: &images,
Streams: &streams,
Pods: &pods,
RCs: &rcs,
BCs: &bcs,
Builds: &builds,
DSs: &dss,
Deployments: &deployments,
DCs: &dcs,
RSs: &rss,
NamespacesToPrune: sets.NewString("foo", "bar"),
Images: &images,
Streams: &streams,
Pods: &pods,
RCs: &rcs,
BCs: &bcs,
Builds: &builds,
DSs: &dss,
Deployments: &deployments,
DCs: &dcs,
RSs: &rss,
}
keepYoungerThan := 24 * time.Hour
keepTagRevisions := 2
Expand Down
Loading