Skip to content

Commit 7240c76

Browse files
committed
Make external builder helpers
1 parent b33814d commit 7240c76

File tree

16 files changed

+69
-42
lines changed

16 files changed

+69
-42
lines changed

pkg/build/apis/build/v1/conversion.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func Convert_v1_BuildConfig_To_build_BuildConfig(in *v1.BuildConfig, out *newer.
1919
// Strip off any default imagechange triggers where the buildconfig's
2020
// "from" is not an ImageStreamTag, because those triggers
2121
// will never be invoked.
22-
imageRef := buildapihelpers.GetInputReference(out.Spec.Strategy)
22+
imageRef := buildapihelpers.GetInputReferenceInternal(out.Spec.Strategy)
2323
hasIST := imageRef != nil && imageRef.Kind == "ImageStreamTag"
2424
for _, trigger := range out.Spec.Triggers {
2525
if trigger.Type != newer.ImageChangeBuildTriggerType {

pkg/build/apis/build/validation/validation.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func ValidateBuildUpdate(build *buildapi.Build, older *buildapi.Build) field.Err
4242
allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&build.ObjectMeta, &older.ObjectMeta, field.NewPath("metadata"))...)
4343
allErrs = append(allErrs, validation.ValidateObjectMeta(&build.ObjectMeta, true, validation.NameIsDNSSubdomain, field.NewPath("metadata"))...)
4444

45-
if buildutil.IsBuildComplete(older) && older.Status.Phase != build.Status.Phase {
45+
if buildutil.IsBuildCompleteInternal(older) && older.Status.Phase != build.Status.Phase {
4646
allErrs = append(allErrs, field.Invalid(field.NewPath("status", "phase"), build.Status.Phase, "phase cannot be updated from a terminal state"))
4747
}
4848

@@ -85,7 +85,7 @@ func ValidateBuildConfig(config *buildapi.BuildConfig) field.ErrorList {
8585
fromRefs := map[string]struct{}{}
8686
specPath := field.NewPath("spec")
8787
triggersPath := specPath.Child("triggers")
88-
buildFrom := buildapihelpers.GetInputReference(config.Spec.Strategy)
88+
buildFrom := buildapihelpers.GetInputReferenceInternal(config.Spec.Strategy)
8989
for i, trg := range config.Spec.Triggers {
9090
allErrs = append(allErrs, validateTrigger(&trg, buildFrom, triggersPath.Index(i))...)
9191
if trg.Type != buildapi.ImageChangeBuildTriggerType || trg.ImageChange == nil {

pkg/build/apiserver/registry/build/strategy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (detailsStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob
9797
// a completed phase.
9898
phase := oldBuild.Status.Phase
9999
stages := newBuild.Status.Stages
100-
if buildutil.IsBuildComplete(newBuild) {
100+
if buildutil.IsBuildCompleteInternal(newBuild) {
101101
phase = newBuild.Status.Phase
102102
}
103103
revision := newBuild.Spec.Revision

pkg/build/buildapihelpers/helpers.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package buildapihelpers
22

33
import (
4+
corev1 "k8s.io/api/core/v1"
45
coreinternalapi "k8s.io/kubernetes/pkg/apis/core"
56

7+
buildv1 "github.com/openshift/api/build/v1"
68
buildinternalapi "github.com/openshift/origin/pkg/build/apis/build"
79
)
810

@@ -36,9 +38,24 @@ func HasTriggerType(triggerType buildinternalapi.BuildTriggerType, bc *buildinte
3638
return len(matches) > 0
3739
}
3840

41+
// GetInputReferenceInternal returns the From ObjectReference associated with the
42+
// BuildStrategy.
43+
func GetInputReferenceInternal(strategy buildinternalapi.BuildStrategy) *coreinternalapi.ObjectReference {
44+
switch {
45+
case strategy.SourceStrategy != nil:
46+
return &strategy.SourceStrategy.From
47+
case strategy.DockerStrategy != nil:
48+
return strategy.DockerStrategy.From
49+
case strategy.CustomStrategy != nil:
50+
return &strategy.CustomStrategy.From
51+
default:
52+
return nil
53+
}
54+
}
55+
3956
// GetInputReference returns the From ObjectReference associated with the
4057
// BuildStrategy.
41-
func GetInputReference(strategy buildinternalapi.BuildStrategy) *coreinternalapi.ObjectReference {
58+
func GetInputReference(strategy buildv1.BuildStrategy) *corev1.ObjectReference {
4259
switch {
4360
case strategy.SourceStrategy != nil:
4461
return &strategy.SourceStrategy.From

pkg/build/controller/build/build_controller.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ func (bc *BuildController) handleBuild(build *buildapi.Build) error {
330330

331331
// If pipeline build, handle pruning.
332332
if build.Spec.Strategy.JenkinsPipelineStrategy != nil {
333-
if buildutil.IsBuildComplete(build) {
333+
if buildutil.IsBuildCompleteInternal(build) {
334334
if err := common.HandleBuildPruning(buildutil.ConfigNameForBuild(build), build.Namespace, bc.buildLister, bc.buildConfigGetter, bc.buildDeleter); err != nil {
335335
utilruntime.HandleError(fmt.Errorf("failed to prune builds for %s/%s: %v", build.Namespace, build.Name, err))
336336
}
@@ -363,7 +363,7 @@ func (bc *BuildController) handleBuild(build *buildapi.Build) error {
363363
case build.Status.Phase == buildapi.BuildPhasePending,
364364
build.Status.Phase == buildapi.BuildPhaseRunning:
365365
update, err = bc.handleActiveBuild(build, pod)
366-
case buildutil.IsBuildComplete(build):
366+
case buildutil.IsBuildCompleteInternal(build):
367367
update, err = bc.handleCompletedBuild(build, pod)
368368
}
369369
if update != nil && !update.isEmpty() {
@@ -395,7 +395,7 @@ func shouldIgnore(build *buildapi.Build) bool {
395395
// state and its completion time or logsnippet is not set, then we should at least attempt to set its
396396
// completion time and logsnippet if possible because the build pod may have put the build in
397397
// this state and it would have not set the completion timestamp or logsnippet data.
398-
if buildutil.IsBuildComplete(build) {
398+
if buildutil.IsBuildCompleteInternal(build) {
399399
switch build.Status.Phase {
400400
case buildapi.BuildPhaseComplete:
401401
if build.Status.CompletionTimestamp == nil {
@@ -415,7 +415,7 @@ func shouldIgnore(build *buildapi.Build) bool {
415415

416416
// shouldCancel returns true if a build is active and its cancellation flag is set
417417
func shouldCancel(build *buildapi.Build) bool {
418-
return !buildutil.IsBuildComplete(build) && build.Status.Cancelled
418+
return !buildutil.IsBuildCompleteInternal(build) && build.Status.Cancelled
419419
}
420420

421421
// 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
10851085
}
10861086

10871087
// Update build completion timestamp if transitioning to a terminal phase
1088-
if buildutil.IsTerminalPhase(*update.phase) {
1088+
if buildutil.IsTerminalPhaseInternal(*update.phase) {
10891089
setBuildCompletionData(build, pod, update)
10901090
}
10911091
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
11141114
buildapi.BuildPhaseFailed:
11151115
bc.recorder.Eventf(patchedBuild, kapi.EventTypeNormal, buildapi.BuildFailedEventReason, fmt.Sprintf(buildapi.BuildFailedEventMessage, patchedBuild.Namespace, patchedBuild.Name))
11161116
}
1117-
if buildutil.IsTerminalPhase(*update.phase) {
1117+
if buildutil.IsTerminalPhaseInternal(*update.phase) {
11181118
bc.handleBuildCompletion(patchedBuild)
11191119
}
11201120
}
@@ -1270,7 +1270,7 @@ func (bc *BuildController) buildDeleted(obj interface{}) {
12701270
}
12711271
}
12721272
// If the build was not in a complete state, poke the buildconfig to run the next build
1273-
if !buildutil.IsBuildComplete(build) {
1273+
if !buildutil.IsBuildCompleteInternal(build) {
12741274
bcName := buildutil.ConfigNameForBuild(build)
12751275
bc.enqueueBuildConfig(build.Namespace, bcName)
12761276
}
@@ -1375,7 +1375,7 @@ func isValidTransition(from, to buildapi.BuildPhase) bool {
13751375
}
13761376

13771377
switch {
1378-
case buildutil.IsTerminalPhase(from):
1378+
case buildutil.IsTerminalPhaseInternal(from):
13791379
return false
13801380
case from == buildapi.BuildPhasePending:
13811381
switch to {

pkg/build/controller/build/build_controller_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ func TestIsValidTransition(t *testing.T) {
758758
}
759759
for _, fromPhase := range phases {
760760
for _, toPhase := range phases {
761-
if buildutil.IsTerminalPhase(fromPhase) && fromPhase != toPhase {
761+
if buildutil.IsTerminalPhaseInternal(fromPhase) && fromPhase != toPhase {
762762
if isValidTransition(fromPhase, toPhase) {
763763
t.Errorf("transition %v -> %v should be invalid", fromPhase, toPhase)
764764
}
@@ -795,7 +795,7 @@ func TestIsTerminal(t *testing.T) {
795795
buildapi.BuildPhaseCancelled: true,
796796
}
797797
for phase, expected := range tests {
798-
if actual := buildutil.IsTerminalPhase(phase); actual != expected {
798+
if actual := buildutil.IsTerminalPhaseInternal(phase); actual != expected {
799799
t.Errorf("unexpected response for %s: %v", phase, actual)
800800
}
801801
}

pkg/build/controller/policy/serial_latest_only.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (s *SerialLatestOnlyPolicy) cancelPreviousBuilds(build *buildapi.Build) []e
6464
builds, err := buildutil.BuildConfigBuilds(s.BuildLister, build.Namespace, bcName, func(b *buildapi.Build) bool {
6565
// Do not cancel the complete builds, builds that were already cancelled, or
6666
// running builds.
67-
if buildutil.IsBuildComplete(b) || b.Status.Phase == buildapi.BuildPhaseRunning {
67+
if buildutil.IsBuildCompleteInternal(b) || b.Status.Phase == buildapi.BuildPhaseRunning {
6868
return false
6969
}
7070

pkg/build/generator/generator.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func findImageChangeTrigger(bc *buildapi.BuildConfig, ref *kapi.ObjectReference)
154154
imageChange := trigger.ImageChange
155155
triggerRef := imageChange.From
156156
if triggerRef == nil {
157-
triggerRef = buildapihelpers.GetInputReference(bc.Spec.Strategy)
157+
triggerRef = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy)
158158
if triggerRef == nil || triggerRef.Kind != "ImageStreamTag" {
159159
continue
160160
}
@@ -356,7 +356,7 @@ func (g *BuildGenerator) updateImageTriggers(ctx context.Context, bc *buildapi.B
356356

357357
triggerImageRef := trigger.ImageChange.From
358358
if triggerImageRef == nil {
359-
triggerImageRef = buildapihelpers.GetInputReference(bc.Spec.Strategy)
359+
triggerImageRef = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy)
360360
}
361361
if triggerImageRef == nil {
362362
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
541541
var sourceImageSpec string
542542
// if the imagesource matches the strategy from, and we have a trigger for the strategy from,
543543
// use the imageid from the trigger rather than resolving it.
544-
if strategyFrom := buildapihelpers.GetInputReference(bcCopy.Spec.Strategy); strategyFrom != nil &&
544+
if strategyFrom := buildapihelpers.GetInputReferenceInternal(bcCopy.Spec.Strategy); strategyFrom != nil &&
545545
reflect.DeepEqual(sourceImage.From, *strategyFrom) &&
546546
strategyImageChangeTrigger != nil {
547547
sourceImageSpec = strategyImageChangeTrigger.LastTriggeredImageID

pkg/build/generator/generator_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ func TestInstantiateWithImageTrigger(t *testing.T) {
449449
if bc.Spec.Triggers[i].Type == buildapi.ImageChangeBuildTriggerType {
450450
from := bc.Spec.Triggers[i].ImageChange.From
451451
if from == nil {
452-
from = buildapihelpers.GetInputReference(bc.Spec.Strategy)
452+
from = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy)
453453
}
454454
if bc.Spec.Triggers[i].ImageChange.LastTriggeredImageID != ("ref/" + from.Name) {
455455
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)

pkg/build/util/util.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"k8s.io/kubernetes/pkg/credentialprovider"
1717
credentialprovidersecrets "k8s.io/kubernetes/pkg/credentialprovider/secrets"
1818

19+
buildv1 "github.com/openshift/api/build/v1"
1920
buildapi "github.com/openshift/origin/pkg/build/apis/build"
2021
"github.com/openshift/origin/pkg/build/buildapihelpers"
2122
buildlister "github.com/openshift/origin/pkg/build/generated/listers/build/internalversion"
@@ -39,13 +40,13 @@ var (
3940
proxyRegex = regexp.MustCompile("(?i)proxy")
4041
)
4142

42-
// IsBuildComplete returns whether the provided build is complete or not
43-
func IsBuildComplete(build *buildapi.Build) bool {
44-
return IsTerminalPhase(build.Status.Phase)
43+
// IsBuildCompleteInternal returns whether the provided build is complete or not
44+
func IsBuildCompleteInternal(build *buildapi.Build) bool {
45+
return IsTerminalPhaseInternal(build.Status.Phase)
4546
}
4647

4748
// IsTerminalPhase returns true if the provided phase is terminal
48-
func IsTerminalPhase(phase buildapi.BuildPhase) bool {
49+
func IsTerminalPhaseInternal(phase buildapi.BuildPhase) bool {
4950
switch phase {
5051
case buildapi.BuildPhaseNew,
5152
buildapi.BuildPhasePending,
@@ -55,6 +56,22 @@ func IsTerminalPhase(phase buildapi.BuildPhase) bool {
5556
return true
5657
}
5758

59+
// IsBuildComplete returns whether the provided build is complete or not
60+
func IsBuildComplete(build *buildv1.Build) bool {
61+
return IsTerminalPhase(build.Status.Phase)
62+
}
63+
64+
// IsTerminalPhase returns true if the provided phase is terminal
65+
func IsTerminalPhase(phase buildv1.BuildPhase) bool {
66+
switch phase {
67+
case buildv1.BuildPhaseNew,
68+
buildv1.BuildPhasePending,
69+
buildv1.BuildPhaseRunning:
70+
return false
71+
}
72+
return true
73+
}
74+
5875
// BuildNameForConfigVersion returns the name of the version-th build
5976
// for the config that has the provided name.
6077
func BuildNameForConfigVersion(name string, version int) string {

0 commit comments

Comments
 (0)