From 6dc136916c3c4ad1b2280f493a5f2fc161969ed0 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 25 Jul 2018 16:38:03 -0400 Subject: [PATCH 1/3] Provide a client method to bypass server side timeout on upload --- .../controller/imagestream_controller.go | 2 +- .../fake/fake_imagestreamimport_expansion.go | 18 +++++++++++++ .../internalversion/generated_expansion.go | 2 -- .../imagestreamimport_expansion.go | 26 +++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 pkg/image/generated/internalclientset/typed/image/internalversion/fake/fake_imagestreamimport_expansion.go create mode 100644 pkg/image/generated/internalclientset/typed/image/internalversion/imagestreamimport_expansion.go diff --git a/pkg/image/controller/imagestream_controller.go b/pkg/image/controller/imagestream_controller.go index f2bd667f588a..524944457f03 100644 --- a/pkg/image/controller/imagestream_controller.go +++ b/pkg/image/controller/imagestream_controller.go @@ -265,7 +265,7 @@ func handleImageStream( ImportPolicy: imageapi.TagImportPolicy{Insecure: insecure}, } } - result, err := client.ImageStreamImports(stream.Namespace).Create(isi) + result, err := client.ImageStreamImports(stream.Namespace).CreateWithoutTimeout(isi) if err != nil { if apierrs.IsNotFound(err) && isStatusErrorKind(err, "imageStream") { return result, ErrNotImportable diff --git a/pkg/image/generated/internalclientset/typed/image/internalversion/fake/fake_imagestreamimport_expansion.go b/pkg/image/generated/internalclientset/typed/image/internalversion/fake/fake_imagestreamimport_expansion.go new file mode 100644 index 000000000000..667f36dcf3b1 --- /dev/null +++ b/pkg/image/generated/internalclientset/typed/image/internalversion/fake/fake_imagestreamimport_expansion.go @@ -0,0 +1,18 @@ +// Code generated by client-gen. DO NOT EDIT. + +package fake + +import ( + image "github.com/openshift/origin/pkg/image/apis/image" + testing "k8s.io/client-go/testing" +) + +func (c *FakeImageStreamImports) CreateWithoutTimeout(imageStreamImport *image.ImageStreamImport) (result *image.ImageStreamImport, err error) { + obj, err := c.Fake. + Invokes(testing.NewCreateAction(imagestreamimportsResource, c.ns, imageStreamImport), &image.ImageStreamImport{}) + + if obj == nil { + return nil, err + } + return obj.(*image.ImageStreamImport), err +} diff --git a/pkg/image/generated/internalclientset/typed/image/internalversion/generated_expansion.go b/pkg/image/generated/internalclientset/typed/image/internalversion/generated_expansion.go index 1cb2022d5c8e..605eff59da48 100644 --- a/pkg/image/generated/internalclientset/typed/image/internalversion/generated_expansion.go +++ b/pkg/image/generated/internalclientset/typed/image/internalversion/generated_expansion.go @@ -10,8 +10,6 @@ type ImageStreamExpansion interface{} type ImageStreamImageExpansion interface{} -type ImageStreamImportExpansion interface{} - type ImageStreamMappingExpansion interface{} type ImageStreamTagExpansion interface{} diff --git a/pkg/image/generated/internalclientset/typed/image/internalversion/imagestreamimport_expansion.go b/pkg/image/generated/internalclientset/typed/image/internalversion/imagestreamimport_expansion.go new file mode 100644 index 000000000000..2a0034f860b4 --- /dev/null +++ b/pkg/image/generated/internalclientset/typed/image/internalversion/imagestreamimport_expansion.go @@ -0,0 +1,26 @@ +package internalversion + +import ( + "time" + + image "github.com/openshift/origin/pkg/image/apis/image" +) + +type ImageStreamImportExpansion interface { + CreateWithoutTimeout(*image.ImageStreamImport) (*image.ImageStreamImport, error) +} + +// CreateWithoutTimeout imports the provided images and won't time out after 30 seconds. Use this when you must +// import a large number of images. +func (c *imageStreamImports) CreateWithoutTimeout(imageStreamImport *image.ImageStreamImport) (result *image.ImageStreamImport, err error) { + result = &image.ImageStreamImport{} + err = c.client.Post(). + Namespace(c.ns). + Resource("imagestreamimports"). + Body(imageStreamImport). + // this instructs the api server to allow our request to take up to an hour - chosen as a high boundary + Timeout(time.Hour). + Do(). + Into(result) + return +} From 62b5ebff621d61bb6ceaae79dd3019503d953734 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 25 Jul 2018 23:24:20 -0400 Subject: [PATCH 2/3] Importer integration test was an off by one The last import had no change, so we were relying on the older, shorter timeout. Instead, we want to explicitly trigger this error. --- test/integration/imageimporter_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/integration/imageimporter_test.go b/test/integration/imageimporter_test.go index b5082cc3c55d..55389ed0a833 100644 --- a/test/integration/imageimporter_test.go +++ b/test/integration/imageimporter_test.go @@ -8,10 +8,12 @@ import ( "net/url" "reflect" "strings" + "sync" "testing" "time" "github.com/docker/distribution/registry/api/errcode" + "github.com/golang/glog" gocontext "golang.org/x/net/context" "k8s.io/apimachinery/pkg/api/errors" @@ -654,7 +656,10 @@ func TestImageStreamImportTagsFromRepository(t *testing.T) { func TestImageStreamImportScheduled(t *testing.T) { written := make(chan struct{}, 2) count := 0 + var lock sync.Mutex server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lock.Lock() + defer lock.Unlock() t.Logf("got %s %s", r.Method, r.URL.Path) switch r.URL.Path { case "/v2/": @@ -663,12 +668,13 @@ func TestImageStreamImportScheduled(t *testing.T) { case "/v2/test/image/manifests/latest", "/v2/test/image/manifests/" + etcdDigest, "/v2/test/image/manifests/" + phpDigest: count++ t.Logf("serving %d", count) + glog.Infof("serving request %d for %s", count, r.URL.Path) var manifest, digest string switch count { case 1, 2: digest = etcdDigest manifest = etcdManifest - case 3, 4, 5, 6: + case 3, 4, 5: digest = phpDigest manifest = phpManifest default: @@ -817,7 +823,7 @@ func TestImageStreamImportScheduled(t *testing.T) { t.Fatalf("expected generation 2 for stream and spec tag: %v %#v", tagGen, change) } conditions := change.Status.Tags["latest"].Conditions - if len(conditions) == 0 || conditions[0].Type != imageapi.ImportSuccess || conditions[0].Generation != 3 { + if len(conditions) == 0 || conditions[0].Type != imageapi.ImportSuccess || string(conditions[0].Status) != "False" || conditions[0].Generation != 3 { t.Fatalf("expected generation 3 for condition and import failed: %#v", conditions) } From e73f7b49a9595439a42c336410520da15d7fd98e Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 25 Jul 2018 23:29:14 -0400 Subject: [PATCH 3/3] Fix Go 1.10 issue with integration test --- test/integration/imagechange_buildtrigger_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/integration/imagechange_buildtrigger_test.go b/test/integration/imagechange_buildtrigger_test.go index a55e1dfa0f06..53ad62cd0a6f 100644 --- a/test/integration/imagechange_buildtrigger_test.go +++ b/test/integration/imagechange_buildtrigger_test.go @@ -302,19 +302,19 @@ func runTest(t *testing.T, testname string, projectAdminClientConfig *rest.Confi if strategy.SourceStrategy.From.Name != registryHostname+"/openshift/test-image-trigger:"+tag { i, _ := projectAdminImageClient.ImageStreams(testutil.Namespace()).Get(imageStream.Name, metav1.GetOptions{}) bc, _ := projectAdminBuildClient.BuildConfigs(testutil.Namespace()).Get(config.Name, metav1.GetOptions{}) - t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\ntrigger is %s\n", registryHostname+"/openshift/test-image-trigger:"+tag, strategy.SourceStrategy.From.Name, i, bc.Spec.Triggers[0].ImageChange) + t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\ntrigger is %#v\n", registryHostname+"/openshift/test-image-trigger:"+tag, strategy.SourceStrategy.From.Name, i, bc.Spec.Triggers[0].ImageChange) } case strategy.DockerStrategy != nil: if strategy.DockerStrategy.From.Name != registryHostname+"/openshift/test-image-trigger:"+tag { i, _ := projectAdminImageClient.ImageStreams(testutil.Namespace()).Get(imageStream.Name, metav1.GetOptions{}) bc, _ := projectAdminBuildClient.BuildConfigs(testutil.Namespace()).Get(config.Name, metav1.GetOptions{}) - t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\ntrigger is %s\n", registryHostname+"/openshift/test-image-trigger:"+tag, strategy.DockerStrategy.From.Name, i, bc.Spec.Triggers[0].ImageChange) + t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\ntrigger is %#v\n", registryHostname+"/openshift/test-image-trigger:"+tag, strategy.DockerStrategy.From.Name, i, bc.Spec.Triggers[0].ImageChange) } case strategy.CustomStrategy != nil: if strategy.CustomStrategy.From.Name != registryHostname+"/openshift/test-image-trigger:"+tag { i, _ := projectAdminImageClient.ImageStreams(testutil.Namespace()).Get(imageStream.Name, metav1.GetOptions{}) bc, _ := projectAdminBuildClient.BuildConfigs(testutil.Namespace()).Get(config.Name, metav1.GetOptions{}) - t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\ntrigger is %s\n", registryHostname+"/openshift/test-image-trigger:"+tag, strategy.CustomStrategy.From.Name, i, bc.Spec.Triggers[0].ImageChange) + t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\ntrigger is %#v\n", registryHostname+"/openshift/test-image-trigger:"+tag, strategy.CustomStrategy.From.Name, i, bc.Spec.Triggers[0].ImageChange) } } event = <-buildWatch.ResultChan() @@ -376,19 +376,19 @@ func runTest(t *testing.T, testname string, projectAdminClientConfig *rest.Confi if strategy.SourceStrategy.From.Name != registryHostname+"/openshift/test-image-trigger:ref-2-random" { i, _ := projectAdminImageClient.ImageStreams(testutil.Namespace()).Get(imageStream.Name, metav1.GetOptions{}) bc, _ := projectAdminBuildClient.BuildConfigs(testutil.Namespace()).Get(config.Name, metav1.GetOptions{}) - t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\trigger is %s\n", registryHostname+"/openshift/test-image-trigger:ref-2-random", strategy.SourceStrategy.From.Name, i, bc.Spec.Triggers[3].ImageChange) + t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\trigger is %#v\n", registryHostname+"/openshift/test-image-trigger:ref-2-random", strategy.SourceStrategy.From.Name, i, bc.Spec.Triggers[3].ImageChange) } case strategy.DockerStrategy != nil: if strategy.DockerStrategy.From.Name != registryHostname+"/openshift/test-image-trigger:ref-2-random" { i, _ := projectAdminImageClient.ImageStreams(testutil.Namespace()).Get(imageStream.Name, metav1.GetOptions{}) bc, _ := projectAdminBuildClient.BuildConfigs(testutil.Namespace()).Get(config.Name, metav1.GetOptions{}) - t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\trigger is %s\n", registryHostname+"/openshift/test-image-trigger:ref-2-random", strategy.DockerStrategy.From.Name, i, bc.Spec.Triggers[3].ImageChange) + t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\trigger is %#v\n", registryHostname+"/openshift/test-image-trigger:ref-2-random", strategy.DockerStrategy.From.Name, i, bc.Spec.Triggers[3].ImageChange) } case strategy.CustomStrategy != nil: if strategy.CustomStrategy.From.Name != registryHostname+"/openshift/test-image-trigger:ref-2-random" { i, _ := projectAdminImageClient.ImageStreams(testutil.Namespace()).Get(imageStream.Name, metav1.GetOptions{}) bc, _ := projectAdminBuildClient.BuildConfigs(testutil.Namespace()).Get(config.Name, metav1.GetOptions{}) - t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\trigger is %s\n", registryHostname+"/openshift/test-image-trigger:ref-2-random", strategy.CustomStrategy.From.Name, i, bc.Spec.Triggers[3].ImageChange) + t.Fatalf("Expected build with base image %s, got %s\n, imagerepo is %v\trigger is %#v\n", registryHostname+"/openshift/test-image-trigger:ref-2-random", strategy.CustomStrategy.From.Name, i, bc.Spec.Triggers[3].ImageChange) } }