Refactor Build Controller to use Informers

pkg/authorization/authorizer/subjects_test.go

@@ -50,7 +50,6 @@ func TestSubjects(t *testing.T) {
 			"system:serviceaccount:openshift-infra:deployer-controller",
 			"system:serviceaccount:openshift-infra:template-instance-controller",
 			"system:serviceaccount:openshift-infra:template-instance-controller",
-			"system:serviceaccount:openshift-infra:build-pod-controller",
 			"system:serviceaccount:openshift-infra:build-controller",
 		),
 		expectedGroups: sets.NewString("RootUsers", "system:cluster-admins", "system:cluster-readers", "system:masters", "system:nodes"),

pkg/build/api/types.go

@@ -475,6 +475,14 @@ const (
 	// build pod but one was already present.
 	StatusReasonBuildPodExists StatusReason = "BuildPodExists"
 
+	// StatusReasonNoBuildContainerStatus indicates that the build failed because the
+	// the build pod has no container statuses.
+	StatusReasonNoBuildContainerStatus StatusReason = "NoBuildContainerStatus"
+
+	// StatusReasonFailedContainer indicates that the pod for the build has at least
+	// one container with a non-zero exit status.
+	StatusReasonFailedContainer StatusReason = "FailedContainer"
+
 	// StatusReasonGenericBuildFailed is the reason associated with a broad
 	// range of build failures.
 	StatusReasonGenericBuildFailed StatusReason = "GenericBuildFailed"
@@ -497,6 +505,8 @@ const (
 	StatusMessageCancelledBuild            = "The build was cancelled by the user."
 	StatusMessageDockerBuildFailed         = "Docker build strategy has failed."
 	StatusMessageBuildPodExists            = "The pod for this build already exists and is older than the build."
+	StatusMessageNoBuildContainerStatus    = "The pod for this build has no container statuses indicating success or failure."
+	StatusMessageFailedContainer           = "The pod for this build has at least one container with a non-zero exit status."
 	StatusMessageGenericBuildFailed        = "Generic Build failure - check logs for details."
 )
 

pkg/build/api/validation/validation.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/golang/glog"
 
-	"github.com/openshift/origin/pkg/util/labelselector"
 	kpath "k8s.io/apimachinery/pkg/api/validation/path"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
@@ -23,6 +22,7 @@ import (
 	buildutil "github.com/openshift/origin/pkg/build/util"
 	imageapi "github.com/openshift/origin/pkg/image/api"
 	imageapivalidation "github.com/openshift/origin/pkg/image/api/validation"
+	"github.com/openshift/origin/pkg/util/labelselector"
 )
 
 // ValidateBuild tests required fields for a Build.
@@ -81,26 +81,6 @@ func ValidateBuildUpdate(build *buildapi.Build, older *buildapi.Build) field.Err
 	return allErrs
 }
 
-func diffBuildSpec(newer buildapi.BuildSpec, older buildapi.BuildSpec) (string, error) {
-	codec := kapi.Codecs.LegacyCodec(v1.LegacySchemeGroupVersion)
-	newerObj := &buildapi.Build{Spec: newer}
-	olderObj := &buildapi.Build{Spec: older}
-
-	newerJSON, err := runtime.Encode(codec, newerObj)
-	if err != nil {
-		return "", fmt.Errorf("error encoding newer: %v", err)
-	}
-	olderJSON, err := runtime.Encode(codec, olderObj)
-	if err != nil {
-		return "", fmt.Errorf("error encoding older: %v", err)
-	}
-	patch, err := strategicpatch.CreateTwoWayMergePatch(olderJSON, newerJSON, &v1.Build{})
-	if err != nil {
-		return "", fmt.Errorf("error creating a strategic patch: %v", err)
-	}
-	return string(patch), nil
-}
-
 // refKey returns a key for the given ObjectReference. If the ObjectReference
 // doesn't include a namespace, the passed in namespace is used for the reference
 func refKey(namespace string, ref *kapi.ObjectReference) string {
@@ -752,3 +732,56 @@ func ValidateNodeSelector(nodeSelector map[string]string, fldPath *field.Path) f
 	}
 	return allErrs
 }
