Bug 1371511 - add namespace awareness to oadm prune commands #11249
Conversation
| sizedImage("id3", registryURL+"/foo/bar@id3", 200, nil), | ||
| ), | ||
| streams: streamList( | ||
| stream(registryURL, "foo", "bar", tags( |
There was a problem hiding this comment.
One more test having at least two image streams where one belongs to different namespace?
There was a problem hiding this comment.
It doesn't make sense, since namespace filtering is done when listing objects.
| @@ -825,6 +829,10 @@ func (p *pruner) Prune( | |||
| errs := []error{} | |||
|
|
|||
| errs = append(errs, pruneStreams(p.g, prunableImageNodes, streamPruner)...) | |||
| // if namespace is specified prune only ImageStreams and nothing more | |||
There was a problem hiding this comment.
I'd avoid computing graphWithoutPrunableImages and prunableComponents if we prune only image streams.
|
@miminar updated, except for the additional test, as mentioned the filtering is done one layer above. ptal |
|
But I would still like to see at least one test that ensures the pruning affects only chosen namespace. Could you please provide integration or cmd test? |
|
Due to some refactoring I had to do to make the code testable I've split that into 2nd commit, ptal |
| t.Errorf("Unexpected error: %v", err) | ||
| } | ||
|
|
||
| for _, a := range osFake.Actions() { |
There was a problem hiding this comment.
Assert also there's at least one action in Actions list.
| t.Errorf("Unexpected namespace while pruning %s: %s", a.GetResource(), a.GetNamespace()) | ||
| } | ||
| } | ||
| for _, a := range kFake.Actions() { |
|
|
||
| OSClient: osFake, | ||
| KClient: kFake, | ||
| Out: ioutil.Discard, |
| t.Errorf("Unexpected namespace while pruning %s: %s", a.GetResource(), a.GetNamespace()) | ||
| } | ||
| } | ||
| for _, a := range kFake.Actions() { |
|
@miminar nitpicks also addressed |
|
LGTM, thanks! |
|
[merge] |
|
Evaluated for origin merge up to af137c1 |
|
[Test]ing while waiting on the merge queue |
|
Evaluated for origin test up to af137c1 |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/9750/) |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/9785/) (Image: devenv-rhel7_5156) |
Fixes #9502 and https://bugzilla.redhat.com/show_bug.cgi?id=1371511.
@miminar ptal