diff --git a/pkg/build/apis/build/v1/conversion.go b/pkg/build/apis/build/v1/conversion.go index 0440876f7b9d..61bc5bbef7d8 100644 --- a/pkg/build/apis/build/v1/conversion.go +++ b/pkg/build/apis/build/v1/conversion.go @@ -19,7 +19,7 @@ func Convert_v1_BuildConfig_To_build_BuildConfig(in *v1.BuildConfig, out *newer. // Strip off any default imagechange triggers where the buildconfig's // "from" is not an ImageStreamTag, because those triggers // will never be invoked. - imageRef := buildapihelpers.GetInputReference(out.Spec.Strategy) + imageRef := buildapihelpers.GetInputReferenceInternal(out.Spec.Strategy) hasIST := imageRef != nil && imageRef.Kind == "ImageStreamTag" for _, trigger := range out.Spec.Triggers { if trigger.Type != newer.ImageChangeBuildTriggerType { diff --git a/pkg/build/apis/build/validation/validation.go b/pkg/build/apis/build/validation/validation.go index 5c707efb9b52..b46c8f62b7c0 100644 --- a/pkg/build/apis/build/validation/validation.go +++ b/pkg/build/apis/build/validation/validation.go @@ -42,7 +42,7 @@ func ValidateBuildUpdate(build *buildapi.Build, older *buildapi.Build) field.Err allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&build.ObjectMeta, &older.ObjectMeta, field.NewPath("metadata"))...) allErrs = append(allErrs, validation.ValidateObjectMeta(&build.ObjectMeta, true, validation.NameIsDNSSubdomain, field.NewPath("metadata"))...) - if buildutil.IsBuildComplete(older) && older.Status.Phase != build.Status.Phase { + if buildutil.IsBuildCompleteInternal(older) && older.Status.Phase != build.Status.Phase { allErrs = append(allErrs, field.Invalid(field.NewPath("status", "phase"), build.Status.Phase, "phase cannot be updated from a terminal state")) } @@ -85,7 +85,7 @@ func ValidateBuildConfig(config *buildapi.BuildConfig) field.ErrorList { fromRefs := map[string]struct{}{} specPath := field.NewPath("spec") triggersPath := specPath.Child("triggers") - buildFrom := buildapihelpers.GetInputReference(config.Spec.Strategy) + buildFrom := buildapihelpers.GetInputReferenceInternal(config.Spec.Strategy) for i, trg := range config.Spec.Triggers { allErrs = append(allErrs, validateTrigger(&trg, buildFrom, triggersPath.Index(i))...) if trg.Type != buildapi.ImageChangeBuildTriggerType || trg.ImageChange == nil { diff --git a/pkg/build/apiserver/registry/build/strategy.go b/pkg/build/apiserver/registry/build/strategy.go index e2381004e5a2..119f5175bdcf 100644 --- a/pkg/build/apiserver/registry/build/strategy.go +++ b/pkg/build/apiserver/registry/build/strategy.go @@ -97,7 +97,7 @@ func (detailsStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob // a completed phase. phase := oldBuild.Status.Phase stages := newBuild.Status.Stages - if buildutil.IsBuildComplete(newBuild) { + if buildutil.IsBuildCompleteInternal(newBuild) { phase = newBuild.Status.Phase } revision := newBuild.Spec.Revision diff --git a/pkg/build/buildapihelpers/helpers.go b/pkg/build/buildapihelpers/helpers.go index 8ea32aa80f38..d76dd8fa6e75 100644 --- a/pkg/build/buildapihelpers/helpers.go +++ b/pkg/build/buildapihelpers/helpers.go @@ -1,8 +1,10 @@ package buildapihelpers import ( + corev1 "k8s.io/api/core/v1" coreinternalapi "k8s.io/kubernetes/pkg/apis/core" + buildv1 "github.com/openshift/api/build/v1" buildinternalapi "github.com/openshift/origin/pkg/build/apis/build" ) @@ -36,9 +38,24 @@ func HasTriggerType(triggerType buildinternalapi.BuildTriggerType, bc *buildinte return len(matches) > 0 } +// GetInputReferenceInternal returns the From ObjectReference associated with the +// BuildStrategy. +func GetInputReferenceInternal(strategy buildinternalapi.BuildStrategy) *coreinternalapi.ObjectReference { + switch { + case strategy.SourceStrategy != nil: + return &strategy.SourceStrategy.From + case strategy.DockerStrategy != nil: + return strategy.DockerStrategy.From + case strategy.CustomStrategy != nil: + return &strategy.CustomStrategy.From + default: + return nil + } +} + // GetInputReference returns the From ObjectReference associated with the // BuildStrategy. -func GetInputReference(strategy buildinternalapi.BuildStrategy) *coreinternalapi.ObjectReference { +func GetInputReference(strategy buildv1.BuildStrategy) *corev1.ObjectReference { switch { case strategy.SourceStrategy != nil: return &strategy.SourceStrategy.From diff --git a/pkg/build/controller/build/build_controller.go b/pkg/build/controller/build/build_controller.go index 3b4a496baf6c..ed140ed5ae01 100644 --- a/pkg/build/controller/build/build_controller.go +++ b/pkg/build/controller/build/build_controller.go @@ -330,7 +330,7 @@ func (bc *BuildController) handleBuild(build *buildapi.Build) error { // If pipeline build, handle pruning. if build.Spec.Strategy.JenkinsPipelineStrategy != nil { - if buildutil.IsBuildComplete(build) { + if buildutil.IsBuildCompleteInternal(build) { if err := common.HandleBuildPruning(buildutil.ConfigNameForBuild(build), build.Namespace, bc.buildLister, bc.buildConfigGetter, bc.buildDeleter); err != nil { utilruntime.HandleError(fmt.Errorf("failed to prune builds for %s/%s: %v", build.Namespace, build.Name, err)) } @@ -363,7 +363,7 @@ func (bc *BuildController) handleBuild(build *buildapi.Build) error { case build.Status.Phase == buildapi.BuildPhasePending, build.Status.Phase == buildapi.BuildPhaseRunning: update, err = bc.handleActiveBuild(build, pod) - case buildutil.IsBuildComplete(build): + case buildutil.IsBuildCompleteInternal(build): update, err = bc.handleCompletedBuild(build, pod) } if update != nil && !update.isEmpty() { @@ -395,7 +395,7 @@ func shouldIgnore(build *buildapi.Build) bool { // state and its completion time or logsnippet is not set, then we should at least attempt to set its // completion time and logsnippet if possible because the build pod may have put the build in // this state and it would have not set the completion timestamp or logsnippet data. - if buildutil.IsBuildComplete(build) { + if buildutil.IsBuildCompleteInternal(build) { switch build.Status.Phase { case buildapi.BuildPhaseComplete: if build.Status.CompletionTimestamp == nil { @@ -415,7 +415,7 @@ func shouldIgnore(build *buildapi.Build) bool { // shouldCancel returns true if a build is active and its cancellation flag is set func shouldCancel(build *buildapi.Build) bool { - return !buildutil.IsBuildComplete(build) && build.Status.Cancelled + return !buildutil.IsBuildCompleteInternal(build) && build.Status.Cancelled } // cancelBuild deletes a build pod and returns an update to mark the build as cancelled @@ -1085,7 +1085,7 @@ func (bc *BuildController) updateBuild(build *buildapi.Build, update *buildUpdat } // Update build completion timestamp if transitioning to a terminal phase - if buildutil.IsTerminalPhase(*update.phase) { + if buildutil.IsTerminalPhaseInternal(*update.phase) { setBuildCompletionData(build, pod, update) } glog.V(4).Infof("Updating build %s -> %s%s", buildDesc(build), *update.phase, reasonText) @@ -1114,7 +1114,7 @@ func (bc *BuildController) updateBuild(build *buildapi.Build, update *buildUpdat buildapi.BuildPhaseFailed: bc.recorder.Eventf(patchedBuild, kapi.EventTypeNormal, buildapi.BuildFailedEventReason, fmt.Sprintf(buildapi.BuildFailedEventMessage, patchedBuild.Namespace, patchedBuild.Name)) } - if buildutil.IsTerminalPhase(*update.phase) { + if buildutil.IsTerminalPhaseInternal(*update.phase) { bc.handleBuildCompletion(patchedBuild) } } @@ -1270,7 +1270,7 @@ func (bc *BuildController) buildDeleted(obj interface{}) { } } // If the build was not in a complete state, poke the buildconfig to run the next build - if !buildutil.IsBuildComplete(build) { + if !buildutil.IsBuildCompleteInternal(build) { bcName := buildutil.ConfigNameForBuild(build) bc.enqueueBuildConfig(build.Namespace, bcName) } @@ -1375,7 +1375,7 @@ func isValidTransition(from, to buildapi.BuildPhase) bool { } switch { - case buildutil.IsTerminalPhase(from): + case buildutil.IsTerminalPhaseInternal(from): return false case from == buildapi.BuildPhasePending: switch to { diff --git a/pkg/build/controller/build/build_controller_test.go b/pkg/build/controller/build/build_controller_test.go index 69aa2eee8212..708e773b2513 100644 --- a/pkg/build/controller/build/build_controller_test.go +++ b/pkg/build/controller/build/build_controller_test.go @@ -758,7 +758,7 @@ func TestIsValidTransition(t *testing.T) { } for _, fromPhase := range phases { for _, toPhase := range phases { - if buildutil.IsTerminalPhase(fromPhase) && fromPhase != toPhase { + if buildutil.IsTerminalPhaseInternal(fromPhase) && fromPhase != toPhase { if isValidTransition(fromPhase, toPhase) { t.Errorf("transition %v -> %v should be invalid", fromPhase, toPhase) } @@ -795,7 +795,7 @@ func TestIsTerminal(t *testing.T) { buildapi.BuildPhaseCancelled: true, } for phase, expected := range tests { - if actual := buildutil.IsTerminalPhase(phase); actual != expected { + if actual := buildutil.IsTerminalPhaseInternal(phase); actual != expected { t.Errorf("unexpected response for %s: %v", phase, actual) } } diff --git a/pkg/build/controller/policy/serial_latest_only.go b/pkg/build/controller/policy/serial_latest_only.go index c601bb7febdd..79975f400d6e 100644 --- a/pkg/build/controller/policy/serial_latest_only.go +++ b/pkg/build/controller/policy/serial_latest_only.go @@ -64,7 +64,7 @@ func (s *SerialLatestOnlyPolicy) cancelPreviousBuilds(build *buildapi.Build) []e builds, err := buildutil.BuildConfigBuilds(s.BuildLister, build.Namespace, bcName, func(b *buildapi.Build) bool { // Do not cancel the complete builds, builds that were already cancelled, or // running builds. - if buildutil.IsBuildComplete(b) || b.Status.Phase == buildapi.BuildPhaseRunning { + if buildutil.IsBuildCompleteInternal(b) || b.Status.Phase == buildapi.BuildPhaseRunning { return false } diff --git a/pkg/build/generator/generator.go b/pkg/build/generator/generator.go index bffdc35a3b65..abf21e93e14e 100644 --- a/pkg/build/generator/generator.go +++ b/pkg/build/generator/generator.go @@ -154,7 +154,7 @@ func findImageChangeTrigger(bc *buildapi.BuildConfig, ref *kapi.ObjectReference) imageChange := trigger.ImageChange triggerRef := imageChange.From if triggerRef == nil { - triggerRef = buildapihelpers.GetInputReference(bc.Spec.Strategy) + triggerRef = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy) if triggerRef == nil || triggerRef.Kind != "ImageStreamTag" { continue } @@ -356,7 +356,7 @@ func (g *BuildGenerator) updateImageTriggers(ctx context.Context, bc *buildapi.B triggerImageRef := trigger.ImageChange.From if triggerImageRef == nil { - triggerImageRef = buildapihelpers.GetInputReference(bc.Spec.Strategy) + triggerImageRef = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy) } if triggerImageRef == nil { glog.Warningf("Could not get ImageStream reference for default ImageChangeTrigger on BuildConfig %s/%s", bc.Namespace, bc.Name) @@ -541,7 +541,7 @@ func (g *BuildGenerator) setBuildSourceImage(ctx context.Context, builderSecrets var sourceImageSpec string // if the imagesource matches the strategy from, and we have a trigger for the strategy from, // use the imageid from the trigger rather than resolving it. - if strategyFrom := buildapihelpers.GetInputReference(bcCopy.Spec.Strategy); strategyFrom != nil && + if strategyFrom := buildapihelpers.GetInputReferenceInternal(bcCopy.Spec.Strategy); strategyFrom != nil && reflect.DeepEqual(sourceImage.From, *strategyFrom) && strategyImageChangeTrigger != nil { sourceImageSpec = strategyImageChangeTrigger.LastTriggeredImageID diff --git a/pkg/build/generator/generator_test.go b/pkg/build/generator/generator_test.go index 4f687b503a0c..e537d9a6ae25 100644 --- a/pkg/build/generator/generator_test.go +++ b/pkg/build/generator/generator_test.go @@ -449,7 +449,7 @@ func TestInstantiateWithImageTrigger(t *testing.T) { if bc.Spec.Triggers[i].Type == buildapi.ImageChangeBuildTriggerType { from := bc.Spec.Triggers[i].ImageChange.From if from == nil { - from = buildapihelpers.GetInputReference(bc.Spec.Strategy) + from = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy) } if bc.Spec.Triggers[i].ImageChange.LastTriggeredImageID != ("ref/" + from.Name) { t.Errorf("%s: expected LastTriggeredImageID for trigger at %d (%+v) to be %s. Got: %s", tc.name, i, bc.Spec.Triggers[i].ImageChange.From, "ref/"+from.Name, bc.Spec.Triggers[i].ImageChange.LastTriggeredImageID) diff --git a/pkg/build/util/util.go b/pkg/build/util/util.go index 4fb4c84e8127..ba01b3af9d68 100644 --- a/pkg/build/util/util.go +++ b/pkg/build/util/util.go @@ -16,6 +16,7 @@ import ( "k8s.io/kubernetes/pkg/credentialprovider" credentialprovidersecrets "k8s.io/kubernetes/pkg/credentialprovider/secrets" + buildv1 "github.com/openshift/api/build/v1" buildapi "github.com/openshift/origin/pkg/build/apis/build" "github.com/openshift/origin/pkg/build/buildapihelpers" buildlister "github.com/openshift/origin/pkg/build/generated/listers/build/internalversion" @@ -39,13 +40,13 @@ var ( proxyRegex = regexp.MustCompile("(?i)proxy") ) -// IsBuildComplete returns whether the provided build is complete or not -func IsBuildComplete(build *buildapi.Build) bool { - return IsTerminalPhase(build.Status.Phase) +// IsBuildCompleteInternal returns whether the provided build is complete or not +func IsBuildCompleteInternal(build *buildapi.Build) bool { + return IsTerminalPhaseInternal(build.Status.Phase) } // IsTerminalPhase returns true if the provided phase is terminal -func IsTerminalPhase(phase buildapi.BuildPhase) bool { +func IsTerminalPhaseInternal(phase buildapi.BuildPhase) bool { switch phase { case buildapi.BuildPhaseNew, buildapi.BuildPhasePending, @@ -55,6 +56,22 @@ func IsTerminalPhase(phase buildapi.BuildPhase) bool { return true } +// IsBuildComplete returns whether the provided build is complete or not +func IsBuildComplete(build *buildv1.Build) bool { + return IsTerminalPhase(build.Status.Phase) +} + +// IsTerminalPhase returns true if the provided phase is terminal +func IsTerminalPhase(phase buildv1.BuildPhase) bool { + switch phase { + case buildv1.BuildPhaseNew, + buildv1.BuildPhasePending, + buildv1.BuildPhaseRunning: + return false + } + return true +} + // BuildNameForConfigVersion returns the name of the version-th build // for the config that has the provided name. func BuildNameForConfigVersion(name string, version int) string { diff --git a/pkg/image/controller/trigger/image_trigger_controller_test.go b/pkg/image/controller/trigger/image_trigger_controller_test.go index e24e36dd511f..95d25be82a2e 100644 --- a/pkg/image/controller/trigger/image_trigger_controller_test.go +++ b/pkg/image/controller/trigger/image_trigger_controller_test.go @@ -948,7 +948,7 @@ func updateBuildConfigImages(bc *buildapi.BuildConfig, tagRetriever trigger.TagR if p.From != nil { from = p.From } else { - from = buildapihelpers.GetInputReference(bc.Spec.Strategy) + from = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy) } namespace := from.Namespace if len(namespace) == 0 { diff --git a/pkg/image/trigger/buildconfigs/buildconfigs.go b/pkg/image/trigger/buildconfigs/buildconfigs.go index d7bdbea65bfa..73cf01a2e407 100644 --- a/pkg/image/trigger/buildconfigs/buildconfigs.go +++ b/pkg/image/trigger/buildconfigs/buildconfigs.go @@ -38,7 +38,7 @@ func calculateBuildConfigTriggers(bc *buildapi.BuildConfig) []triggerapi.ObjectF from = t.ImageChange.From fieldPath = "spec.triggers" } else { - from = buildapihelpers.GetInputReference(bc.Spec.Strategy) + from = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy) fieldPath = "spec.strategy.*.from" } if from == nil || from.Kind != "ImageStreamTag" || len(from.Name) == 0 { @@ -152,7 +152,7 @@ func (r *buildConfigReactor) ImageChanged(obj runtime.Object, tagRetriever trigg if p.From != nil { from = p.From } else { - from = buildapihelpers.GetInputReference(bc.Spec.Strategy) + from = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy) } namespace := from.Namespace if len(namespace) == 0 { diff --git a/pkg/oc/cli/cancelbuild/cancelbuild.go b/pkg/oc/cli/cancelbuild/cancelbuild.go index 835d614f3314..e1a6cf91fa76 100644 --- a/pkg/oc/cli/cancelbuild/cancelbuild.go +++ b/pkg/oc/cli/cancelbuild/cancelbuild.go @@ -20,8 +20,6 @@ import ( "github.com/openshift/api/build" buildv1 "github.com/openshift/api/build/v1" buildtv1client "github.com/openshift/client-go/build/clientset/versioned/typed/build/v1" - buildapi "github.com/openshift/origin/pkg/build/apis/build" - buildapiv1 "github.com/openshift/origin/pkg/build/apis/build/v1" buildclientinternal "github.com/openshift/origin/pkg/build/client" buildclientv1 "github.com/openshift/origin/pkg/build/client/v1" buildinternalclient "github.com/openshift/origin/pkg/build/generated/internalclientset" @@ -212,12 +210,7 @@ func (o *CancelBuildOptions) RunCancelBuild() error { } } - internalBuildStatus := &buildapi.BuildStatus{} - if err := buildapiv1.Convert_v1_BuildStatus_To_build_BuildStatus(&build.Status, internalBuildStatus, nil); err != nil { - return err - } - - if stateMatch && !buildutil.IsTerminalPhase(internalBuildStatus.Phase) { + if stateMatch && !buildutil.IsTerminalPhase(build.Phase) { builds = append(builds, build) } } diff --git a/pkg/oc/lib/newapp/cmd/newapp.go b/pkg/oc/lib/newapp/cmd/newapp.go index bb5fd5c79a2c..e730aa5043d4 100644 --- a/pkg/oc/lib/newapp/cmd/newapp.go +++ b/pkg/oc/lib/newapp/cmd/newapp.go @@ -1192,7 +1192,7 @@ func (c *AppConfig) checkCircularReferences(objects app.Objects) error { } if bc, ok := obj.(*buildapi.BuildConfig); ok { - input := buildapihelpers.GetInputReference(bc.Spec.Strategy) + input := buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy) output := bc.Spec.Output.To if output == nil || input == nil { diff --git a/pkg/template/controller/readiness.go b/pkg/template/controller/readiness.go index 294e4215a334..f38006d6428f 100644 --- a/pkg/template/controller/readiness.go +++ b/pkg/template/controller/readiness.go @@ -29,10 +29,10 @@ import ( func checkBuildReadiness(obj runtime.Object) (bool, bool, error) { b := obj.(*buildapi.Build) - ready := buildutil.IsTerminalPhase(b.Status.Phase) && + ready := buildutil.IsTerminalPhaseInternal(b.Status.Phase) && b.Status.Phase == buildapi.BuildPhaseComplete - failed := buildutil.IsTerminalPhase(b.Status.Phase) && + failed := buildutil.IsTerminalPhaseInternal(b.Status.Phase) && b.Status.Phase != buildapi.BuildPhaseComplete return ready, failed, nil diff --git a/test/extended/builds/run_policy.go b/test/extended/builds/run_policy.go index 299852732742..364e25f20fe7 100644 --- a/test/extended/builds/run_policy.go +++ b/test/extended/builds/run_policy.go @@ -73,7 +73,7 @@ var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy", for { event := <-buildWatch.ResultChan() build := event.Object.(*buildapi.Build) - o.Expect(buildutil.IsBuildComplete(build)).Should(o.BeFalse()) + o.Expect(buildutil.IsBuildCompleteInternal(build)).Should(o.BeFalse()) if build.Name == startedBuilds[0] && build.Status.Phase == buildapi.BuildPhaseRunning { break } @@ -92,7 +92,7 @@ var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy", event := <-buildWatch.ResultChan() build := event.Object.(*buildapi.Build) if build.Name == startedBuilds[0] { - if buildutil.IsBuildComplete(build) { + if buildutil.IsBuildCompleteInternal(build) { break } continue @@ -117,7 +117,7 @@ var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy", counter++ } // When the build failed or completed prematurely, fail the test - o.Expect(buildutil.IsBuildComplete(build)).Should(o.BeFalse()) + o.Expect(buildutil.IsBuildCompleteInternal(build)).Should(o.BeFalse()) if counter == 2 { break } @@ -403,7 +403,7 @@ var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy", expectedRunningBuild = 2 break } - o.Expect(buildutil.IsBuildComplete(build)).Should(o.BeFalse()) + o.Expect(buildutil.IsBuildCompleteInternal(build)).Should(o.BeFalse()) } }