diff --git a/pkg/build/apiserver/apiserver.go b/pkg/build/apiserver/apiserver.go index 866842d8a73a..bcb37d6d0402 100644 --- a/pkg/build/apiserver/apiserver.go +++ b/pkg/build/apiserver/apiserver.go @@ -161,12 +161,12 @@ func (c *BuildServerConfig) newV1RESTStorage() (map[string]rest.Storage, error) v1Storage := map[string]rest.Storage{} v1Storage["builds"] = buildStorage v1Storage["builds/clone"] = buildclone.NewStorage(buildGenerator) - v1Storage["builds/log"] = buildlogregistry.NewREST(buildStorage, buildStorage, kubeInternalClient.Core(), nodeConnectionInfoGetter) + v1Storage["builds/log"] = buildlogregistry.NewREST(buildClient.Build(), kubeInternalClient.Core(), nodeConnectionInfoGetter) v1Storage["builds/details"] = buildDetailsStorage v1Storage["buildConfigs"] = buildConfigStorage v1Storage["buildConfigs/webhooks"] = buildConfigWebHooks v1Storage["buildConfigs/instantiate"] = buildconfiginstantiate.NewStorage(buildGenerator) - v1Storage["buildConfigs/instantiatebinary"] = buildconfiginstantiate.NewBinaryStorage(buildGenerator, buildStorage, kubeInternalClient.Core(), nodeConnectionInfoGetter) + v1Storage["buildConfigs/instantiatebinary"] = buildconfiginstantiate.NewBinaryStorage(buildGenerator, buildClient.Build(), kubeInternalClient.Core(), nodeConnectionInfoGetter) return v1Storage, nil } diff --git a/pkg/build/registry/buildconfiginstantiate/rest.go b/pkg/build/registry/buildconfiginstantiate/rest.go index be35594dd1c6..6be82f014393 100644 --- a/pkg/build/registry/buildconfiginstantiate/rest.go +++ b/pkg/build/registry/buildconfiginstantiate/rest.go @@ -27,6 +27,7 @@ import ( buildapi "github.com/openshift/origin/pkg/build/apis/build" buildapiv1 "github.com/openshift/origin/pkg/build/apis/build/v1" buildstrategy "github.com/openshift/origin/pkg/build/controller/strategy" + buildtypedclient "github.com/openshift/origin/pkg/build/generated/internalclientset/typed/build/internalversion" "github.com/openshift/origin/pkg/build/generator" "github.com/openshift/origin/pkg/build/registry" buildutil "github.com/openshift/origin/pkg/build/util" @@ -84,10 +85,10 @@ func (s *InstantiateREST) ProducesMIMETypes(verb string) []string { var _ rest.StorageMetadata = &InstantiateREST{} -func NewBinaryStorage(generator *generator.BuildGenerator, watcher rest.Watcher, podClient kcoreclient.PodsGetter, info kubeletclient.ConnectionInfoGetter) *BinaryInstantiateREST { +func NewBinaryStorage(generator *generator.BuildGenerator, buildClient buildtypedclient.BuildsGetter, podClient kcoreclient.PodsGetter, info kubeletclient.ConnectionInfoGetter) *BinaryInstantiateREST { return &BinaryInstantiateREST{ Generator: generator, - Watcher: watcher, + BuildClient: buildClient, PodGetter: &podGetter{podClient}, ConnectionInfo: info, Timeout: 5 * time.Minute, @@ -96,7 +97,7 @@ func NewBinaryStorage(generator *generator.BuildGenerator, watcher rest.Watcher, type BinaryInstantiateREST struct { Generator *generator.BuildGenerator - Watcher rest.Watcher + BuildClient buildtypedclient.BuildsGetter PodGetter pod.ResourceGetter ConnectionInfo kubeletclient.ConnectionInfoGetter Timeout time.Duration @@ -224,7 +225,7 @@ func (h *binaryInstantiateHandler) handle(r io.Reader) (runtime.Object, error) { h.cancelBuild(build) }() - latest, ok, err := registry.WaitForRunningBuild(h.r.Watcher, h.ctx, build, remaining) + latest, ok, err := registry.WaitForRunningBuild(h.r.BuildClient, build, remaining) switch { case latest.Status.Phase == buildapi.BuildPhaseError: diff --git a/pkg/build/registry/buildlog/rest.go b/pkg/build/registry/buildlog/rest.go index 3bcab987c739..f2e0e93593a4 100644 --- a/pkg/build/registry/buildlog/rest.go +++ b/pkg/build/registry/buildlog/rest.go @@ -23,19 +23,20 @@ import ( buildapi "github.com/openshift/origin/pkg/build/apis/build" "github.com/openshift/origin/pkg/build/apis/build/validation" + buildtypedclient "github.com/openshift/origin/pkg/build/generated/internalclientset/typed/build/internalversion" "github.com/openshift/origin/pkg/build/registry" buildutil "github.com/openshift/origin/pkg/build/util" ) // REST is an implementation of RESTStorage for the api server. type REST struct { - Getter rest.Getter - Watcher rest.Watcher + BuildClient buildtypedclient.BuildsGetter PodGetter pod.ResourceGetter ConnectionInfo kubeletclient.ConnectionInfoGetter Timeout time.Duration } +// TODO these wrapers shouldb e removed type podGetter struct { kcoreclient.PodsGetter } @@ -53,10 +54,9 @@ const defaultTimeout time.Duration = 10 * time.Second // NewREST creates a new REST for BuildLog // Takes build registry and pod client to get necessary attributes to assemble // URL to which the request shall be redirected in order to get build logs. -func NewREST(getter rest.Getter, watcher rest.Watcher, pn kcoreclient.PodsGetter, connectionInfo kubeletclient.ConnectionInfoGetter) *REST { +func NewREST(buildClient buildtypedclient.BuildsGetter, pn kcoreclient.PodsGetter, connectionInfo kubeletclient.ConnectionInfoGetter) *REST { return &REST{ - Getter: getter, - Watcher: watcher, + BuildClient: buildClient, PodGetter: &podGetter{pn}, ConnectionInfo: connectionInfo, Timeout: defaultTimeout, @@ -74,21 +74,20 @@ func (r *REST) Get(ctx apirequest.Context, name string, opts runtime.Object) (ru if errs := validation.ValidateBuildLogOptions(buildLogOpts); len(errs) > 0 { return nil, errors.NewInvalid(buildapi.Kind("BuildLogOptions"), "", errs) } - obj, err := r.Getter.Get(ctx, name, &metav1.GetOptions{}) + build, err := r.BuildClient.Builds(apirequest.NamespaceValue(ctx)).Get(name, metav1.GetOptions{}) if err != nil { return nil, err } - build := obj.(*buildapi.Build) if buildLogOpts.Previous { version := buildutil.VersionForBuild(build) // Use the previous version version-- previousBuildName := buildutil.BuildNameForConfigVersion(buildutil.ConfigNameForBuild(build), version) - previous, err := r.Getter.Get(ctx, previousBuildName, &metav1.GetOptions{}) + previous, err := r.BuildClient.Builds(apirequest.NamespaceValue(ctx)).Get(previousBuildName, metav1.GetOptions{}) if err != nil { return nil, err } - build = previous.(*buildapi.Build) + build = previous } switch build.Status.Phase { // Build has not launched, wait until it runs @@ -99,7 +98,7 @@ func (r *REST) Get(ctx apirequest.Context, name string, opts runtime.Object) (ru return &genericrest.LocationStreamer{}, nil } glog.V(4).Infof("Build %s/%s is in %s state, waiting for Build to start", build.Namespace, build.Name, build.Status.Phase) - latest, ok, err := registry.WaitForRunningBuild(r.Watcher, ctx, build, r.Timeout) + latest, ok, err := registry.WaitForRunningBuild(r.BuildClient, build, r.Timeout) if err != nil { return nil, errors.NewBadRequest(fmt.Sprintf("unable to wait for build %s to run: %v", build.Name, err)) } @@ -126,7 +125,7 @@ func (r *REST) Get(ctx apirequest.Context, name string, opts runtime.Object) (ru // if we can't at least get the build pod, we're not going to get very far, so // error out now. - obj, err = r.PodGetter.Get(ctx, buildPodName, &metav1.GetOptions{}) + obj, err := r.PodGetter.Get(ctx, buildPodName, &metav1.GetOptions{}) if err != nil { return nil, errors.NewBadRequest(err.Error()) } diff --git a/pkg/build/registry/buildlog/rest_test.go b/pkg/build/registry/buildlog/rest_test.go index 521254277120..768f6d0266c8 100644 --- a/pkg/build/registry/buildlog/rest_test.go +++ b/pkg/build/registry/buildlog/rest_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - metainternal "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -16,11 +15,12 @@ import ( apirequest "k8s.io/apiserver/pkg/endpoints/request" genericrest "k8s.io/apiserver/pkg/registry/generic/rest" "k8s.io/apiserver/pkg/registry/rest" + clientgotesting "k8s.io/client-go/testing" kapi "k8s.io/kubernetes/pkg/api" kubeletclient "k8s.io/kubernetes/pkg/kubelet/client" buildapi "github.com/openshift/origin/pkg/build/apis/build" - "github.com/openshift/origin/pkg/build/registry/test" + buildfakeclient "github.com/openshift/origin/pkg/build/generated/internalclientset/fake" ) type testPodGetter struct{} @@ -139,26 +139,20 @@ func TestWaitForBuild(t *testing.T) { for _, tt := range tests { build := mockBuild(buildapi.BuildPhasePending, "running", 1) - ch := make(chan watch.Event) - watcher := &buildWatcher{ - Build: build, - Watcher: &fakeWatch{ - Channel: ch, - }, - } + buildClient := buildfakeclient.NewSimpleClientset(build) + fakeWatcher := watch.NewFake() + buildClient.PrependWatchReactor("builds", func(action clientgotesting.Action) (handled bool, ret watch.Interface, err error) { + return true, fakeWatcher, nil + }) storage := REST{ - Getter: watcher, - Watcher: watcher, + BuildClient: buildClient.Build(), PodGetter: &testPodGetter{}, ConnectionInfo: &fakeConnectionInfoGetter{}, Timeout: defaultTimeout, } go func() { for _, status := range tt.status { - ch <- watch.Event{ - Type: watch.Modified, - Object: mockBuild(status, "running", 1), - } + fakeWatcher.Modify(mockBuild(status, "running", 1)) } }() _, err := storage.Get(ctx, build.Name, &buildapi.BuildLogOptions{}) @@ -172,18 +166,11 @@ func TestWaitForBuild(t *testing.T) { } func TestWaitForBuildTimeout(t *testing.T) { - ctx := apirequest.NewDefaultContext() build := mockBuild(buildapi.BuildPhasePending, "running", 1) - ch := make(chan watch.Event) - watcher := &buildWatcher{ - Build: build, - Watcher: &fakeWatch{ - Channel: ch, - }, - } + buildClient := buildfakeclient.NewSimpleClientset(build) + ctx := apirequest.NewDefaultContext() storage := REST{ - Getter: watcher, - Watcher: watcher, + BuildClient: buildClient.Build(), PodGetter: &testPodGetter{}, ConnectionInfo: &fakeConnectionInfoGetter{}, Timeout: 100 * time.Millisecond, @@ -194,44 +181,18 @@ func TestWaitForBuildTimeout(t *testing.T) { } } -type buildWatcher struct { - Build *buildapi.Build - Watcher watch.Interface - Err error -} - -func (r *buildWatcher) Get(ctx apirequest.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { - return r.Build, nil -} - -func (r *buildWatcher) Watch(ctx apirequest.Context, options *metainternal.ListOptions) (watch.Interface, error) { - return r.Watcher, r.Err -} - -type fakeWatch struct { - Channel chan watch.Event -} - -func (w *fakeWatch) Stop() { - close(w.Channel) -} - -func (w *fakeWatch) ResultChan() <-chan watch.Event { - return w.Channel -} - func resourceLocationHelper(BuildPhase buildapi.BuildPhase, podPhase string, ctx apirequest.Context, version int) (string, error) { expectedBuild := mockBuild(BuildPhase, podPhase, version) - internal := &test.BuildStorage{Build: expectedBuild} + buildClient := buildfakeclient.NewSimpleClientset(expectedBuild) storage := &REST{ - Getter: internal, + BuildClient: buildClient.Build(), PodGetter: &testPodGetter{}, ConnectionInfo: &fakeConnectionInfoGetter{}, Timeout: defaultTimeout, } getter := rest.GetterWithOptions(storage) - obj, err := getter.Get(ctx, "foo-build", &buildapi.BuildLogOptions{NoWait: true}) + obj, err := getter.Get(ctx, expectedBuild.Name, &buildapi.BuildLogOptions{NoWait: true}) if err != nil { return "", err } @@ -269,7 +230,8 @@ func mockPod(podPhase kapi.PodPhase, podName string) *kapi.Pod { func mockBuild(status buildapi.BuildPhase, podName string, version int) *buildapi.Build { return &buildapi.Build{ ObjectMeta: metav1.ObjectMeta{ - Name: podName, + Namespace: "default", + Name: podName, Annotations: map[string]string{ buildapi.BuildNumberAnnotation: strconv.Itoa(version), }, @@ -303,10 +265,10 @@ func TestPreviousBuildLogs(t *testing.T) { first := mockBuild(buildapi.BuildPhaseComplete, "bc-1", 1) second := mockBuild(buildapi.BuildPhaseComplete, "bc-2", 2) third := mockBuild(buildapi.BuildPhaseComplete, "bc-3", 3) - internal := &test.BuildStorage{Builds: &buildapi.BuildList{Items: []buildapi.Build{*first, *second, *third}}} + buildClient := buildfakeclient.NewSimpleClientset(first, second, third) storage := &REST{ - Getter: internal, + BuildClient: buildClient.Build(), PodGetter: &anotherTestPodGetter{}, ConnectionInfo: &fakeConnectionInfoGetter{}, Timeout: defaultTimeout, diff --git a/pkg/build/registry/rest.go b/pkg/build/registry/rest.go index bfb63a26011c..91e6ab3ea915 100644 --- a/pkg/build/registry/rest.go +++ b/pkg/build/registry/rest.go @@ -4,13 +4,12 @@ import ( "fmt" "time" - metainternal "k8s.io/apimachinery/pkg/apis/meta/internalversion" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/watch" - apirequest "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/apiserver/pkg/registry/rest" buildapi "github.com/openshift/origin/pkg/build/apis/build" + buildtypedclient "github.com/openshift/origin/pkg/build/generated/internalclientset/typed/build/internalversion" ) var ( @@ -22,39 +21,38 @@ var ( // WaitForRunningBuild waits until the specified build is no longer New or Pending. Returns true if // the build ran within timeout, false if it did not, and an error if any other error state occurred. // The last observed Build state is returned. -func WaitForRunningBuild(watcher rest.Watcher, ctx apirequest.Context, build *buildapi.Build, timeout time.Duration) (*buildapi.Build, bool, error) { +func WaitForRunningBuild(buildClient buildtypedclient.BuildsGetter, build *buildapi.Build, timeout time.Duration) (*buildapi.Build, bool, error) { fieldSelector := fields.OneTermEqualSelector("metadata.name", build.Name) - options := &metainternal.ListOptions{FieldSelector: fieldSelector, ResourceVersion: build.ResourceVersion} - w, err := watcher.Watch(ctx, options) + options := metav1.ListOptions{FieldSelector: fieldSelector.String(), ResourceVersion: build.ResourceVersion} + w, err := buildClient.Builds(build.Namespace).Watch(options) if err != nil { return build, false, err } - defer w.Stop() observed := build - ch := w.ResultChan() - expire := time.After(timeout) - for { - select { - case event := <-ch: - obj, ok := event.Object.(*buildapi.Build) - if !ok { - return observed, false, fmt.Errorf("received unknown object while watching for builds") - } - observed = obj - - if event.Type == watch.Deleted { - return observed, false, ErrBuildDeleted - } - switch obj.Status.Phase { - case buildapi.BuildPhaseRunning, buildapi.BuildPhaseComplete, buildapi.BuildPhaseFailed, buildapi.BuildPhaseError, buildapi.BuildPhaseCancelled: - return observed, true, nil - case buildapi.BuildPhaseNew, buildapi.BuildPhasePending: - default: - return observed, false, ErrUnknownBuildPhase - } - case <-expire: - return observed, false, nil + _, err = watch.Until(timeout, w, func(event watch.Event) (bool, error) { + obj, ok := event.Object.(*buildapi.Build) + if !ok { + return false, fmt.Errorf("received unknown object while watching for builds: %T", obj) } + observed = obj + + if event.Type == watch.Deleted { + return false, ErrBuildDeleted + } + switch obj.Status.Phase { + case buildapi.BuildPhaseRunning, buildapi.BuildPhaseComplete, buildapi.BuildPhaseFailed, buildapi.BuildPhaseError, buildapi.BuildPhaseCancelled: + return true, nil + case buildapi.BuildPhaseNew, buildapi.BuildPhasePending: + default: + return false, ErrUnknownBuildPhase + } + + return false, nil + }) + if err != nil { + return nil, false, err } + + return observed, true, nil }