Image pruner: Determine protocol just once

Determine the registry protocol once. Do not change to other protocol
during the run. This will produce nicer output without unrelated
protocol fallback errors.

Signed-off-by: Michal Minář <[email protected]>

pkg/cmd/admin/prune/images.go

@@ -276,7 +276,7 @@ func (o PruneImagesOptions) Run() error {
 		LimitRanges:        limitRangesMap,
 		DryRun:             o.Confirm == false,
 		RegistryClient:     o.RegistryClient,
-		RegistryURL:        o.RegistryUrlOverride,
+		RegistryHost:       o.RegistryUrlOverride,
 		Insecure:           o.Insecure,
 	}
 	if o.Namespace != metav1.NamespaceAll {

pkg/image/prune/prune.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/url"
 	"reflect"
 	"sort"
 	"strings"
@@ -112,7 +113,6 @@ type PrunerOptions struct {
 	// will be considered as candidates for pruning.
 	PruneOverSizeLimit *bool
 	// AllImages considers all images for pruning, not just those pushed directly to the registry.
-	// Requires RegistryURL be set.
 	AllImages *bool
 	// Namespace to be pruned, if specified it should never remove Images.
 	Namespace string
@@ -142,8 +142,8 @@ type PrunerOptions struct {
 	DryRun bool
 	// RegistryClient is the http.Client to use when contacting the registry.
 	RegistryClient *http.Client
-	// RegistryURL is the URL for the registry.
-	RegistryURL string
+	// RegistryHost is the registry's address.
+	RegistryHost string
 	// Allow a fallback to insecure transport when contacting the registry.
 	Insecure bool
 }
@@ -163,15 +163,16 @@ type pruner struct {
 	algorithm      pruneAlgorithm
 	registryPinger registryPinger
 	registryClient *http.Client
-	registryURL    string
+	registryHost   string
 }
 
 var _ Pruner = &pruner{}
 
 // registryPinger performs a health check against a registry.
 type registryPinger interface {
-	// ping performs a health check against registry.
-	ping(registry string) error
+	// ping performs a health check against registry. It returns registry url qualified with schema unless an
+	// error occurs.
+	ping(registry string) (string, error)
 }
 
 // defaultRegistryPinger implements registryPinger.
@@ -180,49 +181,68 @@ type defaultRegistryPinger struct {
 	insecure bool
 }
 
-func (drp *defaultRegistryPinger) ping(registry string) error {
-	healthCheck := func(proto, registry string) error {
-		// TODO: `/healthz` route is deprecated by `/`; remove it in future versions
-		healthResponse, err := drp.client.Get(fmt.Sprintf("%s://%s/healthz", proto, registry))
-		if err != nil {
-			return err
-		}
-		defer healthResponse.Body.Close()
+func (drp *defaultRegistryPinger) ping(registry string) (string, error) {
+	var (
+		registryURL *url.URL
+		err         error
+	)
 
-		if healthResponse.StatusCode != http.StatusOK {
-			return fmt.Errorf("unexpected status: %s", healthResponse.Status)
-		}
+pathLoop:
+	// first try the new default / path, then fall-back to the obsolete /healthz endpoint
+	for _, path := range []string{"/", "/healthz"} {
+		registryURL, err = tryProtocolsWithRegistryURL(registry, drp.insecure, func(u url.URL) error {
+			u.Path = path
+			healthResponse, err := drp.client.Get(u.String())
+			if err != nil {
+				return err
+			}
+			defer healthResponse.Body.Close()
 
-		return nil
-	}
+			if healthResponse.StatusCode != http.StatusOK {
+				return &retryPath{err: fmt.Errorf("unexpected status: %s", healthResponse.Status)}
+			}
 
-	var errs []error
-	protos := make([]string, 0, 2)
-	protos = append(protos, "https")
-	if drp.insecure || netutils.IsPrivateAddress(registry) {
-		protos = append(protos, "http")
-	}
-	for _, proto := range protos {
-		glog.V(4).Infof("Trying %s for %s", proto, registry)
-		err := healthCheck(proto, registry)
-		if err == nil {
 			return nil
+		})
+
+		// determine whether to retry with another endpoint
+		switch t := err.(type) {
+		case *retryPath:
+			// return the nested error if this is the last ping attempt
+			err = t.err
+			continue pathLoop
+		case kerrors.Aggregate:
+			// if any aggregated error indicates a possible retry, do it
+			for _, err := range t.Errors() {
+				if _, ok := err.(*retryPath); ok {
+					continue pathLoop
+				}
+			}
 		}
-		errs = append(errs, err)
-		glog.V(4).Infof("Error with %s for %s: %v", proto, registry, err)
+
+		break
 	}
 
-	return kerrors.NewAggregate(errs)
+	if err != nil {
+		return registry, err
+	}
+
+	return registryURL.String(), nil
 }
 
 // dryRunRegistryPinger implements registryPinger.
 type dryRunRegistryPinger struct {
 }
 
-func (*dryRunRegistryPinger) ping(registry string) error {
-	return nil
+func (*dryRunRegistryPinger) ping(registry string) (string, error) {
+	return "https://" + registry, nil
 }
 
+// retryPath is an error indicating that another connection attempt may be retried with a different path
+type retryPath struct{ err error }
+
+func (rp *retryPath) Error() string { return rp.err.Error() }
+
 // NewPruner creates a Pruner.
 //
 // Images younger than keepYoungerThan and images referenced by image streams
@@ -311,7 +331,7 @@ func NewPruner(options PrunerOptions) Pruner {
 		algorithm:      algorithm,
 		registryPinger: rp,
 		registryClient: options.RegistryClient,
-		registryURL:    options.RegistryURL,
+		registryHost:   options.RegistryHost,
 	}
 }
 
@@ -831,8 +851,8 @@ func (ba isByAge) Less(i, j int) bool {
 }
 
 func (p *pruner) determineRegistry(imageNodes []*imagegraph.ImageNode, isNodes []*imagegraph.ImageStreamNode) (string, error) {
-	if len(p.registryURL) > 0 {
-		return p.registryURL, nil
+	if len(p.registryHost) > 0 {
+		return p.registryHost, nil
 	}
 
 	var pullSpec string
@@ -896,14 +916,15 @@ func (p *pruner) Prune(
 		return nil
 	}
 
-	registryURL, err := p.determineRegistry(imageNodes, getImageStreamNodes(allNodes))
+	registryHost, err := p.determineRegistry(imageNodes, getImageStreamNodes(allNodes))
 	if err != nil {
 		return fmt.Errorf("unable to determine registry: %v", err)
 	}
-	glog.V(1).Infof("Using registry: %s", registryURL)
+	glog.V(1).Infof("Using registry: %s", registryHost)
 
-	if err := p.registryPinger.ping(registryURL); err != nil {
-		return fmt.Errorf("error communicating with registry %s: %v", registryURL, err)
+	registryURL, err := p.registryPinger.ping(registryHost)
+	if err != nil {
+		return fmt.Errorf("error communicating with registry %s: %v", registryHost, err)
 	}
 
 	prunableImageNodes, prunableImageIDs := calculatePrunableImages(p.g, imageNodes)
@@ -1077,63 +1098,42 @@ func (p *imageStreamDeleter) DeleteImageStream(stream *imageapi.ImageStream, ima
 // provided url. It attempts an https request first; if that fails, it fails
 // back to http.
 func deleteFromRegistry(registryClient *http.Client, url string) error {
-	deleteFunc := func(proto, url string) error {
-		req, err := http.NewRequest("DELETE", url, nil)
-		if err != nil {
-			return err
-		}
-
-		glog.V(4).Infof("Sending request to registry")
-		resp, err := registryClient.Do(req)
-		if err != nil {
-			if proto != "https" && strings.Contains(err.Error(), "malformed HTTP response") {
-				return fmt.Errorf("%v.\n* Are you trying to connect to a TLS-enabled registry without TLS?", err)
-			}
-			return err
-		}
-		defer resp.Body.Close()
+	req, err := http.NewRequest("DELETE", url, nil)
+	if err != nil {
+		return err
+	}
 
-		// TODO: investigate why we're getting non-existent layers, for now we're logging
-		// them out and continue working
-		if resp.StatusCode == http.StatusNotFound {
-			glog.Warningf("Unable to prune layer %s, returned %v", url, resp.Status)
-			return nil
-		}
-		// non-2xx/3xx response doesn't cause an error, so we need to check for it
-		// manually and return it to caller
-		if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusBadRequest {
-			return fmt.Errorf(resp.Status)
-		}
-		if resp.StatusCode != http.StatusNoContent && resp.StatusCode != http.StatusAccepted {
-			glog.V(1).Infof("Unexpected status code in response: %d", resp.StatusCode)
-			var response errcode.Errors
-			decoder := json.NewDecoder(resp.Body)
-			if err := decoder.Decode(&response); err != nil {
-				return err
-			}
-			glog.V(1).Infof("Response: %#v", response)
-			return &response
-		}
+	glog.V(4).Infof("Sending request to registry")
+	resp, err := registryClient.Do(req)
+	if err != nil {
+		return err
+	}
+	defer resp.Body.Close()
 
+	// TODO: investigate why we're getting non-existent layers, for now we're logging
+	// them out and continue working
+	if resp.StatusCode == http.StatusNotFound {
+		glog.Warningf("Unable to prune layer %s, returned %v", url, resp.Status)
 		return nil
 	}
 
-	var err error
-	for _, proto := range []string{"https", "http"} {
-		glog.V(4).Infof("Trying %s for %s", proto, url)
-		err = deleteFunc(proto, fmt.Sprintf("%s://%s", proto, url))
-		if err == nil {
-			return nil
-		}
+	// non-2xx/3xx response doesn't cause an error, so we need to check for it
+	// manually and return it to caller
+	if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusBadRequest {
+		return fmt.Errorf(resp.Status)
+	}
 
-		if _, ok := err.(*errcode.Errors); ok {
-			// we got a response back from the registry, so return it
+	if resp.StatusCode != http.StatusNoContent && resp.StatusCode != http.StatusAccepted {
+		glog.V(1).Infof("Unexpected status code in response: %d", resp.StatusCode)
+		var response errcode.Errors
+		decoder := json.NewDecoder(resp.Body)
+		if err := decoder.Decode(&response); err != nil {
 			return err
 		}
-
-		// we didn't get a success or a errcode.Errors response back from the registry
-		glog.V(4).Infof("Error with %s for %s: %v", proto, url, err)
+		glog.V(1).Infof("Response: %#v", response)
+		return &response
 	}
+
 	return err
 }
 
@@ -1190,3 +1190,54 @@ func getName(obj runtime.Object) string {
 	}
 	return fmt.Sprintf("%s/%s", accessor.GetNamespace(), accessor.GetName())
 }
+
+// tryProtocolsWithRegistryURL runs given action with different protocols until no error is returned. The
+// https protocol is the first attempt. If it fails and allowInsecure is true, http will be the next. Obtained
+// errors will be concatenated and returned.
+func tryProtocolsWithRegistryURL(registry string, allowInsecure bool, action func(registryURL url.URL) error) (*url.URL, error) {
+	var errs []error
+
+	if !strings.Contains(registry, "://") {
+		registry = "unset://" + registry
+	}
+	url, err := url.Parse(registry)
+	if err != nil {
+		return nil, err
+	}
+	var protos []string
+	switch {
+	case len(url.Scheme) > 0 && url.Scheme != "unset":
+		protos = []string{url.Scheme}
+	case allowInsecure || netutils.IsPrivateAddress(registry):
+		protos = []string{"https", "http"}
+	default:
+		protos = []string{"https"}
+	}
+	registry = url.Host
+
+	for _, proto := range protos {
+		glog.V(4).Infof("Trying protocol %s for the registry URL %s", proto, registry)
+		url.Scheme = proto
+		err := action(*url)
+		if err == nil {
+			return url, nil
+		}
+
+		if err != nil {
+			glog.V(4).Infof("Error with %s for %s: %v", proto, registry, err)
+		}
+
+		if _, ok := err.(*errcode.Errors); ok {
+			// we got a response back from the registry, so return it
+			return url, err
+		}
+		errs = append(errs, err)
+		if proto == "https" && strings.Contains(err.Error(), "server gave HTTP response to HTTPS client") && !allowInsecure {
+			errs = append(errs, fmt.Errorf("\n* Append --force-insecure if you really want to prune the registry using insecure connection."))
+		} else if proto == "http" && strings.Contains(err.Error(), "malformed HTTP response") {
+			errs = append(errs, fmt.Errorf("\n* Are you trying to connect to a TLS-enabled registry without TLS?"))
+		}
+	}
+
+	return nil, kerrors.NewAggregate(errs)
+}