Merge pull request #9527 from bparees/bc_label_policy

Merged by openshift-bot

pkg/build/controller/policy/errors.go

@@ -6,20 +6,6 @@ import (
 	buildapi "github.com/openshift/origin/pkg/build/api"
 )
 
-// NoBuildConfigLabelError represents an error caused by the build not having
-// the required build config label.
-type NoBuildConfigLabelError struct {
-	build *buildapi.Build
-}
-
-func NewNoBuildConfigLabelError(build *buildapi.Build) error {
-	return NoBuildConfigLabelError{build: build}
-}
-
-func (e NoBuildConfigLabelError) Error() string {
-	return fmt.Sprintf("build %s/%s does not have required %q label set", e.build.Namespace, e.build.Name, buildapi.BuildConfigLabel)
-}
-
 // NoBuildNumberLabelError represents an error caused by the build not having
 // the required build number annotation.
 type NoBuildNumberAnnotationError struct {

pkg/build/controller/policy/parallel.go

@@ -21,7 +21,7 @@ type ParallelPolicy struct {
 func (s *ParallelPolicy) IsRunnable(build *buildapi.Build) (bool, error) {
 	bcName := buildutil.ConfigNameForBuild(build)
 	if len(bcName) == 0 {
-		return false, NewNoBuildConfigLabelError(build)
+		return true, nil
 	}
 	return !hasRunningSerialBuild(s.BuildLister, build.Namespace, bcName), nil
 }

pkg/build/controller/policy/policy.go

@@ -118,7 +118,7 @@ func GetNextConfigBuild(lister buildclient.BuildLister, namespace, buildConfigNa
 func handleComplete(lister buildclient.BuildLister, updater buildclient.BuildUpdater, build *buildapi.Build) error {
 	bcName := buildutil.ConfigNameForBuild(build)
 	if len(bcName) == 0 {
-		return NewNoBuildConfigLabelError(build)
+		return nil
 	}
 	nextBuild, hasRunningBuilds, err := GetNextConfigBuild(lister, build.Namespace, bcName)
 	if err != nil {

pkg/build/controller/policy/serial.go

@@ -20,7 +20,7 @@ type SerialPolicy struct {
 func (s *SerialPolicy) IsRunnable(build *buildapi.Build) (bool, error) {
 	bcName := buildutil.ConfigNameForBuild(build)
 	if len(bcName) == 0 {
-		return false, NewNoBuildConfigLabelError(build)
+		return true, nil
 	}
 	nextBuild, runningBuilds, err := GetNextConfigBuild(s.BuildLister, build.Namespace, bcName)
 	return !runningBuilds && (nextBuild != nil && nextBuild.Name == build.Name), err

pkg/build/controller/policy/serial_latest_only.go

@@ -32,7 +32,7 @@ type SerialLatestOnlyPolicy struct {
 func (s *SerialLatestOnlyPolicy) IsRunnable(build *buildapi.Build) (bool, error) {
 	bcName := buildutil.ConfigNameForBuild(build)
 	if len(bcName) == 0 {
-		return false, NewNoBuildConfigLabelError(build)
+		return true, nil
 	}
 	if err := kerrors.NewAggregate(s.cancelPreviousBuilds(build)); err != nil {
 		return false, err
@@ -54,7 +54,7 @@ func (s *SerialLatestOnlyPolicy) OnComplete(build *buildapi.Build) error {
 func (s *SerialLatestOnlyPolicy) cancelPreviousBuilds(build *buildapi.Build) []error {
 	bcName := buildutil.ConfigNameForBuild(build)
 	if len(bcName) == 0 {
-		return []error{NewNoBuildConfigLabelError(build)}
+		return []error{}
 	}
 	currentBuildNumber, err := buildutil.BuildNumber(build)
 	if err != nil {

pkg/build/controller/policy/serial_latest_only_test.go

@@ -87,58 +87,34 @@ func TestSerialLatestOnlyIsRunnableBuildsWithErrors(t *testing.T) {
 	builds := []buildapi.Build{
 		addBuild("build-1", "sample-bc", buildapi.BuildPhaseNew, buildapi.BuildRunPolicySerialLatestOnly),
 		addBuild("build-2", "sample-bc", buildapi.BuildPhaseNew, buildapi.BuildRunPolicySerialLatestOnly),
-		addBuild("build-3", "sample-bc", buildapi.BuildPhaseNew, buildapi.BuildRunPolicySerialLatestOnly),
 	}
 
 	// The build-1 will lack required labels
 	builds[0].ObjectMeta.Labels = map[string]string{}
 
-	// The build-2 will lack the build config label
-	builds[1].ObjectMeta.Labels = map[string]string{
-		buildapi.BuildRunPolicyLabel: "SerialLatestOnly",
-	}
-
-	// The build-3 will lack the build number annotation
-	builds[2].ObjectMeta.Annotations = map[string]string{}
+	// The build-2 will lack the build number annotation
+	builds[1].ObjectMeta.Annotations = map[string]string{}
 
 	client := newTestClient(builds)
 	policy := SerialLatestOnlyPolicy{BuildLister: client, BuildUpdater: client}
 
-	if _, err := policy.IsRunnable(&builds[0]); err != nil {
-		if _, ok := err.(NoBuildConfigLabelError); !ok {
-			t.Errorf("expected NoBuildConfigLabelError, got %T", err)
-		}
-	} else {
-		t.Errorf("expected error for build-1")
-	}
-	if _, err := policy.IsRunnable(&builds[1]); err != nil {
-		if _, ok := err.(NoBuildConfigLabelError); !ok {
-			t.Errorf("expected NoBuildConfigLabelError, got %T", err)
-		}
-	} else {
-		t.Errorf("expected error for build-2")
+	ok, err := policy.IsRunnable(&builds[0])
+	if !ok || err != nil {
+		t.Errorf("expected build to be runnable, got %v, error: %v", ok, err)
 	}
+
 	// No type-check as this error is returned as kerrors.aggregate
-	if _, err := policy.IsRunnable(&builds[2]); err == nil {
-		t.Errorf("expected error for build-3")
+	if _, err := policy.IsRunnable(&builds[1]); err == nil {
+		t.Errorf("expected error for build-2")
 	}
 
-	if err := policy.OnComplete(&builds[0]); err != nil {
-		if _, ok := err.(NoBuildConfigLabelError); !ok {
-			t.Errorf("expected NoBuildConfigLabelError, got %T", err)
-		}
-	} else {
-		t.Errorf("expected error for build-1")
-	}
-	if err := policy.OnComplete(&builds[1]); err != nil {
-		if _, ok := err.(NoBuildConfigLabelError); !ok {
-			t.Errorf("expected NoBuildConfigLabelError, got %T", err)
-		}
-	} else {
-		t.Errorf("expected error for build-2")
+	err = policy.OnComplete(&builds[0])
+	if err != nil {
+		t.Errorf("expected no error, got %v", err)
 	}
+
 	// No type-check as this error is returned as kerrors.aggregate
-	if err := policy.OnComplete(&builds[2]); err == nil {
-		t.Errorf("expected error for build-3")
+	if err := policy.OnComplete(&builds[1]); err == nil {
+		t.Errorf("expected error for build-2")
 	}
 }