Skip to content

Commit 7785c02

Browse files
author
Michal Minář
committed
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. Do not default to insecure connection when the --registry-url is empty. Move registry client initialization just before the start of the pruner - so we can precisely determine whether to allow for insecure fall-back based on collected images and image streams. Move ping() outside of pruner. Instead, determine the registry URL before the pruner starts and assume it won't change during the run. Signed-off-by: Michal Minář <[email protected]>
1 parent 42b4710 commit 7785c02

File tree

7 files changed

+889
-509
lines changed

7 files changed

+889
-509
lines changed

pkg/cmd/admin/prune/images.go

Lines changed: 86 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,13 @@ var (
5353
authority other than the one present in current user's config, you may need to specify it
5454
using --certificate-authority flag.
5555
56-
Insecure connection is allowed in following cases unless certificate-authority is specified:
57-
1. --force-insecure is given
56+
Insecure connection is allowed if certificate-authority is not specified and one of the following
57+
conditions holds true:
58+
59+
1. --force-insecure is given
5860
2. user's config allows for insecure connection (the user logged in to the cluster with
59-
--insecure-skip-tls-verify or allowed for insecure connection)
60-
3. registry url is not given or it's a private/link-local address`)
61+
--insecure-skip-tls-verify or allowed for insecure connection)
62+
3. registry url is a private or link-local address`)
6163

6264
imagesExample = templates.Examples(`
6365
# See, what the prune command would delete if only images more than an hour old and obsoleted
@@ -93,11 +95,10 @@ type PruneImagesOptions struct {
9395
Namespace string
9496
ForceInsecure bool
9597

96-
OSClient client.Interface
97-
KClient kclientset.Interface
98-
RegistryClient *http.Client
99-
Out io.Writer
100-
Insecure bool
98+
ClientConfig *restclient.Config
99+
OSClient client.Interface
100+
KubeClient kclientset.Interface
101+
Out io.Writer
101102
}
102103

103104
// NewCmdPruneImages implements the OpenShift cli prune images command.
@@ -169,18 +170,14 @@ func (o *PruneImagesOptions) Complete(f *clientcmd.Factory, cmd *cobra.Command,
169170
if err != nil {
170171
return err
171172
}
173+
o.ClientConfig = clientConfig
172174

173-
o.Insecure = o.ForceInsecure
174-
if !o.Insecure && len(o.CABundle) == 0 {
175-
o.Insecure = clientConfig.TLSClientConfig.Insecure || len(o.RegistryUrlOverride) == 0 || netutils.IsPrivateAddress(o.RegistryUrlOverride)
176-
}
177-
osClient, kClient, registryClient, err := getClients(f, o.CABundle, o.Insecure)
175+
osClient, kubeClient, err := getClients(f)
178176
if err != nil {
179177
return err
180178
}
181179
o.OSClient = osClient
182-
o.KClient = kClient
183-
o.RegistryClient = registryClient
180+
o.KubeClient = kubeClient
184181

185182
return nil
186183
}
@@ -217,12 +214,12 @@ func (o PruneImagesOptions) Run() error {
217214
return err
218215
}
219216

220-
allPods, err := o.KClient.Core().Pods(o.Namespace).List(metav1.ListOptions{})
217+
allPods, err := o.KubeClient.Core().Pods(o.Namespace).List(metav1.ListOptions{})
221218
if err != nil {
222219
return err
223220
}
224221

225-
allRCs, err := o.KClient.Core().ReplicationControllers(o.Namespace).List(metav1.ListOptions{})
222+
allRCs, err := o.KubeClient.Core().ReplicationControllers(o.Namespace).List(metav1.ListOptions{})
226223
if err != nil {
227224
return err
228225
}
@@ -246,7 +243,7 @@ func (o PruneImagesOptions) Run() error {
246243
return err
247244
}
248245

249-
limitRangesList, err := o.KClient.Core().LimitRanges(o.Namespace).List(metav1.ListOptions{})
246+
limitRangesList, err := o.KubeClient.Core().LimitRanges(o.Namespace).List(metav1.ListOptions{})
250247
if err != nil {
251248
return err
252249
}
@@ -261,6 +258,42 @@ func (o PruneImagesOptions) Run() error {
261258
limitRangesMap[limit.Namespace] = limits
262259
}
263260