+
+func diffBuildSpec(newer, older buildapi.BuildSpec) (string, error) {
+	newerObj := &buildapi.Build{Spec: newer}
+	olderObj := &buildapi.Build{Spec: older}
+	diffBytes, err := CreateBuildPatch(olderObj, newerObj)
+	if err != nil {
+		return "", err
+	}
+	return string(diffBytes), nil
+}
+
+func CreateBuildPatch(older, newer *buildapi.Build) ([]byte, error) {
+	codec := kapi.Codecs.LegacyCodec(v1.LegacySchemeGroupVersion)
+
+	newerJSON, err := runtime.Encode(codec, newer)
+	if err != nil {
+		return nil, fmt.Errorf("error encoding newer: %v", err)
+	}
+	olderJSON, err := runtime.Encode(codec, older)
+	if err != nil {
+		return nil, fmt.Errorf("error encoding older: %v", err)
+	}
+	patch, err := strategicpatch.CreateTwoWayMergePatch(olderJSON, newerJSON, &v1.Build{})
+	if err != nil {
+		return nil, fmt.Errorf("error creating a strategic patch: %v", err)
+	}
+	return patch, nil
+}
+
+func ApplyBuildPatch(build *buildapi.Build, patch []byte) (*buildapi.Build, error) {
+	codec := kapi.Codecs.LegacyCodec(v1.LegacySchemeGroupVersion)
+	versionedBuild, err := kapi.Scheme.ConvertToVersion(build, v1.SchemeGroupVersion)
+	if err != nil {
+		return nil, err
+	}
+	buildJSON, err := runtime.Encode(codec, versionedBuild)
+	if err != nil {
+		return nil, err
+	}
+	patchedJSON, err := strategicpatch.StrategicMergePatch(buildJSON, patch, &v1.Build{})
+	if err != nil {
+		return nil, err
+	}
+	patchedVersionedBuild, err := runtime.Decode(codec, patchedJSON)
+	if err != nil {
+		return nil, err
+	}
+	patchedBuild, err := kapi.Scheme.ConvertToVersion(patchedVersionedBuild, buildapi.SchemeGroupVersion)
+	if err != nil {
+		return nil, err
+	}
+	return patchedBuild.(*buildapi.Build), nil
+}

pkg/build/client/clients.go

@@ -1,9 +1,11 @@
 package client
 
 import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+
 	buildapi "github.com/openshift/origin/pkg/build/api"
 	osclient "github.com/openshift/origin/pkg/client"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // BuildConfigGetter provides methods for getting BuildConfigs
@@ -42,11 +44,21 @@ type BuildUpdater interface {
 	Update(namespace string, build *buildapi.Build) error
 }
 
+type BuildPatcher interface {
+	Patch(namespace, name string, patch []byte) (*buildapi.Build, error)
+}
+
 // BuildLister provides methods for listing the Builds.
 type BuildLister interface {
 	List(namespace string, opts metav1.ListOptions) (*buildapi.BuildList, error)
 }
 
+// BuildDeleter knows how to delete builds from OpenShift.
+type BuildDeleter interface {
+	// DeleteBuild removes the build from OpenShift's storage.
+	DeleteBuild(build *buildapi.Build) error
+}
+
 // OSClientBuildClient delegates build create and update operations to the OpenShift client interface
 type OSClientBuildClient struct {
 	Client osclient.Interface
@@ -63,11 +75,21 @@ func (c OSClientBuildClient) Update(namespace string, build *buildapi.Build) err
 	return e
 }
 
+// Patch patches builds using the OpenShift client.
+func (c OSClientBuildClient) Patch(namespace, name string, patch []byte) (*buildapi.Build, error) {
+	return c.Client.Builds(namespace).Patch(name, types.StrategicMergePatchType, patch)
+}
+
 // List lists the builds using the OpenShift client.
 func (c OSClientBuildClient) List(namespace string, opts metav1.ListOptions) (*buildapi.BuildList, error) {
 	return c.Client.Builds(namespace).List(opts)
 }
 
+// DeleteBuild deletes a build from OpenShift.
+func (c OSClientBuildClient) DeleteBuild(build *buildapi.Build) error {
+	return c.Client.Builds(build.Namespace).Delete(build.Name)
+}
+
 // BuildCloner provides methods for cloning builds
 type BuildCloner interface {
 	Clone(namespace string, request *buildapi.BuildRequest) (*buildapi.Build, error)
@@ -88,29 +110,6 @@ func (c OSClientBuildClonerClient) Clone(namespace string, request *buildapi.Bui
 	return c.Client.Builds(namespace).Clone(request)
 }
 
-// BuildDeleter knows how to delete builds from OpenShift.
-type BuildDeleter interface {
-	// DeleteBuild removes the build from OpenShift's storage.
-	DeleteBuild(build *buildapi.Build) error
-}
-
-// buildDeleter removes a build from OpenShift.
-type buildDeleter struct {
-	builds osclient.BuildsNamespacer
-}
-
-// NewBuildDeleter creates a new buildDeleter.
-func NewBuildDeleter(builds osclient.BuildsNamespacer) BuildDeleter {
-	return &buildDeleter{
-		builds: builds,
-	}
-}
-
-// DeleteBuild deletes a build from OpenShift.
-func (p *buildDeleter) DeleteBuild(build *buildapi.Build) error {
-	return p.builds.Builds(build.Namespace).Delete(build.Name)
-}
-
 // BuildConfigInstantiator provides methods for instantiating builds from build configs
 type BuildConfigInstantiator interface {
 	Instantiate(namespace string, request *buildapi.BuildRequest) (*buildapi.Build, error)