Merge pull request #17344 from openshift/revert-16580-image-exclude-from-prune

Revert "Imagestream tag exclude from pruning"

Author: mfojtik

contrib/completions/bash/oadm

@@ -4746,10 +4746,6 @@ _oadm_prune_images()
     local_nonpersistent_flags+=("--certificate-authority=")
     flags+=("--confirm")
     local_nonpersistent_flags+=("--confirm")
-    flags+=("--exclude-imagestreamtag=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag=")
-    flags+=("--exclude-imagestreamtag-file=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag-file=")
     flags+=("--force-insecure")
     local_nonpersistent_flags+=("--force-insecure")
     flags+=("--keep-tag-revisions=")

contrib/completions/bash/oc

@@ -4911,10 +4911,6 @@ _oc_adm_prune_images()
     local_nonpersistent_flags+=("--certificate-authority=")
     flags+=("--confirm")
     local_nonpersistent_flags+=("--confirm")
-    flags+=("--exclude-imagestreamtag=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag=")
-    flags+=("--exclude-imagestreamtag-file=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag-file=")
     flags+=("--force-insecure")
     local_nonpersistent_flags+=("--force-insecure")
     flags+=("--keep-tag-revisions=")

contrib/completions/bash/openshift

@@ -4746,10 +4746,6 @@ _openshift_admin_prune_images()
     local_nonpersistent_flags+=("--certificate-authority=")
     flags+=("--confirm")
     local_nonpersistent_flags+=("--confirm")
-    flags+=("--exclude-imagestreamtag=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag=")
-    flags+=("--exclude-imagestreamtag-file=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag-file=")
     flags+=("--force-insecure")
     local_nonpersistent_flags+=("--force-insecure")
     flags+=("--keep-tag-revisions=")
@@ -10080,10 +10076,6 @@ _openshift_cli_adm_prune_images()
     local_nonpersistent_flags+=("--certificate-authority=")
     flags+=("--confirm")
     local_nonpersistent_flags+=("--confirm")
-    flags+=("--exclude-imagestreamtag=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag=")
-    flags+=("--exclude-imagestreamtag-file=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag-file=")
     flags+=("--force-insecure")
     local_nonpersistent_flags+=("--force-insecure")
     flags+=("--keep-tag-revisions=")

contrib/completions/zsh/oadm

@@ -4895,10 +4895,6 @@ _oadm_prune_images()
     local_nonpersistent_flags+=("--certificate-authority=")
     flags+=("--confirm")
     local_nonpersistent_flags+=("--confirm")
-    flags+=("--exclude-imagestreamtag=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag=")
-    flags+=("--exclude-imagestreamtag-file=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag-file=")
     flags+=("--force-insecure")
     local_nonpersistent_flags+=("--force-insecure")
     flags+=("--keep-tag-revisions=")

contrib/completions/zsh/oc

@@ -5060,10 +5060,6 @@ _oc_adm_prune_images()
     local_nonpersistent_flags+=("--certificate-authority=")
     flags+=("--confirm")
     local_nonpersistent_flags+=("--confirm")
-    flags+=("--exclude-imagestreamtag=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag=")
-    flags+=("--exclude-imagestreamtag-file=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag-file=")
     flags+=("--force-insecure")
     local_nonpersistent_flags+=("--force-insecure")
     flags+=("--keep-tag-revisions=")

contrib/completions/zsh/openshift

@@ -4895,10 +4895,6 @@ _openshift_admin_prune_images()
     local_nonpersistent_flags+=("--certificate-authority=")
     flags+=("--confirm")
     local_nonpersistent_flags+=("--confirm")
-    flags+=("--exclude-imagestreamtag=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag=")
-    flags+=("--exclude-imagestreamtag-file=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag-file=")
     flags+=("--force-insecure")
     local_nonpersistent_flags+=("--force-insecure")
     flags+=("--keep-tag-revisions=")
@@ -10229,10 +10225,6 @@ _openshift_cli_adm_prune_images()
     local_nonpersistent_flags+=("--certificate-authority=")
     flags+=("--confirm")
     local_nonpersistent_flags+=("--confirm")
-    flags+=("--exclude-imagestreamtag=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag=")
-    flags+=("--exclude-imagestreamtag-file=")
-    local_nonpersistent_flags+=("--exclude-imagestreamtag-file=")
     flags+=("--force-insecure")
     local_nonpersistent_flags+=("--force-insecure")
     flags+=("--keep-tag-revisions=")

