From 7c72f2e2cab83ea6601a8c973290ad9de308b03b Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Tue, 22 Aug 2017 13:23:33 +0200 Subject: [PATCH] Retry image stream updates when pruning images --- pkg/image/prune/prune.go | 71 +++++++++++++++++++++-------------- pkg/image/prune/prune_test.go | 6 ++- pkg/oc/admin/prune/images.go | 14 ++++--- 3 files changed, 56 insertions(+), 35 deletions(-) diff --git a/pkg/image/prune/prune.go b/pkg/image/prune/prune.go index 28af53bd3379..1291673145f3 100644 --- a/pkg/image/prune/prune.go +++ b/pkg/image/prune/prune.go @@ -20,6 +20,7 @@ import ( kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" kapi "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/client/retry" "github.com/openshift/origin/pkg/api/graph" kubegraph "github.com/openshift/origin/pkg/api/kubegraph/nodes" @@ -71,9 +72,11 @@ type ImageDeleter interface { // ImageStreamDeleter knows how to remove an image reference from an image stream. type ImageStreamDeleter interface { - // DeleteImageStream removes all references to the image from the image + // GetImageStream returns a fresh copy of an image stream. + GetImageStream(stream *imageapi.ImageStream) (*imageapi.ImageStream, error) + // UpdateImageStream removes all references to the image from the image // stream's status.tags. The updated image stream is returned. - DeleteImageStream(stream *imageapi.ImageStream, image *imageapi.Image, updatedTags []string) (*imageapi.ImageStream, error) + UpdateImageStream(stream *imageapi.ImageStream, image *imageapi.Image, updatedTags []string) (*imageapi.ImageStream, error) } // BlobDeleter knows how to delete a blob from the Docker registry. @@ -671,7 +674,7 @@ func calculatePrunableImageComponents(g graph.Graph) []*imagegraph.ImageComponen } // pruneStreams removes references from all image streams' status.tags entries -// to prunable images, invoking streamPruner.DeleteImageStream for each updated +// to prunable images, invoking streamPruner.UpdateImageStream for each updated // stream. func pruneStreams(g graph.Graph, imageNodes []*imagegraph.ImageNode, streamPruner ImageStreamDeleter) []error { errs := []error{} @@ -684,43 +687,49 @@ func pruneStreams(g graph.Graph, imageNodes []*imagegraph.ImageNode, streamPrune continue } - stream := streamNode.ImageStream - updatedTags := sets.NewString() + if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + stream, err := streamPruner.GetImageStream(streamNode.ImageStream) + if err != nil { + return err + } + updatedTags := sets.NewString() - glog.V(4).Infof("Checking if ImageStream %s has references to image %s in status.tags", getName(stream), imageNode.Image.Name) + glog.V(4).Infof("Checking if ImageStream %s has references to image %s in status.tags", getName(stream), imageNode.Image.Name) - for tag, history := range stream.Status.Tags { - glog.V(4).Infof("Checking tag %q", tag) + for tag, history := range stream.Status.Tags { + glog.V(4).Infof("Checking tag %q", tag) - newHistory := imageapi.TagEventList{} + newHistory := imageapi.TagEventList{} - for i, tagEvent := range history.Items { - glog.V(4).Infof("Checking tag event %d with image %q", i, tagEvent.Image) + for i, tagEvent := range history.Items { + glog.V(4).Infof("Checking tag event %d with image %q", i, tagEvent.Image) - if tagEvent.Image != imageNode.Image.Name { - glog.V(4).Infof("Tag event doesn't match deleted image - keeping") - newHistory.Items = append(newHistory.Items, tagEvent) + if tagEvent.Image != imageNode.Image.Name { + glog.V(4).Infof("Tag event doesn't match deleted image - keeping") + newHistory.Items = append(newHistory.Items, tagEvent) + } else { + glog.V(4).Infof("Tag event matches deleted image - removing reference") + updatedTags.Insert(tag) + } + } + if len(newHistory.Items) == 0 { + glog.V(4).Infof("Removing tag %q from status.tags of ImageStream %s", tag, getName(stream)) + delete(stream.Status.Tags, tag) } else { - glog.V(4).Infof("Tag event matches deleted image - removing reference") - updatedTags.Insert(tag) + stream.Status.Tags[tag] = newHistory } } - if len(newHistory.Items) == 0 { - glog.V(4).Infof("Removing tag %q from status.tags of ImageStream %s", tag, getName(stream)) - delete(stream.Status.Tags, tag) - } else { - stream.Status.Tags[tag] = newHistory - } - } - updatedStream, err := streamPruner.DeleteImageStream(stream, imageNode.Image, updatedTags.List()) - if err != nil { + updatedStream, err := streamPruner.UpdateImageStream(stream, imageNode.Image, updatedTags.List()) + if err == nil { + streamNode.ImageStream = updatedStream + } + return err + }); err != nil { errs = append(errs, fmt.Errorf("error removing image %s from stream %s: %v", - imageNode.Image.Name, getName(stream), err)) + imageNode.Image.Name, getName(streamNode.ImageStream), err)) continue } - - streamNode.ImageStream = updatedStream } } glog.V(4).Infof("Done removing pruned image references from streams") @@ -922,7 +931,11 @@ func NewImageStreamDeleter(streams client.ImageStreamsNamespacer) ImageStreamDel } } -func (p *imageStreamDeleter) DeleteImageStream(stream *imageapi.ImageStream, image *imageapi.Image, updatedTags []string) (*imageapi.ImageStream, error) { +func (p *imageStreamDeleter) GetImageStream(stream *imageapi.ImageStream) (*imageapi.ImageStream, error) { + return p.streams.ImageStreams(stream.Namespace).Get(stream.Name, metav1.GetOptions{}) +} + +func (p *imageStreamDeleter) UpdateImageStream(stream *imageapi.ImageStream, image *imageapi.Image, updatedTags []string) (*imageapi.ImageStream, error) { glog.V(4).Infof("Updating ImageStream %s", getName(stream)) is, err := p.streams.ImageStreams(stream.Namespace).UpdateStatus(stream) if err == nil { diff --git a/pkg/image/prune/prune_test.go b/pkg/image/prune/prune_test.go index 8398741de895..a2e3a11526ba 100644 --- a/pkg/image/prune/prune_test.go +++ b/pkg/image/prune/prune_test.go @@ -365,7 +365,11 @@ type fakeImageStreamDeleter struct { var _ ImageStreamDeleter = &fakeImageStreamDeleter{} -func (p *fakeImageStreamDeleter) DeleteImageStream(stream *imageapi.ImageStream, image *imageapi.Image, updatedTags []string) (*imageapi.ImageStream, error) { +func (p *fakeImageStreamDeleter) GetImageStream(stream *imageapi.ImageStream) (*imageapi.ImageStream, error) { + return stream, p.err +} + +func (p *fakeImageStreamDeleter) UpdateImageStream(stream *imageapi.ImageStream, image *imageapi.Image, updatedTags []string) (*imageapi.ImageStream, error) { p.invocations.Insert(fmt.Sprintf("%s/%s|%s", stream.Namespace, stream.Name, image.Name)) return stream, p.err } diff --git a/pkg/oc/admin/prune/images.go b/pkg/oc/admin/prune/images.go index 4b6c60d2383b..7234d27ef10f 100644 --- a/pkg/oc/admin/prune/images.go +++ b/pkg/oc/admin/prune/images.go @@ -58,9 +58,9 @@ var ( Insecure connection is allowed in the following cases unless certificate-authority is specified: - 1. --force-insecure is given - 2. provided registry-url is prefixed with http:// - 3. registry url is a private or link-local address + 1. --force-insecure is given + 2. provided registry-url is prefixed with http:// + 3. registry url is a private or link-local address 4. user's config allows for insecure connection (the user logged in to the cluster with --insecure-skip-tls-verify or allowed for insecure connection)`) @@ -361,7 +361,11 @@ type describingImageStreamDeleter struct { var _ prune.ImageStreamDeleter = &describingImageStreamDeleter{} -func (p *describingImageStreamDeleter) DeleteImageStream(stream *imageapi.ImageStream, image *imageapi.Image, updatedTags []string) (*imageapi.ImageStream, error) { +func (p *describingImageStreamDeleter) GetImageStream(stream *imageapi.ImageStream) (*imageapi.ImageStream, error) { + return stream, nil +} + +func (p *describingImageStreamDeleter) UpdateImageStream(stream *imageapi.ImageStream, image *imageapi.Image, updatedTags []string) (*imageapi.ImageStream, error) { if !p.headerPrinted { p.headerPrinted = true fmt.Fprintln(p.w, "Deleting references from image streams to images ...") @@ -374,7 +378,7 @@ func (p *describingImageStreamDeleter) DeleteImageStream(stream *imageapi.ImageS return stream, nil } - updatedStream, err := p.delegate.DeleteImageStream(stream, image, updatedTags) + updatedStream, err := p.delegate.UpdateImageStream(stream, image, updatedTags) if err != nil { fmt.Fprintf(os.Stderr, "error updating image stream %s/%s to remove references to image %s: %v\n", stream.Namespace, stream.Name, image.Name, err) }