Skip to content

Commit 72300b7

Browse files
author
Jan Wozniak
committed
oc import-image: remove legacy code using annotations
fixes #13214
1 parent 0658af0 commit 72300b7

File tree

2 files changed

+35
-147
lines changed

2 files changed

+35
-147
lines changed

hack/import-restrictions.json

-1
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,6 @@
507507
"github.com/openshift/origin/pkg/project/registry/projectrequest/delegated",
508508
"github.com/openshift/origin/pkg/project/util",
509509
"github.com/openshift/origin/pkg/quota/apis/quota",
510-
"github.com/openshift/origin/pkg/quota/util",
511510
"github.com/openshift/origin/pkg/route/apis/route",
512511
"github.com/openshift/origin/pkg/route/apis/route/validation",
513512
"github.com/openshift/origin/pkg/route/generator",

pkg/oc/cli/cmd/importimage.go

+35-146
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,11 @@ import (
44
"fmt"
55
"io"
66
"strings"
7-
"time"
87

98
"github.com/spf13/cobra"
109

1110
"k8s.io/apimachinery/pkg/api/errors"
1211
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13-
"k8s.io/apimachinery/pkg/fields"
14-
"k8s.io/apimachinery/pkg/watch"
1512
"k8s.io/kubernetes/pkg/api/legacyscheme"
1613
kapi "k8s.io/kubernetes/pkg/apis/core"
1714
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
@@ -24,7 +21,6 @@ import (
2421
imageclientinternal "github.com/openshift/origin/pkg/image/generated/internalclientset"
2522
imageclient "github.com/openshift/origin/pkg/image/generated/internalclientset/typed/image/internalversion"
2623
"github.com/openshift/origin/pkg/oc/cli/describe"
27-
quotautil "github.com/openshift/origin/pkg/quota/util"
2824
)
2925

3026
var (
@@ -171,53 +167,36 @@ func (o *ImportImageOptions) Run() error {
171167
return err
172168
}
173169

174-
// Attempt the new, direct import path
175170
result, err := o.imageClient.ImageStreamImports(isi.Namespace).Create(isi)
176-
err = TransformUnsupportedError(err)
177-
switch {
178-
case err == imageapi.ErrImageStreamImportUnsupported:
179-
case err != nil:
171+
if err != nil {
180172
return err
181-
default:
182-
if o.DryRun {
183-
if wasError(result) {
184-
fmt.Fprintf(o.errout, "The dry-run import completed with errors.\n\n")
185-
} else {
186-
fmt.Fprint(o.out, "The dry-run import completed successfully.\n\n")
187-
}
173+
}
174+
175+
if o.DryRun {
176+
if wasError(result) {
177+
fmt.Fprintf(o.errout, "The dry-run import completed with errors.\n\n")
188178
} else {
189-
if wasError(result) {
190-
fmt.Fprintf(o.errout, "The import completed with errors.\n\n")
191-
} else {
192-
fmt.Fprint(o.out, "The import completed successfully.\n\n")
193-
}
179+
fmt.Fprint(o.out, "The dry-run import completed successfully.\n\n")
194180
}
195-
196-
if result.Status.Import != nil {
197-
// TODO: dry-run doesn't return an image stream, so we have to display partial results
198-
info, err := describe.DescribeImageStream(result.Status.Import)
199-
if err != nil {
200-
return err
201-
}
202-
fmt.Fprintln(o.out, info)
181+
} else {
182+
if wasError(result) {
183+
fmt.Fprintf(o.errout, "The import completed with errors.\n\n")
184+
} else {
185+
fmt.Fprint(o.out, "The import completed successfully.\n\n")
203186
}
187+
}
204188

205-
if repo := result.Status.Repository; repo != nil {
206-
for _, image := range repo.Images {
207-
if image.Image != nil {
208-
info, err := describe.DescribeImage(image.Image, imageapi.JoinImageStreamTag(stream.Name, image.Tag))
209-
if err != nil {
210-
fmt.Fprintf(o.errout, "error: tag %s failed: %v\n", image.Tag, err)
211-
} else {
212-
fmt.Fprintln(o.out, info)
213-
}
214-
} else {
215-
fmt.Fprintf(o.errout, "error: repository tag %s failed: %v\n", image.Tag, image.Status.Message)
216-
}
217-
}
189+
if result.Status.Import != nil {
190+
// TODO: dry-run doesn't return an image stream, so we have to display partial results
191+
info, err := describe.DescribeImageStream(result.Status.Import)
192+
if err != nil {
193+
return err
218194
}
195+
fmt.Fprintln(o.out, info)
196+
}
219197

220-
for _, image := range result.Status.Images {
198+
if repo := result.Status.Repository; repo != nil {
199+
for _, image := range repo.Images {
221200
if image.Image != nil {
222201
info, err := describe.DescribeImage(image.Image, imageapi.JoinImageStreamTag(stream.Name, image.Tag))
223202
if err != nil {
@@ -226,54 +205,27 @@ func (o *ImportImageOptions) Run() error {
226205
fmt.Fprintln(o.out, info)
227206
}
228207
} else {
229-
fmt.Fprintf(o.errout, "error: tag %s failed: %v\n", image.Tag, image.Status.Message)
208+
fmt.Fprintf(o.errout, "error: repository tag %s failed: %v\n", image.Tag, image.Status.Message)
230209
}
231210
}
232-
233-
if r := result.Status.Repository; r != nil && len(r.AdditionalTags) > 0 {
234-
fmt.Fprintf(o.out, "\ninfo: The remote repository contained %d additional tags which were not imported: %s\n", len(r.AdditionalTags), strings.Join(r.AdditionalTags, ", "))
235-
}
236-
return nil
237-
}
238-
239-
// Legacy path, remove when support for older importers is removed
240-
delete(stream.Annotations, imageapi.DockerImageRepositoryCheckAnnotation)
241-
if o.Insecure != nil && *o.Insecure {
242-
if stream.Annotations == nil {
243-
stream.Annotations = make(map[string]string)
244-
}
245-
stream.Annotations[imageapi.InsecureRepositoryAnnotation] = "true"
246-
}
247-
248-
if stream.CreationTimestamp.IsZero() {
249-
stream, err = o.isClient.Create(stream)
250-
} else {
251-
stream, err = o.isClient.Update(stream)
252211
}
253-
if err != nil {
254-
return err
255-
}
256-
257-
fmt.Fprintln(o.out, "Importing (ctrl+c to stop waiting) ...")
258212

259-
resourceVersion := stream.ResourceVersion
260-
updatedStream, err := o.waitForImport(resourceVersion)
261-
if err != nil {
262-
if _, ok := err.(importError); ok {
263-
return err
213+
for _, image := range result.Status.Images {
214+
if image.Image != nil {
215+
info, err := describe.DescribeImage(image.Image, imageapi.JoinImageStreamTag(stream.Name, image.Tag))
216+
if err != nil {
217+
fmt.Fprintf(o.errout, "error: tag %s failed: %v\n", image.Tag, err)
218+
} else {
219+
fmt.Fprintln(o.out, info)
220+
}
221+
} else {
222+
fmt.Fprintf(o.errout, "error: tag %s failed: %v\n", image.Tag, image.Status.Message)
264223
}
265-
return fmt.Errorf("unable to determine if the import completed successfully - please run '%s describe -n %s imagestream/%s' to see if the tags were updated as expected: %v", o.CommandName, stream.Namespace, stream.Name, err)
266224
}
267225

268-
fmt.Fprint(o.out, "The import completed successfully.\n\n")
269-
270-
d := describe.ImageStreamDescriber{ImageClient: o.imageClient}
271-
info, err := d.Describe(updatedStream.Namespace, updatedStream.Name, kprinters.DescriberSettings{})
272-
if err != nil {
273-
return err
226+
if r := result.Status.Repository; r != nil && len(r.AdditionalTags) > 0 {
227+
fmt.Fprintf(o.out, "\ninfo: The remote repository contained %d additional tags which were not imported: %s\n", len(r.AdditionalTags), strings.Join(r.AdditionalTags, ", "))
274228
}
275-
276-
fmt.Fprintln(o.out, info)
277229
return nil
278230
}
279231

@@ -298,45 +250,6 @@ func (e importError) Error() string {
298250
return fmt.Sprintf("unable to import image: %s", e.annotation)
299251
}
300252

301-
func (o *ImportImageOptions) waitForImport(resourceVersion string) (*imageapi.ImageStream, error) {
302-
streamWatch, err := o.isClient.Watch(metav1.ListOptions{FieldSelector: fields.OneTermEqualSelector("metadata.name", o.Name).String(), ResourceVersion: resourceVersion})
303-
if err != nil {
304-
return nil, err
305-
}
306-
defer streamWatch.Stop()
307-
308-
for {
309-
select {
310-
case event, ok := <-streamWatch.ResultChan():
311-
if !ok {
312-
return nil, fmt.Errorf("image stream watch ended prematurely")
313-
}
314-
315-
switch event.Type {
316-
case watch.Modified:
317-
s, ok := event.Object.(*imageapi.ImageStream)
318-
if !ok {
319-
continue
320-
}
321-
annotation, ok := s.Annotations[imageapi.DockerImageRepositoryCheckAnnotation]
322-
if !ok {
323-
continue
324-
}
325-
326-
if _, err := time.Parse(time.RFC3339, annotation); err == nil {
327-
return s, nil
328-
}
329-
return nil, importError{annotation}
330-
331-
case watch.Deleted:
332-
return nil, fmt.Errorf("the image stream was deleted")
333-
case watch.Error:
334-
return nil, fmt.Errorf("error watching image stream")
335-
}
336-
}
337-
}
338-
}
339-
340253
func (o *ImportImageOptions) createImageImport() (*imageapi.ImageStream, *imageapi.ImageStreamImport, error) {
341254
var isi *imageapi.ImageStreamImport
342255
stream, err := o.isClient.Get(o.Name, metav1.GetOptions{})
@@ -594,27 +507,3 @@ func (o *ImportImageOptions) newImageStreamImportTags(stream *imageapi.ImageStre
594507
}
595508
return isi
596509
}
597-
598-
// TransformUnsupportedError converts specific error conditions to unsupported
599-
func TransformUnsupportedError(err error) error {
600-
if err == nil {
601-
return nil
602-
}
603-
if errors.IsNotFound(err) {
604-
status, ok := err.(errors.APIStatus)
605-
if !ok {
606-
return imageapi.ErrImageStreamImportUnsupported
607-
}
608-
if status.Status().Details == nil || status.Status().Details.Kind == "" {
609-
return imageapi.ErrImageStreamImportUnsupported
610-
}
611-
}
612-
// The ImageStreamImport resource exists in v1.1.1 of origin but is not yet
613-
// enabled by policy. A create request will return a Forbidden(403) error.
614-
// We want to return ErrImageStreamImportUnsupported to allow fallback behavior
615-
// in clients.
616-
if errors.IsForbidden(err) && !quotautil.IsErrorQuotaExceeded(err) {
617-
return imageapi.ErrImageStreamImportUnsupported
618-
}
619-
return err
620-
}

0 commit comments

Comments
 (0)