pkg/image/prune/prune.go

@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"net/url"
 	"reflect"
-	"regexp"
 	"time"
 
 	"github.com/docker/distribution/manifest/schema2"
@@ -60,12 +59,11 @@ const (
 // pruneAlgorithm contains the various settings to use when evaluating images
 // and layers for pruning.
 type pruneAlgorithm struct {
-	keepYoungerThan               time.Time
-	keepTagRevisions              int
-	pruneOverSizeLimit            bool
-	namespace                     string
-	allImages                     bool
-	excludeImageStreamTagPatterns []*regexp.Regexp
+	keepYoungerThan    time.Time
+	keepTagRevisions   int
+	pruneOverSizeLimit bool
+	namespace          string
+	allImages          bool
 }
 
 // ImageDeleter knows how to remove images from OpenShift.
@@ -155,8 +153,6 @@ type PrunerOptions struct {
 	RegistryClient *http.Client
 	// RegistryURL is the URL of the integrated Docker registry.
 	RegistryURL *url.URL
-	// ExcludeImageStreamTagPatterns is the list of regular expressions to exclude an imagesteam tag from pruning.
-	ExcludeImageStreamTagPatterns []*regexp.Regexp
 }
 
 // Pruner knows how to prune istags, images, layers and image configs.
@@ -244,7 +240,6 @@ func NewPruner(options PrunerOptions) (Pruner, kerrors.Aggregate) {
 		algorithm.allImages = *options.AllImages
 	}
 	algorithm.namespace = options.Namespace
-	algorithm.excludeImageStreamTagPatterns = options.ExcludeImageStreamTagPatterns
 
 	p := &pruner{
 		algorithm:      algorithm,
@@ -356,30 +351,8 @@ func (p *pruner) addImageStreamsToGraph(streams *imageapi.ImageStreamList, limit
 					continue
 				}
 
-				istExcluded := false
-				dockerImageReference := ""
-
-				if p.algorithm.excludeImageStreamTagPatterns != nil {
-					dockerImageReference = imageapi.DockerImageReference{
-						Namespace: stream.Namespace,
-						Name:      stream.Name,
-						Tag:       tag,
-					}.Exact()
-
-					for _, pattern := range p.algorithm.excludeImageStreamTagPatterns {
-						if pattern.MatchString(dockerImageReference) {
-							istExcluded = true
-							break
-						}
-					}
-				}
-
 				kind := oldImageRevisionReferenceKind
-
-				if istExcluded {
-					glog.V(4).Infof("ImageStreamTag %q is excluded by the regular expression", dockerImageReference)
-					kind = ReferencedImageEdgeKind
-				} else if p.algorithm.pruneOverSizeLimit {
+				if p.algorithm.pruneOverSizeLimit {
 					if exceedsLimits(stream, imageNode.Image, limits) {
 						kind = WeakReferencedImageEdgeKind
 					} else {

pkg/oc/admin/prune/images.go

@@ -1,7 +1,6 @@
 package prune
 
 import (
-	"bufio"
 	"crypto/x509"
 	"encoding/json"
 	"errors"
@@ -87,10 +86,6 @@ var (
 	  # To actually perform the prune operation, the confirm flag must be appended
 	  %[1]s %[2]s --keep-tag-revisions=3 --keep-younger-than=60m --confirm
 
-	  # To exclude an imagestream from pruning you can specify a regular expression
-	  # which will be applied to the '<namespace>/<name>:<tag>' string.
-	  %[1]s %[2]s --exclude-imagestreamtag='^myproject/hello-openshift:.*$'
-
 	  # See, what the prune command would delete if we're interested in removing images
 	  # exceeding currently set limit ranges ('openshift.io/Image')
 	  %[1]s %[2]s --prune-over-size-limit
@@ -113,17 +108,15 @@ var (
 
 // PruneImagesOptions holds all the required options for pruning images.
 type PruneImagesOptions struct {
-	Confirm                   bool
-	KeepYoungerThan           *time.Duration
-	KeepTagRevisions          *int
-	PruneOverSizeLimit        *bool
-	AllImages                 *bool
-	CABundle                  string
-	RegistryUrlOverride       string
-	Namespace                 string
-	ForceInsecure             bool
-	ExcludeImageStreamTag     string
-	ExcludeImageStreamTagFile string
+	Confirm             bool
+	KeepYoungerThan     *time.Duration
+	KeepTagRevisions    *int
+	PruneOverSizeLimit  *bool
+	AllImages           *bool
+	CABundle            string
+	RegistryUrlOverride string
+	Namespace           string
+	ForceInsecure       bool
 
 	ClientConfig    *restclient.Config
 	AppsClient      appsclient.AppsInterface
@@ -165,8 +158,6 @@ func NewCmdPruneImages(f *clientcmd.Factory, parentName, name string, out io.Wri
 	cmd.Flags().BoolVar(opts.AllImages, "all", *opts.AllImages, "Include images that were imported from external registries as candidates for pruning.  If pruned, all the mirrored objects associated with them will also be removed from the integrated registry.")
 	cmd.Flags().DurationVar(opts.KeepYoungerThan, "keep-younger-than", *opts.KeepYoungerThan, "Specify the minimum age of an image and its referrers for it to be considered a candidate for pruning.")
 	cmd.Flags().IntVar(opts.KeepTagRevisions, "keep-tag-revisions", *opts.KeepTagRevisions, "Specify the number of image revisions for a tag in an image stream that will be preserved.")
-	cmd.Flags().StringVar(&opts.ExcludeImageStreamTag, "exclude-imagestreamtag", "", "The regular expression matching ImageStreamTags excluded from pruning.")
-	cmd.Flags().StringVar(&opts.ExcludeImageStreamTagFile, "exclude-imagestreamtag-file", "", "The filename that contains the regular expressions matching ImageStreamTags excluded from pruning.")
 	cmd.Flags().BoolVar(opts.PruneOverSizeLimit, "prune-over-size-limit", *opts.PruneOverSizeLimit, "Specify if images which are exceeding LimitRanges (see 'openshift.io/Image'), specified in the same namespace, should be considered for pruning. This flag cannot be combined with --keep-younger-than nor --keep-tag-revisions.")
 	cmd.Flags().StringVar(&opts.CABundle, "certificate-authority", opts.CABundle, "The path to a certificate authority bundle to use when communicating with the managed Docker registries. Defaults to the certificate authority data from the current user's config file. It cannot be used together with --force-insecure.")
 	cmd.Flags().StringVar(&opts.RegistryUrlOverride, "registry-url", opts.RegistryUrlOverride, "The address to use when contacting the registry, instead of using the default value. This is useful if you can't resolve or reach the registry (e.g.; the default is a cluster-internal URL) but you do have an alternative route that works. Particular transport protocol can be enforced using '<scheme>://' prefix.")
@@ -396,19 +387,6 @@ func (o PruneImagesOptions) Run() error {
 	if o.Namespace != metav1.NamespaceAll {
 		options.Namespace = o.Namespace
 	}
-	if len(o.ExcludeImageStreamTagFile) > 0 {
-		options.ExcludeImageStreamTagPatterns, err = parsePatternsFile(o.ExcludeImageStreamTagFile)
-		if err != nil {
-			return err
-		}
-	}
-	if len(o.ExcludeImageStreamTag) > 0 {
-		pattern, err := regexp.Compile(o.ExcludeImageStreamTag)
-		if err != nil {
-			return fmt.Errorf("bad regular expression %q: %v", o.ExcludeImageStreamTag, err)
-		}
-		options.ExcludeImageStreamTagPatterns = append(options.ExcludeImageStreamTagPatterns, pattern)
-	}
 	pruner, errs := prune.NewPruner(options)
 	if errs != nil {
 		o.printGraphBuildErrors(errs)
@@ -792,41 +770,3 @@ func getClientAndMasterVersions(client discovery.DiscoveryInterface, timeout tim
 
 	return
 }
-
-func parsePatternsFile(filename string) ([]*regexp.Regexp, error) {
-	file, err := os.Open(filename)
-	if err != nil {
-		return nil, err
-	}
-	defer file.Close()
-
-	var (
-		patterns  []*regexp.Regexp
-		line      string
-		readerErr error
-	)
-
-	reader := bufio.NewReader(file)
-	for readerErr == nil {
-		line, readerErr = reader.ReadString('\n')
-
-		if readerErr != nil && readerErr != io.EOF {
-			return nil, readerErr
-		}
-
-		line = strings.TrimSuffix(line, "\n")
-
-		if len(line) == 0 {
-			continue
-		}
-
-		pattern, err := regexp.Compile(line)
-		if err != nil {
-			return nil, fmt.Errorf("%s: bad regular expression %q: %v", filename, line, err)
-		}
-
-		patterns = append(patterns, pattern)
-	}
-
-	return patterns, nil
-}

test/extended/images/prune.go

@@ -103,7 +103,7 @@ var _ = g.Describe("[Feature:ImagePrune][registry][Serial] Image prune", func()
 			}
 		})
 
-		g.It("should prune both internally managed and external images", func() { testPruneAllImages(oc, true, 2, false) })
+		g.It("should prune both internally managed and external images", func() { testPruneAllImages(oc, true, 2) })
 	})
 
 	g.Describe("with --all=false flag", func() {
@@ -122,26 +122,7 @@ var _ = g.Describe("[Feature:ImagePrune][registry][Serial] Image prune", func()
 			}
 		})
 
-		g.It("should prune only internally managed images", func() { testPruneAllImages(oc, false, 2, false) })
-	})
-
-	g.Describe("with --all flag, but exclude external image", func() {
-		g.JustBeforeEach(func() {
-			if !*originalAcceptSchema2 {
-				g.By("ensure the registry accepts schema 2")
-				err := registryutil.EnsureRegistryAcceptsSchema2(oc, true)
-				o.Expect(err).NotTo(o.HaveOccurred())
-			}
-		})
-
-		g.AfterEach(func() {
-			if !*originalAcceptSchema2 {
-				err := registryutil.EnsureRegistryAcceptsSchema2(oc, false)
-				o.Expect(err).NotTo(o.HaveOccurred())
-			}
-		})
-
-		g.It("should prune only internally managed images and ignore external images", func() { testPruneAllImages(oc, false, 2, true) })
+		g.It("should prune only internally managed images", func() { testPruneAllImages(oc, false, 2) })
 	})
 })
 
@@ -259,7 +240,7 @@ func testPruneImages(oc *exutil.CLI, schemaVersion int) {
 	o.Expect(imgPrune.DockerImageMetadata.Size <= keepSize-confirmSize).To(o.BeTrue())
 }
 
-func testPruneAllImages(oc *exutil.CLI, setAllImagesToFalse bool, schemaVersion int, excludeExternalImage bool) {
+func testPruneAllImages(oc *exutil.CLI, setAllImagesToFalse bool, schemaVersion int) {
 	isName := fmt.Sprintf("prune-schema%d-all-images-%t", schemaVersion, setAllImagesToFalse)
 	repository := oc.Namespace() + "/" + isName
 
@@ -301,14 +282,14 @@ func testPruneAllImages(oc *exutil.CLI, setAllImagesToFalse bool, schemaVersion
 			o.Expect(inRepository).To(o.Equal(dryRun))
 		}
 
-		if setAllImagesToFalse || excludeExternalImage {
+		if setAllImagesToFalse {
 			o.Expect(output).NotTo(o.ContainSubstring(externalImage.Name))
 		} else {
 			o.Expect(output).To(o.ContainSubstring(externalImage.Name))
 		}
 
 		for _, layer := range externalImage.DockerImageLayers {
-			if setAllImagesToFalse || excludeExternalImage {
+			if setAllImagesToFalse {
 				o.Expect(output).NotTo(o.ContainSubstring(layer.Name))
 			} else {
 				o.Expect(output).To(o.ContainSubstring(layer.Name))
@@ -319,7 +300,7 @@ func testPruneAllImages(oc *exutil.CLI, setAllImagesToFalse bool, schemaVersion
 			}
 			globally, inRepository, err := IsBlobStoredInRegistry(oc, digest.Digest(layer.Name), repository)
 			o.Expect(err).NotTo(o.HaveOccurred())
-			o.Expect(globally).To(o.Equal(dryRun || setAllImagesToFalse || excludeExternalImage))
+			o.Expect(globally).To(o.Equal(dryRun || setAllImagesToFalse))
 			// mirrored blobs are not linked into any repository/_layers directory
 			o.Expect(inRepository).To(o.BeFalse())
 		}
@@ -330,10 +311,6 @@ func testPruneAllImages(oc *exutil.CLI, setAllImagesToFalse bool, schemaVersion
 		args = append(args, "--all=false")
 	}
 
-	if excludeExternalImage {
-		args = append(args, "--exclude-imagestreamtag=/origin-release:latest")
-	}
-
 	g.By(fmt.Sprintf("dry-running oc adm %s", strings.Join(args, " ")))
 	output, err := oc.WithoutNamespace().Run("adm").Args(args...).Output()