Skip to content

Commit 7240c76

Browse files
soltyshsoltysh
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 {

‎pkg/image/controller/trigger/image_trigger_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,7 @@ func updateBuildConfigImages(bc *buildapi.BuildConfig, tagRetriever trigger.TagR
948948
if p.From != nil {
949949
from = p.From
950950
} else {
951-
from = buildapihelpers.GetInputReference(bc.Spec.Strategy)
951+
from = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy)
952952
}
953953
namespace := from.Namespace
954954
if len(namespace) == 0 {

‎pkg/image/trigger/buildconfigs/buildconfigs.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func calculateBuildConfigTriggers(bc *buildapi.BuildConfig) []triggerapi.ObjectF
3838
from = t.ImageChange.From
3939
fieldPath = "spec.triggers"
4040
} else {
41-
from = buildapihelpers.GetInputReference(bc.Spec.Strategy)
41+
from = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy)
4242
fieldPath = "spec.strategy.*.from"
4343
}
4444
if from == nil || from.Kind != "ImageStreamTag" || len(from.Name) == 0 {
@@ -152,7 +152,7 @@ func (r *buildConfigReactor) ImageChanged(obj runtime.Object, tagRetriever trigg
152152
if p.From != nil {
153153
from = p.From
154154
} else {
155-
from = buildapihelpers.GetInputReference(bc.Spec.Strategy)
155+
from = buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy)
156156
}
157157
namespace := from.Namespace
158158
if len(namespace) == 0 {

‎pkg/oc/cli/cancelbuild/cancelbuild.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import (
2020
"github.com/openshift/api/build"
2121
buildv1 "github.com/openshift/api/build/v1"
2222
buildtv1client "github.com/openshift/client-go/build/clientset/versioned/typed/build/v1"
23-
buildapi "github.com/openshift/origin/pkg/build/apis/build"
24-
buildapiv1 "github.com/openshift/origin/pkg/build/apis/build/v1"
2523
buildclientinternal "github.com/openshift/origin/pkg/build/client"
2624
buildclientv1 "github.com/openshift/origin/pkg/build/client/v1"
2725
buildinternalclient "github.com/openshift/origin/pkg/build/generated/internalclientset"
@@ -212,12 +210,7 @@ func (o *CancelBuildOptions) RunCancelBuild() error {
212210
}
213211
}
214212

215-
internalBuildStatus := &buildapi.BuildStatus{}
216-
if err := buildapiv1.Convert_v1_BuildStatus_To_build_BuildStatus(&build.Status, internalBuildStatus, nil); err != nil {
217-
return err
218-
}
219-
220-
if stateMatch && !buildutil.IsTerminalPhase(internalBuildStatus.Phase) {
213+
if stateMatch && !buildutil.IsTerminalPhase(build.Phase) {
221214
builds = append(builds, build)
222215
}
223216
}

‎pkg/oc/lib/newapp/cmd/newapp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1192,7 +1192,7 @@ func (c *AppConfig) checkCircularReferences(objects app.Objects) error {
11921192
}
11931193

11941194
if bc, ok := obj.(*buildapi.BuildConfig); ok {
1195-
input := buildapihelpers.GetInputReference(bc.Spec.Strategy)
1195+
input := buildapihelpers.GetInputReferenceInternal(bc.Spec.Strategy)
11961196
output := bc.Spec.Output.To
11971197

11981198
if output == nil || input == nil {

‎pkg/template/controller/readiness.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ import (
2929
func checkBuildReadiness(obj runtime.Object) (bool, bool, error) {
3030
b := obj.(*buildapi.Build)
3131

32-
ready := buildutil.IsTerminalPhase(b.Status.Phase) &&
32+
ready := buildutil.IsTerminalPhaseInternal(b.Status.Phase) &&
3333
b.Status.Phase == buildapi.BuildPhaseComplete
3434

35-
failed := buildutil.IsTerminalPhase(b.Status.Phase) &&
35+
failed := buildutil.IsTerminalPhaseInternal(b.Status.Phase) &&
3636
b.Status.Phase != buildapi.BuildPhaseComplete
3737

3838
return ready, failed, nil

‎test/extended/builds/run_policy.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy",
7373
for {
7474
event := <-buildWatch.ResultChan()
7575
build := event.Object.(*buildapi.Build)
76-
o.Expect(buildutil.IsBuildComplete(build)).Should(o.BeFalse())
76+
o.Expect(buildutil.IsBuildCompleteInternal(build)).Should(o.BeFalse())
7777
if build.Name == startedBuilds[0] && build.Status.Phase == buildapi.BuildPhaseRunning {
7878
break
7979
}
@@ -92,7 +92,7 @@ var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy",
9292
event := <-buildWatch.ResultChan()
9393
build := event.Object.(*buildapi.Build)
9494
if build.Name == startedBuilds[0] {
95-
if buildutil.IsBuildComplete(build) {
95+
if buildutil.IsBuildCompleteInternal(build) {
9696
break
9797
}
9898
continue
@@ -117,7 +117,7 @@ var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy",
117117
counter++
118118
}
119119
// When the build failed or completed prematurely, fail the test
120-
o.Expect(buildutil.IsBuildComplete(build)).Should(o.BeFalse())
120+
o.Expect(buildutil.IsBuildCompleteInternal(build)).Should(o.BeFalse())
121121
if counter == 2 {
122122
break
123123
}
@@ -403,7 +403,7 @@ var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy",
403403
expectedRunningBuild = 2
404404
break
405405
}
406-
o.Expect(buildutil.IsBuildComplete(build)).Should(o.BeFalse())
406+
o.Expect(buildutil.IsBuildCompleteInternal(build)).Should(o.BeFalse())
407407
}
408408
}
409409

0 commit comments

Comments
 (0)