diff --git a/pkg/build/builder/util.go b/pkg/build/builder/util.go index c3870a77d51f..e6d659df8bc9 100644 --- a/pkg/build/builder/util.go +++ b/pkg/build/builder/util.go @@ -1,7 +1,7 @@ package builder import ( - "github.com/openshift/origin/pkg/build/api" + buildapi "github.com/openshift/origin/pkg/build/api" ) type Builder interface { @@ -10,7 +10,7 @@ type Builder interface { // getBuildEnvVars returns a map with the environment variables that should be added // to the built image -func getBuildEnvVars(build *api.Build) map[string]string { +func getBuildEnvVars(build *buildapi.Build) map[string]string { envVars := map[string]string{ "OPENSHIFT_BUILD_NAME": build.Name, "OPENSHIFT_BUILD_NAMESPACE": build.Namespace, diff --git a/pkg/build/client/clients.go b/pkg/build/client/clients.go new file mode 100644 index 000000000000..c9724e3a979b --- /dev/null +++ b/pkg/build/client/clients.go @@ -0,0 +1,69 @@ +package client + +import ( + buildapi "github.com/openshift/origin/pkg/build/api" + osclient "github.com/openshift/origin/pkg/client" +) + +// BuildConfigGetter provides methods for getting BuildConfigs +type BuildConfigGetter interface { + Get(namespace, name string) (*buildapi.BuildConfig, error) +} + +// BuildConfigUpdater provides methods for updating BuildConfigs +type BuildConfigUpdater interface { + Update(buildConfig *buildapi.BuildConfig) error +} + +// OSClientBuildConfigClient delegates get and update operations to the OpenShift client interface +type OSClientBuildConfigClient struct { + Client osclient.Interface +} + +// NewOSClientBuildConfigClient creates a new build config client that uses an openshift client to create and get BuildConfigs +func NewOSClientBuildConfigClient(client osclient.Interface) *OSClientBuildConfigClient { + return &OSClientBuildConfigClient{Client: client} +} + +// Get returns a BuildConfig using the OpenShift client. +func (c OSClientBuildConfigClient) Get(namespace, name string) (*buildapi.BuildConfig, error) { + return c.Client.BuildConfigs(namespace).Get(name) +} + +// Update updates a BuildConfig using the OpenShift client. +func (c OSClientBuildConfigClient) Update(buildConfig *buildapi.BuildConfig) error { + _, err := c.Client.BuildConfigs(buildConfig.Namespace).Update(buildConfig) + return err +} + +// BuildCreator provides methods for creating new Builds +type BuildCreator interface { + Create(namespace string, build *buildapi.Build) error +} + +// BuildUpdater provides methods for updating existing Builds. +type BuildUpdater interface { + Update(namespace string, build *buildapi.Build) error +} + +// OSClientBuildClient deletes build create and update operations to the OpenShift client interface +type OSClientBuildClient struct { + Client osclient.Interface +} + +// NewOSClientBuildClient creates a new build client that uses an openshift client to create builds +func NewOSClientBuildClient(client osclient.Interface) *OSClientBuildClient { + return &OSClientBuildClient{Client: client} +} + +// Create creates builds using the OpenShift client. +func (c OSClientBuildClient) Create(namespace string, build *buildapi.Build) error { + _, e := c.Client.Builds(namespace).Create(build) + return e +} + +// Update updates builds using the OpenShift client. +func (c OSClientBuildClient) Update(namespace string, build *buildapi.Build) error { + _, e := c.Client.Builds(namespace).Update(build) + return e +} diff --git a/pkg/build/controller/controller.go b/pkg/build/controller/controller.go index 4ce6dc2aa787..a3ce54a97961 100644 --- a/pkg/build/controller/controller.go +++ b/pkg/build/controller/controller.go @@ -11,6 +11,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/util" buildapi "github.com/openshift/origin/pkg/build/api" + buildclient "github.com/openshift/origin/pkg/build/client" imageapi "github.com/openshift/origin/pkg/image/api" ) @@ -19,7 +20,7 @@ type BuildController struct { BuildStore cache.Store NextBuild func() *buildapi.Build NextPod func() *kapi.Pod - BuildUpdater buildUpdater + BuildUpdater buildclient.BuildUpdater PodManager podManager BuildStrategy BuildStrategy @@ -31,10 +32,6 @@ type BuildStrategy interface { CreateBuildPod(build *buildapi.Build) (*kapi.Pod, error) } -type buildUpdater interface { - UpdateBuild(namespace string, build *buildapi.Build) (*buildapi.Build, error) -} - type podManager interface { CreatePod(namespace string, pod *kapi.Pod) (*kapi.Pod, error) DeletePod(namespace string, pod *kapi.Pod) error @@ -67,7 +64,7 @@ func (bc *BuildController) HandleBuild(build *buildapi.Build) { build.Message = err.Error() } - if _, err := bc.BuildUpdater.UpdateBuild(build.Namespace, build); err != nil { + if err := bc.BuildUpdater.Update(build.Namespace, build); err != nil { glog.V(2).Infof("Failed to record changes to build %s/%s: %#v", build.Namespace, build.Name, err) } } @@ -177,7 +174,7 @@ func (bc *BuildController) HandlePod(pod *kapi.Pod) { if build.Status != nextStatus { glog.V(4).Infof("Updating build %s status %s -> %s", build.Name, build.Status, nextStatus) build.Status = nextStatus - if _, err := bc.BuildUpdater.UpdateBuild(build.Namespace, build); err != nil { + if err := bc.BuildUpdater.Update(build.Namespace, build); err != nil { glog.Errorf("Failed to update build %s: %#v", build.Name, err) } } @@ -196,7 +193,7 @@ func (bc *BuildController) CancelBuild(build *buildapi.Build, pod *kapi.Pod) err } build.Status = buildapi.BuildStatusCancelled - if _, err := bc.BuildUpdater.UpdateBuild(build.Namespace, build); err != nil { + if err := bc.BuildUpdater.Update(build.Namespace, build); err != nil { return err } diff --git a/pkg/build/controller/controller_test.go b/pkg/build/controller/controller_test.go index 0d25408b98d4..4b3ce128b541 100644 --- a/pkg/build/controller/controller_test.go +++ b/pkg/build/controller/controller_test.go @@ -8,20 +8,21 @@ import ( kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" buildapi "github.com/openshift/origin/pkg/build/api" + buildclient "github.com/openshift/origin/pkg/build/client" buildtest "github.com/openshift/origin/pkg/build/controller/test" imageapi "github.com/openshift/origin/pkg/image/api" ) type okBuildUpdater struct{} -func (obu *okBuildUpdater) UpdateBuild(namespace string, build *buildapi.Build) (*buildapi.Build, error) { - return &buildapi.Build{}, nil +func (okc *okBuildUpdater) Update(namespace string, build *buildapi.Build) error { + return nil } type errBuildUpdater struct{} -func (ebu *errBuildUpdater) UpdateBuild(namespace string, build *buildapi.Build) (*buildapi.Build, error) { - return &buildapi.Build{}, errors.New("UpdateBuild error!") +func (ec *errBuildUpdater) Update(namespace string, build *buildapi.Build) error { + return errors.New("UpdateBuild error!") } type okStrategy struct { @@ -153,7 +154,7 @@ func TestHandleBuild(t *testing.T) { outStatus buildapi.BuildStatus buildOutput buildapi.BuildOutput buildStrategy BuildStrategy - buildUpdater buildUpdater + buildUpdater buildclient.BuildUpdater imageClient imageRepositoryClient podManager podManager outputSpec string @@ -316,7 +317,7 @@ func TestHandlePod(t *testing.T) { outStatus buildapi.BuildStatus podStatus kapi.PodPhase exitCode int - buildUpdater buildUpdater + buildUpdater buildclient.BuildUpdater } tests := []handlePodTest{ diff --git a/pkg/build/controller/factory/factory.go b/pkg/build/controller/factory/factory.go index f6af56a9ff65..2c7b776c314c 100644 --- a/pkg/build/controller/factory/factory.go +++ b/pkg/build/controller/factory/factory.go @@ -14,16 +14,17 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" buildapi "github.com/openshift/origin/pkg/build/api" + buildclient "github.com/openshift/origin/pkg/build/client" controller "github.com/openshift/origin/pkg/build/controller" strategy "github.com/openshift/origin/pkg/build/controller/strategy" - buildutil "github.com/openshift/origin/pkg/build/util" osclient "github.com/openshift/origin/pkg/client" imageapi "github.com/openshift/origin/pkg/image/api" ) type BuildControllerFactory struct { - Client *osclient.Client - KubeClient *kclient.Client + OSClient osclient.Interface + KubeClient kclient.Interface + BuildUpdater buildclient.BuildUpdater DockerBuildStrategy *strategy.DockerBuildStrategy STIBuildStrategy *strategy.STIBuildStrategy CustomBuildStrategy *strategy.CustomBuildStrategy @@ -35,10 +36,10 @@ type BuildControllerFactory struct { func (factory *BuildControllerFactory) Create() *controller.BuildController { factory.buildStore = cache.NewStore() - cache.NewReflector(&buildLW{client: factory.Client}, &buildapi.Build{}, factory.buildStore).RunUntil(factory.Stop) + cache.NewReflector(&buildLW{client: factory.OSClient}, &buildapi.Build{}, factory.buildStore).RunUntil(factory.Stop) buildQueue := cache.NewFIFO() - cache.NewReflector(&buildLW{client: factory.Client}, &buildapi.Build{}, buildQueue).RunUntil(factory.Stop) + cache.NewReflector(&buildLW{client: factory.OSClient}, &buildapi.Build{}, buildQueue).RunUntil(factory.Stop) // Kubernetes does not currently synchronize Pod status in storage with a Pod's container // states. Because of this, we can't receive events related to container (and thus Pod) @@ -51,10 +52,10 @@ func (factory *BuildControllerFactory) Create() *controller.BuildController { podQueue := cache.NewFIFO() cache.NewPoller(factory.pollPods, 10*time.Second, podQueue).RunUntil(factory.Stop) - client := ControllerClient{factory.KubeClient, factory.Client} + client := ControllerClient{factory.KubeClient, factory.OSClient} return &controller.BuildController{ BuildStore: factory.buildStore, - BuildUpdater: client, + BuildUpdater: factory.BuildUpdater, ImageRepositoryClient: client, PodManager: client, NextBuild: func() *buildapi.Build { @@ -78,7 +79,9 @@ func (factory *BuildControllerFactory) Create() *controller.BuildController { // ImageChangeControllerFactory can create an ImageChangeController which obtains ImageRepositories // from a queue populated from a watch of all ImageRepositories. type ImageChangeControllerFactory struct { - Client *osclient.Client + Client osclient.Interface + BuildCreator buildclient.BuildCreator + BuildConfigUpdater buildclient.BuildConfigUpdater // Stop may be set to allow controllers created by this factory to be terminated. Stop <-chan struct{} } @@ -92,11 +95,10 @@ func (factory *ImageChangeControllerFactory) Create() *controller.ImageChangeCon store := cache.NewStore() cache.NewReflector(&buildConfigLW{client: factory.Client}, &buildapi.BuildConfig{}, store).RunUntil(factory.Stop) - client := ControllerClient{nil, factory.Client} return &controller.ImageChangeController{ BuildConfigStore: store, - BuildConfigUpdater: client, - BuildCreator: client, + BuildConfigUpdater: factory.BuildConfigUpdater, + BuildCreator: factory.BuildCreator, NextImageRepository: func() *imageapi.ImageRepository { repo := queue.Pop().(*imageapi.ImageRepository) panicIfStopped(factory.Stop, "build image change controller stopped") @@ -235,30 +237,6 @@ func (c ControllerClient) DeletePod(namespace string, pod *kapi.Pod) error { return c.KubeClient.Pods(namespace).Delete(pod.Name) } -// UpdateBuild updates build using the OpenShift client. -func (c ControllerClient) UpdateBuild(namespace string, build *buildapi.Build) (*buildapi.Build, error) { - return c.Client.Builds(namespace).Update(build) -} - -// CreateBuild updates build using the OpenShift client. -func (c ControllerClient) CreateBuild(config *buildapi.BuildConfig, imageSubstitutions map[string]string) error { - build := buildutil.GenerateBuildFromConfig(config, nil) - for originalImage, newImage := range imageSubstitutions { - buildutil.SubstituteImageReferences(build, originalImage, newImage) - } - if _, err := c.Client.Builds(config.Namespace).Create(build); err != nil { - glog.V(2).Infof("Error creating build for buildConfig %v: %v", config.Name, err) - return err - } - return nil -} - -// UpdateBuildConfig updates buildConfig using the OpenShift client. -func (c ControllerClient) UpdateBuildConfig(buildConfig *buildapi.BuildConfig) error { - _, err := c.Client.BuildConfigs(buildConfig.Namespace).Update(buildConfig) - return err -} - // GetImageRepository retrieves an image repository by namespace and name func (c ControllerClient) GetImageRepository(namespace, name string) (*imageapi.ImageRepository, error) { return c.Client.ImageRepositories(namespace).Get(name) diff --git a/pkg/build/controller/image_change_controller.go b/pkg/build/controller/image_change_controller.go index 6576de32ae12..d948c5f22cc8 100644 --- a/pkg/build/controller/image_change_controller.go +++ b/pkg/build/controller/image_change_controller.go @@ -7,6 +7,8 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/util" buildapi "github.com/openshift/origin/pkg/build/api" + buildclient "github.com/openshift/origin/pkg/build/client" + buildutil "github.com/openshift/origin/pkg/build/util" imageapi "github.com/openshift/origin/pkg/image/api" ) @@ -16,20 +18,12 @@ import ( type ImageChangeController struct { NextImageRepository func() *imageapi.ImageRepository BuildConfigStore cache.Store - BuildConfigUpdater buildConfigUpdater - BuildCreator buildCreator + BuildCreator buildclient.BuildCreator + BuildConfigUpdater buildclient.BuildConfigUpdater // Stop is an optional channel that controls when the controller exits Stop <-chan struct{} } -type buildConfigUpdater interface { - UpdateBuildConfig(buildConfig *buildapi.BuildConfig) error -} - -type buildCreator interface { - CreateBuild(build *buildapi.BuildConfig, imageSubstitutions map[string]string) error -} - // Run processes ImageRepository events one by one. func (c *ImageChangeController) Run() { go util.Until(c.HandleImageRepo, 0, c.Stop) @@ -54,7 +48,8 @@ func (c *ImageChangeController) HandleImageRepo() { continue } icTrigger := trigger.ImageChange - if icTrigger.From.Name != imageRepo.Name { + // only trigger a build if this image repo matches the name and namespace of the ref in the build trigger + if icTrigger.From.Name != imageRepo.Name || (len(icTrigger.From.Namespace) != 0 && icTrigger.From.Namespace != imageRepo.Namespace) { continue } // for every ImageChange trigger, record the image it substitutes for and get the latest @@ -78,11 +73,12 @@ func (c *ImageChangeController) HandleImageRepo() { } if shouldTriggerBuild { - glog.V(4).Infof("Running build for buildConfig %s", config.Name) - if err := c.BuildCreator.CreateBuild(config, imageSubstitutions); err != nil { + glog.V(4).Infof("Running build for buildConfig %s in namespace %s", config.Name, config.Namespace) + b := buildutil.GenerateBuildFromConfig(config, nil, imageSubstitutions) + if err := c.BuildCreator.Create(config.Namespace, b); err != nil { glog.V(2).Infof("Error starting build for buildConfig %v: %v", config.Name, err) } else { - if err := c.BuildConfigUpdater.UpdateBuildConfig(config); err != nil { + if err := c.BuildConfigUpdater.Update(config); err != nil { glog.V(2).Infof("Error updating buildConfig %v: %v", config.Name, err) } } diff --git a/pkg/build/controller/image_change_controller_test.go b/pkg/build/controller/image_change_controller_test.go index c3c88f310acb..24100d0d7c95 100644 --- a/pkg/build/controller/image_change_controller_test.go +++ b/pkg/build/controller/image_change_controller_test.go @@ -15,20 +15,18 @@ type mockBuildConfigUpdater struct { buildcfg *buildapi.BuildConfig } -func (m *mockBuildConfigUpdater) UpdateBuildConfig(buildcfg *buildapi.BuildConfig) error { +func (m *mockBuildConfigUpdater) Update(buildcfg *buildapi.BuildConfig) error { m.buildcfg = buildcfg return nil } type mockBuildCreator struct { - buildcfg *buildapi.BuildConfig - imageSubstitutions map[string]string - err error + build *buildapi.Build + err error } -func (m *mockBuildCreator) CreateBuild(buildcfg *buildapi.BuildConfig, imageSubstitutions map[string]string) error { - m.buildcfg = buildcfg - m.imageSubstitutions = imageSubstitutions +func (m *mockBuildCreator) Create(namespace string, build *buildapi.Build) error { + m.build = build return m.err } @@ -86,8 +84,8 @@ func mockImageChangeController(buildcfg *buildapi.BuildConfig, repoName, dockerI return &ImageChangeController{ NextImageRepository: func() *imageapi.ImageRepository { return &imageRepo }, BuildConfigStore: buildtest.NewFakeBuildConfigStore(buildcfg), - BuildConfigUpdater: &mockBuildConfigUpdater{}, BuildCreator: &mockBuildCreator{}, + BuildConfigUpdater: &mockBuildConfigUpdater{}, } } @@ -99,11 +97,11 @@ func TestNewImageID(t *testing.T) { buildCreator := controller.BuildCreator.(*mockBuildCreator) buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg == nil { + if buildCreator.build == nil { t.Error("Expected new build when new image was created!") } - if buildCreator.imageSubstitutions["registry.com/namespace/imagename"] != "registry.com/namespace/imagename:newImageID123" { - t.Errorf("Image substitutions not properly setup for new build: %s |", buildCreator.imageSubstitutions["registry.com/namespace/imagename"]) + if buildCreator.build.Parameters.Strategy.DockerStrategy.BaseImage != "registry.com/namespace/imagename:newImageID123" { + t.Errorf("Image substitutions not properly setup for new build. Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.BaseImage) } if buildConfigUpdater.buildcfg == nil { t.Fatal("Expected buildConfig update when new image was created!") @@ -121,11 +119,11 @@ func TestNewImageIDDefaultTag(t *testing.T) { buildCreator := controller.BuildCreator.(*mockBuildCreator) buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg == nil { + if buildCreator.build == nil { t.Error("Expected new build when new image was created!") } - if buildCreator.imageSubstitutions["registry.com/namespace/imagename"] != "registry.com/namespace/imagename:newImageID123" { - t.Errorf("Image substitutions not properly setup for new build using default tag: %s |", buildCreator.imageSubstitutions["registry.com/namespace/imagename"]) + if buildCreator.build.Parameters.Strategy.DockerStrategy.BaseImage != "registry.com/namespace/imagename:newImageID123" { + t.Errorf("Image substitutions not properly setup for new build. Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.BaseImage) } if buildConfigUpdater.buildcfg == nil { t.Fatal("Expected buildConfig update when new image was created!") @@ -144,12 +142,9 @@ func TestNonExistentImageRepository(t *testing.T) { buildCreator := controller.BuildCreator.(*mockBuildCreator) buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg != nil { + if buildCreator.build != nil { t.Error("New build created when a different repository was updated!") } - if len(buildCreator.imageSubstitutions) != 0 { - t.Errorf("Should not have had any image substitutions since tag does not exist in imagerepo") - } if buildConfigUpdater.buildcfg != nil { t.Error("BuildConfig was updated when a different repository was updated!") } @@ -163,12 +158,9 @@ func TestNewImageDifferentTagUpdate(t *testing.T) { buildCreator := controller.BuildCreator.(*mockBuildCreator) buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg != nil { + if buildCreator.build != nil { t.Error("New build created when a different repository was updated!") } - if len(buildCreator.imageSubstitutions) != 0 { - t.Errorf("Should not have had any image substitutions since tag does not exist in imagerepo") - } if buildConfigUpdater.buildcfg != nil { t.Error("BuildConfig was updated when a different repository was updated!") } @@ -176,6 +168,7 @@ func TestNewImageDifferentTagUpdate(t *testing.T) { func TestNewImageDifferentTagUpdate2(t *testing.T) { // this buildconfig references a different tag than the one that will be updated + // it has previously run a build for the testTagID123 tag. buildcfg := mockBuildConfig("registry.com/namespace/imagename", "registry.com/namespace/imagename", "testImageRepo", "testTag") buildcfg.Triggers[0].ImageChange.LastTriggeredImageID = "testTagID123" controller := mockImageChangeController(buildcfg, "testImageRepo", "registry.com/namespace/imagename", @@ -184,12 +177,9 @@ func TestNewImageDifferentTagUpdate2(t *testing.T) { buildCreator := controller.BuildCreator.(*mockBuildCreator) buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg != nil { + if buildCreator.build != nil { t.Error("New build created when a different repository was updated!") } - if len(buildCreator.imageSubstitutions) != 0 { - t.Errorf("Should not have had any image substitutions since tag does not exist in imagerepo") - } if buildConfigUpdater.buildcfg != nil { t.Error("BuildConfig was updated when a different repository was updated!") } @@ -203,12 +193,9 @@ func TestNewDifferentImageUpdate(t *testing.T) { buildCreator := controller.BuildCreator.(*mockBuildCreator) buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg != nil { + if buildCreator.build != nil { t.Error("New build created when a different repository was updated!") } - if len(buildCreator.imageSubstitutions) != 0 { - t.Errorf("Should not have had any image substitutions since tag does not exist in imagerepo") - } if buildConfigUpdater.buildcfg != nil { t.Error("BuildConfig was updated when a different repository was updated!") } @@ -223,15 +210,13 @@ func TestMultipleTriggers(t *testing.T) { buildCreator := controller.BuildCreator.(*mockBuildCreator) buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg == nil { + if buildCreator.build == nil { t.Error("Expected new build when new image was created!") } - if len(buildCreator.imageSubstitutions) != 1 { - t.Errorf("Expected exactly 1 substitution, got different count: %d", len(buildCreator.imageSubstitutions)) - } - if buildCreator.imageSubstitutions["registry.com/namespace/imagename2"] != "registry.com/namespace/imagename2:newImageID123" { - t.Errorf("Image substitutions not properly setup for new build using default tag: %s |", buildCreator.imageSubstitutions["registry.com/namespace/imagename2"]) + if buildCreator.build.Parameters.Strategy.DockerStrategy.BaseImage != "registry.com/namespace/imagename1" { + t.Errorf("Image substitutions not properly setup for new build. Expected %s, got %s |", "registry.com/namespace/imagename1", buildCreator.build.Parameters.Strategy.DockerStrategy.BaseImage) } + if buildConfigUpdater.buildcfg == nil { t.Fatal("Expected buildConfig update when new image was created!") } @@ -249,12 +234,9 @@ func TestBuildConfigWithDifferentTriggerType(t *testing.T) { buildCreator := controller.BuildCreator.(*mockBuildCreator) buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg != nil { + if buildCreator.build != nil { t.Error("New build created when a different trigger type was defined!") } - if len(buildCreator.imageSubstitutions) != 0 { - t.Errorf("Should not have had any image substitutions since different trigger type was defined!") - } if buildConfigUpdater.buildcfg != nil { t.Error("BuildConfig was updated when a different trigger was defined!") } @@ -270,12 +252,9 @@ func TestNoImageIDChange(t *testing.T) { buildCreator := controller.BuildCreator.(*mockBuildCreator) buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg != nil { + if buildCreator.build != nil { t.Error("New build created when no change happened!") } - if len(buildCreator.imageSubstitutions) != 0 { - t.Errorf("Should not have had any image substitutions since no change happened!") - } if buildConfigUpdater.buildcfg != nil { t.Error("BuildConfig was updated when no change happened!") } @@ -290,11 +269,11 @@ func TestBuildCreateError(t *testing.T) { controller.HandleImageRepo() buildConfigUpdater := controller.BuildConfigUpdater.(*mockBuildConfigUpdater) - if buildCreator.buildcfg == nil { + if buildCreator.build == nil { t.Error("Expected new build when new image was created!") } - if buildCreator.imageSubstitutions["registry.com/namespace/imagename"] != "registry.com/namespace/imagename:newImageID123" { - t.Errorf("Image substitutions not properly setup for new build: %s |", buildCreator.imageSubstitutions["registry.com/namespace/imagename"]) + if buildCreator.build.Parameters.Strategy.DockerStrategy.BaseImage != "registry.com/namespace/imagename:newImageID123" { + t.Errorf("Image substitutions not properly setup for new build. Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.BaseImage) } if buildConfigUpdater.buildcfg != nil { t.Fatal("Expected no buildConfig update on BuildCreate error!") diff --git a/pkg/build/util/generate.go b/pkg/build/util/generate.go index 88a8fe3a24e5..87d71183799d 100644 --- a/pkg/build/util/generate.go +++ b/pkg/build/util/generate.go @@ -4,35 +4,40 @@ import ( "github.com/golang/glog" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/openshift/origin/pkg/build/api" + buildapi "github.com/openshift/origin/pkg/build/api" ) // GenerateBuildFromConfig creates a new build based on a given BuildConfig. Optionally a SourceRevision for the new -// build can be specified -func GenerateBuildFromConfig(bc *api.BuildConfig, r *api.SourceRevision) (build *api.Build) { +// build can be specified. Also optionally a list of image names to be substituted can be supplied. Values in the BuildConfig +// that have a substitution provided will be replaced in the resulting Build +func GenerateBuildFromConfig(bc *buildapi.BuildConfig, r *buildapi.SourceRevision, imageSubstitutions map[string]string) (build *buildapi.Build) { // Need to copy the buildConfig here so that it doesn't share pointers with // the build object which could be (will be) modified later. obj, _ := kapi.Scheme.Copy(bc) - bcCopy := obj.(*api.BuildConfig) + bcCopy := obj.(*buildapi.BuildConfig) - return &api.Build{ - Parameters: api.BuildParameters{ + b := &buildapi.Build{ + Parameters: buildapi.BuildParameters{ Source: bcCopy.Parameters.Source, Strategy: bcCopy.Parameters.Strategy, Output: bcCopy.Parameters.Output, Revision: r, }, ObjectMeta: kapi.ObjectMeta{ - Labels: map[string]string{api.BuildConfigLabel: bcCopy.Name}, + Labels: map[string]string{buildapi.BuildConfigLabel: bcCopy.Name}, }, } + for originalImage, newImage := range imageSubstitutions { + SubstituteImageReferences(b, originalImage, newImage) + } + return b } // GenerateBuildFromBuild creates a new build based on a given Build. -func GenerateBuildFromBuild(build *api.Build) *api.Build { +func GenerateBuildFromBuild(build *buildapi.Build) *buildapi.Build { obj, _ := kapi.Scheme.Copy(build) - buildCopy := obj.(*api.Build) - return &api.Build{ + buildCopy := obj.(*buildapi.Build) + return &buildapi.Build{ Parameters: buildCopy.Parameters, ObjectMeta: kapi.ObjectMeta{ Labels: buildCopy.ObjectMeta.Labels, @@ -41,27 +46,27 @@ func GenerateBuildFromBuild(build *api.Build) *api.Build { } // SubstituteImageReferences replaces references to an image with a new value -func SubstituteImageReferences(build *api.Build, oldImage string, newImage string) { +func SubstituteImageReferences(build *buildapi.Build, oldImage string, newImage string) { switch { - case build.Parameters.Strategy.Type == api.DockerBuildStrategyType && + case build.Parameters.Strategy.Type == buildapi.DockerBuildStrategyType && build.Parameters.Strategy.DockerStrategy != nil && build.Parameters.Strategy.DockerStrategy.BaseImage == oldImage: build.Parameters.Strategy.DockerStrategy.BaseImage = newImage - case build.Parameters.Strategy.Type == api.STIBuildStrategyType && + case build.Parameters.Strategy.Type == buildapi.STIBuildStrategyType && build.Parameters.Strategy.STIStrategy != nil && build.Parameters.Strategy.STIStrategy.Image == oldImage: build.Parameters.Strategy.STIStrategy.Image = newImage - case build.Parameters.Strategy.Type == api.CustomBuildStrategyType: + case build.Parameters.Strategy.Type == buildapi.CustomBuildStrategyType: // update env variable references to the old image with the new image strategy := build.Parameters.Strategy.CustomStrategy if strategy.Env == nil { strategy.Env = make([]kapi.EnvVar, 1) - strategy.Env[0] = kapi.EnvVar{Name: api.CustomBuildStrategyBaseImageKey, Value: newImage} + strategy.Env[0] = kapi.EnvVar{Name: buildapi.CustomBuildStrategyBaseImageKey, Value: newImage} } else { found := false for i := range strategy.Env { glog.V(4).Infof("Checking env variable %s %s", strategy.Env[i].Name, strategy.Env[i].Value) - if strategy.Env[i].Name == api.CustomBuildStrategyBaseImageKey { + if strategy.Env[i].Name == buildapi.CustomBuildStrategyBaseImageKey { found = true if strategy.Env[i].Value == oldImage { strategy.Env[i].Value = newImage @@ -71,7 +76,7 @@ func SubstituteImageReferences(build *api.Build, oldImage string, newImage strin } } if !found { - strategy.Env = append(strategy.Env, kapi.EnvVar{Name: api.CustomBuildStrategyBaseImageKey, Value: newImage}) + strategy.Env = append(strategy.Env, kapi.EnvVar{Name: buildapi.CustomBuildStrategyBaseImageKey, Value: newImage}) } } // update the actual custom build image with the new image, if applicable diff --git a/pkg/build/util/generate_test.go b/pkg/build/util/generate_test.go index 10fea89fdcd8..fccce15495fd 100644 --- a/pkg/build/util/generate_test.go +++ b/pkg/build/util/generate_test.go @@ -38,7 +38,7 @@ func TestGenerateBuildFromConfig(t *testing.T) { Commit: "abcd", }, } - build := GenerateBuildFromConfig(bc, revision) + build := GenerateBuildFromConfig(bc, revision, nil) if !reflect.DeepEqual(source, build.Parameters.Source) { t.Errorf("Build source does not match BuildConfig source") } @@ -88,7 +88,7 @@ func TestSubstituteImageDockerNil(t *testing.T) { strategy := mockDockerStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // Docker build with nil base image // base image should still be nil @@ -103,7 +103,7 @@ func TestSubstituteImageDockerMatch(t *testing.T) { strategy := mockDockerStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // Docker build with a matched base image // base image should be replaced. @@ -122,7 +122,7 @@ func TestSubstituteImageDockerMismatch(t *testing.T) { strategy := mockDockerStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // Docker build with an unmatched base image // base image should not be replaced. @@ -137,7 +137,7 @@ func TestSubstituteImageSTIMatch(t *testing.T) { strategy := mockSTIStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // STI build with a matched base image // base image should be replaced @@ -156,7 +156,7 @@ func TestSubstituteImageSTIMismatch(t *testing.T) { strategy := mockSTIStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // STI build with an unmatched base image // base image should not be replaced @@ -171,7 +171,7 @@ func TestSubstituteImageCustomAllMatch(t *testing.T) { strategy := mockCustomStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // Full custom build with a BaseImage and a well defined environment variable image value, // both should be replaced. Additional environment variables should not be touched. @@ -205,7 +205,7 @@ func TestSubstituteImageCustomAllMismatch(t *testing.T) { strategy := mockCustomStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // Full custom build with base image that is not matched // Base image name should be unchanged @@ -220,7 +220,7 @@ func TestSubstituteImageCustomBaseMatchEnvMismatch(t *testing.T) { strategy := mockCustomStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // Full custom build with a BaseImage and a well defined environment variable image value that does not match the new image // Only base image should be replaced. Environment variables should not be touched. @@ -247,7 +247,7 @@ func TestSubstituteImageCustomBaseMatchEnvMissing(t *testing.T) { strategy := mockCustomStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // Custom build with a base Image but no image environment variable. // base image should be replaced, new image environment variable should be added, @@ -274,7 +274,7 @@ func TestSubstituteImageCustomBaseMatchEnvNil(t *testing.T) { strategy := mockCustomStrategy() output := mockOutput() bc := mockBuildConfig(source, strategy, output) - build := GenerateBuildFromConfig(bc, nil) + build := GenerateBuildFromConfig(bc, nil, nil) // Custom build with a base Image but no environment variables // base image should be replaced, new image environment variable should be added diff --git a/pkg/build/webhook/controller.go b/pkg/build/webhook/controller.go index 2bbdfa5f7e4e..4898c116e2f0 100644 --- a/pkg/build/webhook/controller.go +++ b/pkg/build/webhook/controller.go @@ -7,6 +7,7 @@ import ( kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/openshift/origin/pkg/build/api" + buildclient "github.com/openshift/origin/pkg/build/client" "github.com/openshift/origin/pkg/build/util" ) @@ -23,13 +24,9 @@ type Plugin interface { // controller used for processing webhook requests. type controller struct { - osClient webhookBuildInterface - plugins map[string]Plugin -} - -type webhookBuildInterface interface { - CreateBuild(namespace string, build *api.Build) (*api.Build, error) - GetBuildConfig(namespace, name string) (*api.BuildConfig, error) + buildCreator buildclient.BuildCreator + buildConfigGetter buildclient.BuildConfigGetter + plugins map[string]Plugin } // urlVars holds parsed URL parts. @@ -42,8 +39,12 @@ type urlVars struct { } // NewController creates new webhook controller and feed it with provided plugins. -func NewController(osClient webhookBuildInterface, plugins map[string]Plugin) http.Handler { - return &controller{osClient: osClient, plugins: plugins} +func NewController(buildConfigGetter buildclient.BuildConfigGetter, buildCreator buildclient.BuildCreator, plugins map[string]Plugin) http.Handler { + return &controller{ + buildConfigGetter: buildConfigGetter, + buildCreator: buildCreator, + plugins: plugins, + } } // ServeHTTP main REST service method. @@ -54,7 +55,7 @@ func (c *controller) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } - buildCfg, err := c.osClient.GetBuildConfig(uv.namespace, uv.buildConfigName) + buildCfg, err := c.buildConfigGetter.Get(uv.namespace, uv.buildConfigName) if err != nil { badRequest(w, err.Error()) return @@ -73,8 +74,9 @@ func (c *controller) ServeHTTP(w http.ResponseWriter, req *http.Request) { if !proceed { return } - build := util.GenerateBuildFromConfig(buildCfg, revision) - if _, err := c.osClient.CreateBuild(uv.namespace, build); err != nil { + build := util.GenerateBuildFromConfig(buildCfg, revision, nil) + + if err := c.buildCreator.Create(uv.namespace, build); err != nil { badRequest(w, err.Error()) } } diff --git a/pkg/build/webhook/controller_test.go b/pkg/build/webhook/controller_test.go index 55d90e1174d4..93be8b36a217 100644 --- a/pkg/build/webhook/controller_test.go +++ b/pkg/build/webhook/controller_test.go @@ -11,9 +11,9 @@ import ( "github.com/openshift/origin/pkg/build/api" ) -type okClient struct{} +type okBuildConfigGetter struct{} -func (*okClient) GetBuildConfig(namespace, name string) (*api.BuildConfig, error) { +func (*okBuildConfigGetter) Get(namespace, name string) (*api.BuildConfig, error) { return &api.BuildConfig{ Triggers: []api.BuildTriggerPolicy{ { @@ -26,23 +26,21 @@ func (*okClient) GetBuildConfig(namespace, name string) (*api.BuildConfig, error }, nil } -func (*okClient) CreateBuild(namespace string, build *api.Build) (*api.Build, error) { - return &api.Build{}, nil -} +type okBuildCreator struct{} -type buildErrorClient struct { - okClient +func (*okBuildCreator) Create(namespace string, build *api.Build) error { + return nil } -func (*buildErrorClient) CreateBuild(namespace string, build *api.Build) (*api.Build, error) { - return &api.Build{}, errors.New("Build error!") -} +type errorBuildCreator struct{} -type configErrorClient struct { - okClient +func (*errorBuildCreator) Create(namespace string, build *api.Build) error { + return errors.New("Build error!") } -func (*configErrorClient) GetBuildConfig(namespace, name string) (*api.BuildConfig, error) { +type errorBuildConfigGetter struct{} + +func (*errorBuildConfigGetter) Get(namespace, name string) (*api.BuildConfig, error) { return &api.BuildConfig{}, errors.New("BuildConfig error!") } @@ -62,7 +60,7 @@ func (*errPlugin) Extract(buildCfg *api.BuildConfig, secret, path string, req *h } func TestParseUrlError(t *testing.T) { - server := httptest.NewServer(NewController(&okClient{}, nil)) + server := httptest.NewServer(NewController(&okBuildConfigGetter{}, &okBuildCreator{}, nil)) defer server.Close() resp, err := http.Post(server.URL, "application/json", nil) @@ -77,7 +75,7 @@ func TestParseUrlError(t *testing.T) { } func TestParseUrlOK(t *testing.T) { - server := httptest.NewServer(NewController(&okClient{}, map[string]Plugin{ + server := httptest.NewServer(NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]Plugin{ "pathplugin": &pathPlugin{}, })) defer server.Close() @@ -96,7 +94,7 @@ func TestParseUrlOK(t *testing.T) { func TestParseUrlLong(t *testing.T) { plugin := &pathPlugin{} - server := httptest.NewServer(NewController(&okClient{}, map[string]Plugin{ + server := httptest.NewServer(NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]Plugin{ "pathplugin": plugin, })) defer server.Close() @@ -117,7 +115,7 @@ func TestParseUrlLong(t *testing.T) { } func TestInvokeWebhookErrorSecret(t *testing.T) { - server := httptest.NewServer(NewController(&okClient{}, nil)) + server := httptest.NewServer(NewController(&okBuildConfigGetter{}, &okBuildCreator{}, nil)) defer server.Close() resp, err := http.Post(server.URL+"/build100/wrongsecret/somePlugin", @@ -133,7 +131,7 @@ func TestInvokeWebhookErrorSecret(t *testing.T) { } func TestInvokeWebhookMissingPlugin(t *testing.T) { - server := httptest.NewServer(NewController(&okClient{}, nil)) + server := httptest.NewServer(NewController(&okBuildConfigGetter{}, &okBuildCreator{}, nil)) defer server.Close() resp, err := http.Post(server.URL+"/build100/secret101/missingplugin", @@ -150,7 +148,7 @@ func TestInvokeWebhookMissingPlugin(t *testing.T) { } func TestInvokeWebhookErrorBuildConfig(t *testing.T) { - server := httptest.NewServer(NewController(&buildErrorClient{}, map[string]Plugin{ + server := httptest.NewServer(NewController(&okBuildConfigGetter{}, &errorBuildCreator{}, map[string]Plugin{ "okPlugin": &pathPlugin{}, })) defer server.Close() @@ -169,7 +167,7 @@ func TestInvokeWebhookErrorBuildConfig(t *testing.T) { } func TestInvokeWebhookErrorGetConfig(t *testing.T) { - server := httptest.NewServer(NewController(&configErrorClient{}, nil)) + server := httptest.NewServer(NewController(&errorBuildConfigGetter{}, &okBuildCreator{}, nil)) defer server.Close() resp, err := http.Post(server.URL+"/build100/secret101/errPlugin", @@ -186,7 +184,7 @@ func TestInvokeWebhookErrorGetConfig(t *testing.T) { } func TestInvokeWebhookErrorCreateBuild(t *testing.T) { - server := httptest.NewServer(NewController(&okClient{}, map[string]Plugin{ + server := httptest.NewServer(NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]Plugin{ "errPlugin": &errPlugin{}, })) defer server.Close() @@ -204,19 +202,25 @@ func TestInvokeWebhookErrorCreateBuild(t *testing.T) { } } -type mockOkClient struct { +type mockOkBuildCreator struct { testBuildInterface } - type testBuildInterface struct { - CreateBuildFunc func(namespace string, build *api.Build) (*api.Build, error) - GetBuildConfigFunc func(namespace, name string) (*api.BuildConfig, error) + CreateFunc func(namespace string, build *api.Build) error +} + +func (i *testBuildInterface) Create(namespace string, build *api.Build) error { + return i.CreateFunc(namespace, build) } -func (i *testBuildInterface) CreateBuild(namespace string, build *api.Build) (*api.Build, error) { - return i.CreateBuildFunc(namespace, build) +type mockOkBuildConfigGetter struct { + testBuildConfigInterface +} +type testBuildConfigInterface struct { + GetBuildConfigFunc func(namespace, name string) (*api.BuildConfig, error) } -func (i *testBuildInterface) GetBuildConfig(namespace, name string) (*api.BuildConfig, error) { + +func (i *testBuildConfigInterface) Get(namespace, name string) (*api.BuildConfig, error) { return i.GetBuildConfigFunc(namespace, name) } @@ -226,20 +230,27 @@ func TestInvokeWebhookOk(t *testing.T) { Parameters: api.BuildParameters{}, } - server := httptest.NewServer(NewController(&mockOkClient{ - testBuildInterface: testBuildInterface{ - CreateBuildFunc: func(namespace string, build *api.Build) (*api.Build, error) { - buildRequest = build - return build, nil + server := httptest.NewServer(NewController( + &mockOkBuildConfigGetter{ + testBuildConfigInterface: testBuildConfigInterface{ + GetBuildConfigFunc: func(namespace, name string) (*api.BuildConfig, error) { + buildConfig.Name = name + return buildConfig, nil + }, }, - GetBuildConfigFunc: func(namespace, name string) (*api.BuildConfig, error) { - buildConfig.Name = name - return buildConfig, nil + }, + &mockOkBuildCreator{ + testBuildInterface: testBuildInterface{ + CreateFunc: func(namespace string, build *api.Build) error { + buildRequest = build + return nil + }, }, }, - }, map[string]Plugin{ - "okPlugin": &pathPlugin{}, - })) + + map[string]Plugin{ + "okPlugin": &pathPlugin{}, + })) defer server.Close() resp, err := http.Post(server.URL+"/build100/secret101/okPlugin", @@ -253,6 +264,6 @@ func TestInvokeWebhookOk(t *testing.T) { string(body)) } if e, a := buildConfig.Name, buildRequest.Labels[api.BuildConfigLabel]; e != a { - t.Fatalf("expected build with label '%s', got '%s'", e, a) + t.Fatalf("expected buildconfig names to match '%s', got '%s'", e, a) } } diff --git a/pkg/build/webhook/github/github_test.go b/pkg/build/webhook/github/github_test.go index 5d0627f7eb8a..d617c55cee6d 100644 --- a/pkg/build/webhook/github/github_test.go +++ b/pkg/build/webhook/github/github_test.go @@ -12,9 +12,9 @@ import ( "github.com/openshift/origin/pkg/build/webhook" ) -type okClient struct{} +type okBuildConfigGetter struct{} -func (c *okClient) GetBuildConfig(namespace, name string) (*api.BuildConfig, error) { +func (c *okBuildConfigGetter) Get(namespace, name string) (*api.BuildConfig, error) { return &api.BuildConfig{ Triggers: []api.BuildTriggerPolicy{ { @@ -35,12 +35,14 @@ func (c *okClient) GetBuildConfig(namespace, name string) (*api.BuildConfig, err }, nil } -func (c *okClient) CreateBuild(namespace string, build *api.Build) (*api.Build, error) { - return &api.Build{}, nil +type okBuildCreator struct{} + +func (c *okBuildCreator) Create(namespace string, build *api.Build) error { + return nil } func TestWrongMethod(t *testing.T) { - server := httptest.NewServer(webhook.NewController(&okClient{}, map[string]webhook.Plugin{"github": New()})) + server := httptest.NewServer(webhook.NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]webhook.Plugin{"github": New()})) defer server.Close() resp, _ := http.Get(server.URL + "/build100/secret101/github") @@ -52,7 +54,7 @@ func TestWrongMethod(t *testing.T) { } func TestWrongContentType(t *testing.T) { - server := httptest.NewServer(webhook.NewController(&okClient{}, map[string]webhook.Plugin{"github": New()})) + server := httptest.NewServer(webhook.NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]webhook.Plugin{"github": New()})) defer server.Close() client := &http.Client{} @@ -69,7 +71,7 @@ func TestWrongContentType(t *testing.T) { } func TestWrongUserAgent(t *testing.T) { - server := httptest.NewServer(webhook.NewController(&okClient{}, map[string]webhook.Plugin{"github": New()})) + server := httptest.NewServer(webhook.NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]webhook.Plugin{"github": New()})) defer server.Close() client := &http.Client{} @@ -86,7 +88,7 @@ func TestWrongUserAgent(t *testing.T) { } func TestMissingGithubEvent(t *testing.T) { - server := httptest.NewServer(webhook.NewController(&okClient{}, map[string]webhook.Plugin{"github": New()})) + server := httptest.NewServer(webhook.NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]webhook.Plugin{"github": New()})) defer server.Close() client := &http.Client{} @@ -102,7 +104,7 @@ func TestMissingGithubEvent(t *testing.T) { } func TestWrongGithubEvent(t *testing.T) { - server := httptest.NewServer(webhook.NewController(&okClient{}, map[string]webhook.Plugin{"github": New()})) + server := httptest.NewServer(webhook.NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]webhook.Plugin{"github": New()})) defer server.Close() client := &http.Client{} @@ -119,7 +121,7 @@ func TestWrongGithubEvent(t *testing.T) { } func TestJsonPingEvent(t *testing.T) { - server := httptest.NewServer(webhook.NewController(&okClient{}, map[string]webhook.Plugin{"github": New()})) + server := httptest.NewServer(webhook.NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]webhook.Plugin{"github": New()})) defer server.Close() postFile("ping", "pingevent.json", server.URL+"/build100/secret101/github", @@ -127,14 +129,14 @@ func TestJsonPingEvent(t *testing.T) { } func TestJsonPushEventError(t *testing.T) { - server := httptest.NewServer(webhook.NewController(&okClient{}, map[string]webhook.Plugin{"github": New()})) + server := httptest.NewServer(webhook.NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]webhook.Plugin{"github": New()})) defer server.Close() post("push", []byte{}, server.URL+"/build100/secret101/github", http.StatusBadRequest, t) } func TestJsonPushEvent(t *testing.T) { - server := httptest.NewServer(webhook.NewController(&okClient{}, map[string]webhook.Plugin{"github": New()})) + server := httptest.NewServer(webhook.NewController(&okBuildConfigGetter{}, &okBuildCreator{}, map[string]webhook.Plugin{"github": New()})) defer server.Close() postFile("push", "pushevent.json", server.URL+"/build100/secret101/github", diff --git a/pkg/cmd/cli/cmd/startbuild.go b/pkg/cmd/cli/cmd/startbuild.go index fe954b1dcb77..107183582eea 100644 --- a/pkg/cmd/cli/cmd/startbuild.go +++ b/pkg/cmd/cli/cmd/startbuild.go @@ -44,7 +44,7 @@ Examples: config, err := client.BuildConfigs(namespace).Get(args[0]) checkErr(err) - newBuild = util.GenerateBuildFromConfig(config, nil) + newBuild = util.GenerateBuildFromConfig(config, nil, nil) } else { build, err := client.Builds(namespace).Get(buildName) checkErr(err) diff --git a/pkg/cmd/client/kubecfg.go b/pkg/cmd/client/kubecfg.go index 27b28248e63a..e79c546deacf 100644 --- a/pkg/cmd/client/kubecfg.go +++ b/pkg/cmd/client/kubecfg.go @@ -531,7 +531,7 @@ func (c *KubeConfig) executeBuildRequest(method string, client *osclient.Client) if err != nil { glog.Fatalf("failed to trigger build manually: %v", err) } - build = buildutil.GenerateBuildFromConfig(buildConfig, buildConfig.Parameters.Revision) + build = buildutil.GenerateBuildFromConfig(buildConfig, buildConfig.Parameters.Revision, nil) } request := client.Post().Namespace(c.getNamespace()).Resource("/builds").Body(build) if err := request.Do().Error(); err != nil { diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go index 9343d53a005c..c018f476d375 100644 --- a/pkg/cmd/server/origin/master.go +++ b/pkg/cmd/server/origin/master.go @@ -31,7 +31,7 @@ import ( "github.com/openshift/origin/pkg/auth/authenticator" authcontext "github.com/openshift/origin/pkg/auth/context" authfilter "github.com/openshift/origin/pkg/auth/handlers" - buildapi "github.com/openshift/origin/pkg/build/api" + buildclient "github.com/openshift/origin/pkg/build/client" buildcontrollerfactory "github.com/openshift/origin/pkg/build/controller/factory" buildstrategy "github.com/openshift/origin/pkg/build/controller/strategy" buildregistry "github.com/openshift/origin/pkg/build/registry/build" @@ -148,24 +148,37 @@ func (c *MasterConfig) BuildClients() { c.osClient = osclient } +// KubeClient returns the kubernetes client object func (c *MasterConfig) KubeClient() *kclient.Client { return c.kubeClient } + +// DeploymentClient returns the deployment client object func (c *MasterConfig) DeploymentClient() *kclient.Client { return c.kubeClient } + +// BuildLogClient returns the build log client object func (c *MasterConfig) BuildLogClient() *kclient.Client { return c.kubeClient } + +// WebHookClient returns the webhook client object func (c *MasterConfig) WebHookClient() *osclient.Client { return c.osClient } + +// BuildControllerClients returns the build controller client objects func (c *MasterConfig) BuildControllerClients() (*osclient.Client, *kclient.Client) { return c.osClient, c.kubeClient } + +// ImageChangeControllerClient returns the openshift client object func (c *MasterConfig) ImageChangeControllerClient() *osclient.Client { return c.osClient } + +// DeploymentControllerClients returns the deployment controller client object func (c *MasterConfig) DeploymentControllerClients() (*osclient.Client, *kclient.Client) { return c.osClient, c.kubeClient } @@ -263,8 +276,9 @@ func (c *MasterConfig) InstallAPI(container *restful.Container) []string { } whPrefix := OpenShiftAPIPrefixV1Beta1 + "/buildConfigHooks/" + bcClient, _ := c.BuildControllerClients() container.ServeMux.Handle(whPrefix, http.StripPrefix(whPrefix, - webhook.NewController(ClientWebhookInterface{c.WebHookClient()}, map[string]webhook.Plugin{ + webhook.NewController(buildclient.NewOSClientBuildConfigClient(bcClient), buildclient.NewOSClientBuildClient(bcClient), map[string]webhook.Plugin{ "generic": generic.New(), "github": github.New(), }))) @@ -487,8 +501,9 @@ func (c *MasterConfig) RunBuildController() { osclient, kclient := c.BuildControllerClients() factory := buildcontrollerfactory.BuildControllerFactory{ - Client: osclient, - KubeClient: kclient, + OSClient: osclient, + KubeClient: kclient, + BuildUpdater: buildclient.NewOSClientBuildClient(osclient), DockerBuildStrategy: &buildstrategy.DockerBuildStrategy{ Image: dockerImage, UseLocalImages: useLocalImages, @@ -515,7 +530,10 @@ func (c *MasterConfig) RunBuildController() { // RunDeploymentController starts the build image change trigger controller process. func (c *MasterConfig) RunBuildImageChangeTriggerController() { - factory := buildcontrollerfactory.ImageChangeControllerFactory{Client: c.ImageChangeControllerClient()} + bcClient, _ := c.BuildControllerClients() + bcUpdater := buildclient.NewOSClientBuildConfigClient(bcClient) + bCreator := buildclient.NewOSClientBuildClient(bcClient) + factory := buildcontrollerfactory.ImageChangeControllerFactory{Client: bcClient, BuildCreator: bCreator, BuildConfigUpdater: bcUpdater} factory.Create().Run() } @@ -592,21 +610,6 @@ func env(key string, defaultValue string) string { return val } -// ClientWebhookInterface is a webhookBuildInterface which delegates to the OpenShift client interfaces -type ClientWebhookInterface struct { - Client osclient.Interface -} - -// CreateBuild creates build using OpenShift client. -func (c ClientWebhookInterface) CreateBuild(namespace string, build *buildapi.Build) (*buildapi.Build, error) { - return c.Client.Builds(namespace).Create(build) -} - -// GetBuildConfig returns buildConfig using OpenShift client. -func (c ClientWebhookInterface) GetBuildConfig(namespace, name string) (*buildapi.BuildConfig, error) { - return c.Client.BuildConfigs(namespace).Get(name) -} - type clientDeploymentInterface struct { KubeClient kclient.Interface } diff --git a/test/integration/buildclient_test.go b/test/integration/buildclient_test.go index c70e72f8818b..fb8d62d37c0c 100644 --- a/test/integration/buildclient_test.go +++ b/test/integration/buildclient_test.go @@ -19,6 +19,7 @@ import ( "github.com/openshift/origin/pkg/api/latest" buildapi "github.com/openshift/origin/pkg/build/api" + buildclient "github.com/openshift/origin/pkg/build/client" buildcontrollerfactory "github.com/openshift/origin/pkg/build/controller/factory" buildstrategy "github.com/openshift/origin/pkg/build/controller/strategy" buildregistry "github.com/openshift/origin/pkg/build/registry/build" @@ -195,13 +196,14 @@ func NewTestBuildOpenshift(t *testing.T) *testBuildOpenshift { openshift.whPrefix = osPrefix + "/buildConfigHooks/" osMux.Handle(openshift.whPrefix, http.StripPrefix(openshift.whPrefix, - webhook.NewController(clientWebhookInterface{osClient}, map[string]webhook.Plugin{ + webhook.NewController(buildclient.NewOSClientBuildConfigClient(osClient), buildclient.NewOSClientBuildClient(osClient), map[string]webhook.Plugin{ "github": github.New(), }))) factory := buildcontrollerfactory.BuildControllerFactory{ - Client: osClient, - KubeClient: kubeClient, + OSClient: osClient, + KubeClient: kubeClient, + BuildUpdater: buildclient.NewOSClientBuildClient(osClient), DockerBuildStrategy: &buildstrategy.DockerBuildStrategy{ Image: "test-docker-builder", UseLocalImages: false, @@ -226,15 +228,3 @@ func (t *testBuildOpenshift) Close() { t.server.CloseClientConnections() t.server.Close() } - -type clientWebhookInterface struct { - Client osclient.Interface -} - -func (c clientWebhookInterface) CreateBuild(namespace string, build *buildapi.Build) (*buildapi.Build, error) { - return c.Client.Builds(namespace).Create(build) -} - -func (c clientWebhookInterface) GetBuildConfig(namespace, name string) (*buildapi.BuildConfig, error) { - return c.Client.BuildConfigs(namespace).Get(name) -} diff --git a/test/integration/deploy_trigger_test.go b/test/integration/deploy_trigger_test.go index 9ce975d95cbc..84c1ab6a691b 100644 --- a/test/integration/deploy_trigger_test.go +++ b/test/integration/deploy_trigger_test.go @@ -21,8 +21,8 @@ import ( "github.com/openshift/origin/pkg/api/latest" "github.com/openshift/origin/pkg/api/v1beta1" + buildclient "github.com/openshift/origin/pkg/build/client" buildcontrollerfactory "github.com/openshift/origin/pkg/build/controller/factory" - buildstrategy "github.com/openshift/origin/pkg/build/controller/strategy" buildregistry "github.com/openshift/origin/pkg/build/registry/build" buildconfigregistry "github.com/openshift/origin/pkg/build/registry/buildconfig" buildetcd "github.com/openshift/origin/pkg/build/registry/etcd" @@ -402,29 +402,13 @@ func NewTestOpenshift(t *testing.T) *testOpenshift { iccFactory.Create().Run() biccFactory := buildcontrollerfactory.ImageChangeControllerFactory{ - Client: osClient, - Stop: openshift.stop, + Client: osClient, + BuildConfigUpdater: buildclient.NewOSClientBuildConfigClient(osClient), + BuildCreator: buildclient.NewOSClientBuildClient(osClient), + Stop: openshift.stop, } biccFactory.Create().Run() - - bcFactory := buildcontrollerfactory.BuildControllerFactory{ - Client: osClient, - KubeClient: kubeClient, - DockerBuildStrategy: &buildstrategy.DockerBuildStrategy{ - Image: "test-docker-builder", - UseLocalImages: false, - }, - STIBuildStrategy: &buildstrategy.STIBuildStrategy{ - Image: "test-sti-builder", - TempDirectoryCreator: buildstrategy.STITempDirectoryCreator, - UseLocalImages: false, - }, - Stop: openshift.stop, - } - - bcFactory.Create().Run() - return openshift }