261+
var (
262+
registryHost = o.RegistryUrlOverride
263+
registryClient *http.Client
264+
registryPinger prune.RegistryPinger
265+
)
266+
267+
if o.Confirm {
268+
if len(registryHost) == 0 {
269+
registryHost, err = prune.DetermineRegistryHost(allImages, allStreams)
270+
if err != nil {
271+
return fmt.Errorf("unable to determine registry: %v", err)
272+
}
273+
}
274+
275+
insecure := o.ForceInsecure
276+
if !insecure && len(o.CABundle) == 0 {
277+
insecure = o.ClientConfig.TLSClientConfig.Insecure || netutils.IsPrivateAddress(registryHost)
278+
}
279+
280+
registryClient, err = getRegistryClient(o.ClientConfig, o.CABundle, insecure)
281+
if err != nil {
282+
return err
283+
}
284+
registryPinger = &prune.DefaultRegistryPinger{
285+
Client: registryClient,
286+
Insecure: insecure,
287+
}
288+
} else {
289+
registryPinger = &prune.DryRunRegistryPinger{}
290+
}
291+
292+
registryURL, err := registryPinger.Ping(registryHost)
293+
if err != nil {
294+
return fmt.Errorf("error communicating with registry %s: %v", registryHost, err)
295+
}
296+
264297
options := prune.PrunerOptions{
265298
KeepYoungerThan: o.KeepYoungerThan,
266299
KeepTagRevisions: o.KeepTagRevisions,
@@ -275,9 +308,8 @@ func (o PruneImagesOptions) Run() error {
275308
DCs: allDCs,
276309
LimitRanges: limitRangesMap,
277310
DryRun: o.Confirm == false,
278-
RegistryClient: o.RegistryClient,
279-
RegistryURL: o.RegistryUrlOverride,
280-
Insecure: o.Insecure,
311+
RegistryClient: registryClient,
312+
RegistryURL: registryURL,
281313
}
282314
if o.Namespace != metav1.NamespaceAll {
283315
options.Namespace = o.Namespace
@@ -378,7 +410,7 @@ type describingLayerLinkDeleter struct {
378410

379411
var _ prune.LayerLinkDeleter = &describingLayerLinkDeleter{}
380412

381-
func (p *describingLayerLinkDeleter) DeleteLayerLink(registryClient *http.Client, registryURL, repo, name string) error {
413+
func (p *describingLayerLinkDeleter) DeleteLayerLink(registryClient *http.Client, registryURL *url.URL, repo, name string) error {
382414
if !p.headerPrinted {
383415
p.headerPrinted = true
384416
fmt.Fprintln(p.w, "\nDeleting registry repository layer links ...")
@@ -409,7 +441,7 @@ type describingBlobDeleter struct {
409441

410442
var _ prune.BlobDeleter = &describingBlobDeleter{}
411443

412-
func (p *describingBlobDeleter) DeleteBlob(registryClient *http.Client, registryURL, layer string) error {
444+
func (p *describingBlobDeleter) DeleteBlob(registryClient *http.Client, registryURL *url.URL, layer string) error {
413445
if !p.headerPrinted {
414446
p.headerPrinted = true
415447
fmt.Fprintln(p.w, "\nDeleting registry layer blobs ...")
@@ -441,7 +473,7 @@ type describingManifestDeleter struct {
441473

442474
var _ prune.ManifestDeleter = &describingManifestDeleter{}
443475

444-
func (p *describingManifestDeleter) DeleteManifest(registryClient *http.Client, registryURL, repo, manifest string) error {
476+
func (p *describingManifestDeleter) DeleteManifest(registryClient *http.Client, registryURL *url.URL, repo, manifest string) error {
445477
if !p.headerPrinted {
446478
p.headerPrinted = true
447479
fmt.Fprintln(p.w, "\nDeleting registry repository manifest data ...")
@@ -456,46 +488,48 @@ func (p *describingManifestDeleter) DeleteManifest(registryClient *http.Client,
456488

457489
err := p.delegate.DeleteManifest(registryClient, registryURL, repo, manifest)
458490
if err != nil {
459-
fmt.Fprintf(os.Stderr, "error deleting data for repository %s image manifest %s from the registry: %v\n", repo, manifest, err)
491+
fmt.Fprintf(os.Stderr, "error deleting manifest %s from repository %s: %v\n", manifest, repo, err)
460492
}
461493

462494
return err
463495
}
464496

465-
// getClients returns a Kube client, OpenShift client, and registry client. Note that
466-
// registryCABundle and registryInsecure=true are mutually exclusive. If registryInsecure=true is
467-
// specified, the ca bundle is ignored.
468-
func getClients(f *clientcmd.Factory, registryCABundle string, registryInsecure bool) (*client.Client, kclientset.Interface, *http.Client, error) {
497+
// getClients returns a OpenShift client and Kube client.
498+
func getClients(f *clientcmd.Factory) (*client.Client, kclientset.Interface, error) {
469499
clientConfig, err := f.ClientConfig()
470500
if err != nil {
471-
return nil, nil, nil, err
501+
return nil, nil, err
502+
}
503+
504+
if len(clientConfig.BearerToken) == 0 {
505+
return nil, nil, noTokenError
506+
}
507+
508+
osClient, kubeClient, err := f.Clients()
509+
if err != nil {
510+
return nil, nil, err
472511
}
512+
return osClient, kubeClient, err
513+
}
473514

515+
// getRegistryClient returns a registry client. Note that registryCABundle and registryInsecure=true are
516+
// mutually exclusive. If registryInsecure=true is specified, the ca bundle is ignored.
517+
func getRegistryClient(clientConfig *restclient.Config, registryCABundle string, registryInsecure bool) (*http.Client, error) {
474518
var (
475-
token string
476-
osClient *client.Client
477-
kClient kclientset.Interface
478-
registryClient *http.Client
519+
err error
520+
cadata []byte
521+
registryCABundleIncluded = false
522+
token = clientConfig.BearerToken
479523
)
480524

481-
switch {
482-
case len(clientConfig.BearerToken) > 0:
483-
osClient, kClient, err = f.Clients()
484-
if err != nil {
485-
return nil, nil, nil, err
486-
}
487-
token = clientConfig.BearerToken
488-
default:
489-
err = errors.New("you must use a client config with a token")
490-
return nil, nil, nil, err
525+
if len(token) == 0 {
526+
return nil, noTokenError
491527
}
492528

493-
cadata := []byte{}
494-
registryCABundleIncluded := false
495529
if len(registryCABundle) > 0 {
496530
cadata, err = ioutil.ReadFile(registryCABundle)
497531
if err != nil {
498-
return nil, nil, nil, fmt.Errorf("failed to read registry ca bundle: %v", err)
532+
return nil, fmt.Errorf("failed to read registry ca bundle: %v", err)
499533
}
500534
}
501535

@@ -531,7 +565,7 @@ func getClients(f *clientcmd.Factory, registryCABundle string, registryInsecure
531565

532566
tlsConfig, err := restclient.TLSConfigFor(&registryClientConfig)
533567
if err != nil {
534-
return nil, nil, nil, err
568+
return nil, err
535569
}
536570

537571
// Add the CA bundle to the client config's CA roots if provided and we haven't done that already.
@@ -549,12 +583,10 @@ func getClients(f *clientcmd.Factory, registryCABundle string, registryInsecure
549583

550584
wrappedTransport, err := restclient.HTTPWrappersForConfig(&registryClientConfig, transport)
551585
if err != nil {
552-
return nil, nil, nil, err
586+
return nil, err
553587
}
554588

555-
registryClient = &http.Client{
589+
return &http.Client{
556590
Transport: wrappedTransport,
557-
}
558-
559-
return osClient, kClient, registryClient, nil
591+
}, nil
560592
}

pkg/cmd/admin/prune/images_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ func TestImagePruneNamespaced(t *testing.T) {
1515
opts := &PruneImagesOptions{
1616
Namespace: "foo",
1717

18-
OSClient: osFake,
19-
KClient: kFake,
20-
Out: ioutil.Discard,
18+
OSClient: osFake,
19+
KubeClient: kFake,
20+
Out: ioutil.Discard,
2121
}
2222

2323
if err := opts.Run(); err != nil {

pkg/image/prune/doc.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// Package prune contains logic for pruning images and interoperating with the integrated Docker registry.
2+
package prune

0 commit comments

Comments
 (0)