Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/build/builder/util.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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,
Expand Down
69 changes: 69 additions & 0 deletions pkg/build/client/clients.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package client
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd propose renaming the file to clients.go, similar to what we have in pkg/client/client.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since there are multiple clients in this file, i think i prefer it as is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more of a convention thing. If you have one file in a package, and you want to call the package a certain something, you generally call the only file in the package the same as the package. That's usually the stronger convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so as soon as we add a second file to this package, the name would be ok, right?


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
}
13 changes: 5 additions & 8 deletions pkg/build/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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
}

Expand Down
13 changes: 7 additions & 6 deletions pkg/build/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -316,7 +317,7 @@ func TestHandlePod(t *testing.T) {
outStatus buildapi.BuildStatus
podStatus kapi.PodPhase
exitCode int
buildUpdater buildUpdater
buildUpdater buildclient.BuildUpdater
}

tests := []handlePodTest{
Expand Down
48 changes: 13 additions & 35 deletions pkg/build/controller/factory/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good.

KubeClient kclient.Interface
BuildUpdater buildclient.BuildUpdater
DockerBuildStrategy *strategy.DockerBuildStrategy
STIBuildStrategy *strategy.STIBuildStrategy
CustomBuildStrategy *strategy.CustomBuildStrategy
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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{}
}
Expand All @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
24 changes: 10 additions & 14 deletions pkg/build/controller/image_change_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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)
}
}
Expand Down
Loading