Merge pull request #16423 from deads2k/build-05-snip

Automatic merge from submit-queue (batch tested with PRs 16205, 16423)

move some naming helpers to apihelpers

prunes the build api tree by moving naming to general helpers and moving a build util into the buildapi helpers.

@bparees 
/assign mfojtik

hack/import-restrictions.json

@@ -90,8 +90,6 @@
     "allowedImportPackages": [
       "vendor/k8s.io/kubernetes/pkg/api",
       "vendor/k8s.io/kubernetes/pkg/api/v1",
-      "github.com/openshift/origin/pkg/util/namer",
-      "github.com/openshift/origin/pkg/build/util",
       "github.com/openshift/origin/pkg/image/apis/image",
       "vendor/github.com/golang/glog"
     ]
@@ -324,4 +322,4 @@
       "github.com/openshift/origin/pkg/user/apis/user/v1"
     ]
   }
-]
+]

pkg/util/namer/namer.go → pkg/api/apihelpers/namer.go

@@ -1,4 +1,4 @@
-package namer
+package apihelpers
 
 import (
 	"fmt"

pkg/util/namer/namer_test.go → pkg/api/apihelpers/namer_test.go

@@ -1,4 +1,4 @@
-package namer
+package apihelpers
 
 import (
 	"math/rand"

pkg/apps/strategy/support/lifecycle.go

@@ -19,13 +19,13 @@ import (
 	kapi "k8s.io/kubernetes/pkg/api"
 	kcoreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
 
+	"github.com/openshift/origin/pkg/api/apihelpers"
 	deployapi "github.com/openshift/origin/pkg/apps/apis/apps"
 	strategyutil "github.com/openshift/origin/pkg/apps/strategy/util"
 	deployutil "github.com/openshift/origin/pkg/apps/util"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	imageclient "github.com/openshift/origin/pkg/image/generated/internalclientset/typed/image/internalversion"
 	"github.com/openshift/origin/pkg/util"
-	namer "github.com/openshift/origin/pkg/util/namer"
 )
 
 // hookContainerName is the name used for the container that runs inside hook pods.
@@ -398,7 +398,7 @@ func makeHookPod(hook *deployapi.LifecycleHook, rc *kapi.ReplicationController,
 
 	pod := &kapi.Pod{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: namer.GetPodName(rc.Name, suffix),
+			Name: apihelpers.GetPodName(rc.Name, suffix),
 			Annotations: map[string]string{
 				deployapi.DeploymentAnnotation: rc.Name,
 			},

pkg/apps/strategy/support/lifecycle_test.go

@@ -25,8 +25,8 @@ import (
 	deploytest "github.com/openshift/origin/pkg/apps/apis/apps/test"
 	deployv1 "github.com/openshift/origin/pkg/apps/apis/apps/v1"
 	deployutil "github.com/openshift/origin/pkg/apps/util"
-	"github.com/openshift/origin/pkg/util/namer"
 
+	"github.com/openshift/origin/pkg/api/apihelpers"
 	_ "github.com/openshift/origin/pkg/api/install"
 )
 
@@ -249,7 +249,7 @@ func TestHookExecutor_makeHookPod(t *testing.T) {
 			},
 			expected: &kapi.Pod{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: namer.GetPodName(deploymentName, "hook"),
+					Name: apihelpers.GetPodName(deploymentName, "hook"),
 					Labels: map[string]string{
 						deployapi.DeploymentPodTypeLabel:        "hook",
 						deployapi.DeployerPodForDeploymentLabel: deploymentName,
@@ -328,7 +328,7 @@ func TestHookExecutor_makeHookPod(t *testing.T) {
 			},
 			expected: &kapi.Pod{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: namer.GetPodName(deploymentName, "hook"),
+					Name: apihelpers.GetPodName(deploymentName, "hook"),
 					Labels: map[string]string{
 						deployapi.DeploymentPodTypeLabel:        "hook",
 						deployapi.DeployerPodForDeploymentLabel: deploymentName,
@@ -387,7 +387,7 @@ func TestHookExecutor_makeHookPod(t *testing.T) {
 			},
 			expected: &kapi.Pod{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: namer.GetPodName(deploymentName, "hook"),
+					Name: apihelpers.GetPodName(deploymentName, "hook"),
 					Labels: map[string]string{
 						deployapi.DeploymentPodTypeLabel:        "hook",
 						deployapi.DeployerPodForDeploymentLabel: deploymentName,
@@ -453,7 +453,7 @@ func TestHookExecutor_makeHookPod(t *testing.T) {
 			},
 			expected: &kapi.Pod{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: namer.GetPodName(deploymentName, "hook"),
+					Name: apihelpers.GetPodName(deploymentName, "hook"),
 					Labels: map[string]string{
 						deployapi.DeploymentPodTypeLabel:        "hook",
 						deployapi.DeployerPodForDeploymentLabel: deploymentName,

pkg/apps/util/util.go

@@ -20,9 +20,9 @@ import (
 	kcoreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
 	kdeplutil "k8s.io/kubernetes/pkg/controller/deployment/util"
 
+	"github.com/openshift/origin/pkg/api/apihelpers"
 	deployapi "github.com/openshift/origin/pkg/apps/apis/apps"
 	deployapiv1 "github.com/openshift/origin/pkg/apps/apis/apps/v1"
-	"github.com/openshift/origin/pkg/util/namer"
 )
 
 var (
@@ -143,7 +143,7 @@ const DeployerPodSuffix = "deploy"
 
 // DeployerPodNameForDeployment returns the name of a pod for a given deployment
 func DeployerPodNameForDeployment(deployment string) string {
-	return namer.GetPodName(deployment, DeployerPodSuffix)
+	return apihelpers.GetPodName(deployment, DeployerPodSuffix)
 }
 
 // LabelForDeployment builds a string identifier for a Deployment.

pkg/build/apis/build/helpers.go

@@ -129,3 +129,18 @@ func AppendStageAndStepInfo(stages []StageInfo, stagesToMerge []StageInfo) []Sta
 	}
 	return stages
 }
+
+// GetInputReference returns the From ObjectReference associated with the
+// BuildStrategy.
+func GetInputReference(strategy BuildStrategy) *kapi.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
+	}
+}

pkg/build/apis/build/util.go

@@ -1,10 +1,8 @@
 package build
 
 import (
+	"github.com/openshift/origin/pkg/api/apihelpers"
 	"k8s.io/apimachinery/pkg/util/validation"
-	kapi "k8s.io/kubernetes/pkg/api"
-
-	"github.com/openshift/origin/pkg/util/namer"
 )
 
 const (
@@ -14,7 +12,7 @@ const (
 
 // GetBuildPodName returns name of the build pod.
 func GetBuildPodName(build *Build) string {
-	return namer.GetPodName(build.Name, BuildPodSuffix)
+	return apihelpers.GetPodName(build.Name, BuildPodSuffix)
 }
 
 func StrategyType(strategy BuildStrategy) string {
@@ -60,9 +58,3 @@ func LabelValue(name string) string {
 	}
 	return name[:validation.DNS1123LabelMaxLength]
 }
-
-// GetBuildName returns the name of a Build associated with the
-// given Pod.
-func GetBuildName(pod *kapi.Pod) string {
-	return pod.Annotations[BuildAnnotation]
-}

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

@@ -6,7 +6,6 @@ import (
 
 	"github.com/openshift/origin/pkg/api/apihelpers"
 	newer "github.com/openshift/origin/pkg/build/apis/build"
-	buildutil "github.com/openshift/origin/pkg/build/util"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 )
 
@@ -19,7 +18,7 @@ func Convert_v1_BuildConfig_To_build_BuildConfig(in *BuildConfig, out *newer.Bui
 	// Strip off any default imagechange triggers where the buildconfig's
 	// "from" is not an ImageStreamTag, because those triggers
 	// will never be invoked.
-	imageRef := buildutil.GetInputReference(out.Spec.Strategy)
+	imageRef := newer.GetInputReference(out.Spec.Strategy)
 	hasIST := imageRef != nil && imageRef.Kind == "ImageStreamTag"
 	for _, trigger := range out.Spec.Triggers {
 		if trigger.Type != newer.ImageChangeBuildTriggerType {

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

@@ -86,7 +86,7 @@ func ValidateBuildConfig(config *buildapi.BuildConfig) field.ErrorList {
 	fromRefs := map[string]struct{}{}
 	specPath := field.NewPath("spec")
 	triggersPath := specPath.Child("triggers")
-	buildFrom := buildutil.GetInputReference(config.Spec.Strategy)
+	buildFrom := buildapi.GetInputReference(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 {

pkg/build/controller/strategy/util.go

@@ -12,11 +12,11 @@ import (
 	"k8s.io/kubernetes/pkg/api/v1"
 
 	"github.com/golang/glog"
+	"github.com/openshift/origin/pkg/api/apihelpers"
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
 	buildapiv1 "github.com/openshift/origin/pkg/build/apis/build/v1"
 	"github.com/openshift/origin/pkg/build/builder/cmd/dockercfg"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
-	"github.com/openshift/origin/pkg/util/namer"
 	"github.com/openshift/origin/pkg/version"
 )
 
@@ -94,7 +94,7 @@ func setupDockerSocket(pod *v1.Pod) {
 // mountSecretVolume is a helper method responsible for actual mounting secret
 // volumes into a pod.
 func mountSecretVolume(pod *v1.Pod, container *v1.Container, secretName, mountPath, volumeSuffix string) {
-	volumeName := namer.GetName(secretName, volumeSuffix, kvalidation.DNS1123LabelMaxLength)
+	volumeName := apihelpers.GetName(secretName, volumeSuffix, kvalidation.DNS1123LabelMaxLength)
 
 	// coerce from RFC1123 subdomain to RFC1123 label.
 	volumeName = strings.Replace(volumeName, ".", "-", -1)

pkg/build/generator/generator.go

@@ -17,6 +17,7 @@ import (
 	kapi "k8s.io/kubernetes/pkg/api"
 	kcoreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
 
+	"github.com/openshift/origin/pkg/api/apihelpers"
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
 	buildapiv1 "github.com/openshift/origin/pkg/build/apis/build/v1"
 	buildclient "github.com/openshift/origin/pkg/build/generated/internalclientset/typed/build/internalversion"
@@ -25,7 +26,6 @@ import (
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	imageclient "github.com/openshift/origin/pkg/image/generated/internalclientset/typed/image/internalversion"
 	"github.com/openshift/origin/pkg/oc/admin/policy"
-	"github.com/openshift/origin/pkg/util/namer"
 )
 
 const conflictRetries = 3
@@ -157,7 +157,7 @@ func findImageChangeTrigger(bc *buildapi.BuildConfig, ref *kapi.ObjectReference)
 		imageChange := trigger.ImageChange
 		triggerRef := imageChange.From
 		if triggerRef == nil {
-			triggerRef = buildutil.GetInputReference(bc.Spec.Strategy)
+			triggerRef = buildapi.GetInputReference(bc.Spec.Strategy)
 			if triggerRef == nil || triggerRef.Kind != "ImageStreamTag" {
 				continue
 			}
@@ -353,7 +353,7 @@ func (g *BuildGenerator) updateImageTriggers(ctx apirequest.Context, bc *buildap
 
 		triggerImageRef := trigger.ImageChange.From
 		if triggerImageRef == nil {
-			triggerImageRef = buildutil.GetInputReference(bc.Spec.Strategy)
+			triggerImageRef = buildapi.GetInputReference(bc.Spec.Strategy)
 		}
 		if triggerImageRef == nil {
 			glog.Warningf("Could not get ImageStream reference for default ImageChangeTrigger on BuildConfig %s/%s", bc.Namespace, bc.Name)
@@ -536,7 +536,7 @@ func (g *BuildGenerator) setBuildSourceImage(ctx apirequest.Context, builderSecr
 		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 := buildutil.GetInputReference(bcCopy.Spec.Strategy); strategyFrom != nil &&
+		if strategyFrom := buildapi.GetInputReference(bcCopy.Spec.Strategy); strategyFrom != nil &&
 			reflect.DeepEqual(sourceImage.From, *strategyFrom) &&
 			strategyImageChangeTrigger != nil {
 			sourceImageSpec = strategyImageChangeTrigger.LastTriggeredImageID
@@ -787,7 +787,7 @@ func resolveError(kind string, namespace string, name string, err error) error {
 // getNextBuildName returns name of the next build and increments BuildConfig's LastVersion.
 func getNextBuildName(buildConfig *buildapi.BuildConfig) string {
 	buildConfig.Status.LastVersion++
-	return namer.GetName(buildConfig.Name, strconv.FormatInt(buildConfig.Status.LastVersion, 10), kvalidation.DNS1123SubdomainMaxLength)
+	return apihelpers.GetName(buildConfig.Name, strconv.FormatInt(buildConfig.Status.LastVersion, 10), kvalidation.DNS1123SubdomainMaxLength)
 }
 
 //updateCustomImageEnv updates base image env variable reference with the new image for a custom build strategy.
@@ -875,7 +875,7 @@ func getNextBuildNameFromBuild(build *buildapi.Build, buildConfig *buildapi.Buil
 	if len(suffix) > 10 {
 		suffix = suffix[len(suffix)-10:]
 	}
-	return namer.GetName(buildName, suffix, kvalidation.DNS1123SubdomainMaxLength)
+	return apihelpers.GetName(buildName, suffix, kvalidation.DNS1123SubdomainMaxLength)
 
 }
 

pkg/build/generator/generator_test.go

@@ -18,7 +18,6 @@ import (
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
 	"github.com/openshift/origin/pkg/build/apis/build/validation"
 	mocks "github.com/openshift/origin/pkg/build/generator/test"
-	buildutil "github.com/openshift/origin/pkg/build/util"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 )
 
@@ -446,7 +445,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 = buildutil.GetInputReference(bc.Spec.Strategy)
+					from = buildapi.GetInputReference(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)

pkg/build/graph/edges.go

@@ -7,7 +7,6 @@ import (
 	osgraph "github.com/openshift/origin/pkg/api/graph"
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
 	buildgraph "github.com/openshift/origin/pkg/build/graph/nodes"
-	buildutil "github.com/openshift/origin/pkg/build/util"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	imagegraph "github.com/openshift/origin/pkg/image/graph/nodes"
 )
@@ -94,7 +93,7 @@ func AddInputEdges(g osgraph.MutableUniqueGraph, node *buildgraph.BuildConfigNod
 	if in := buildgraph.EnsureSourceRepositoryNode(g, node.BuildConfig.Spec.Source); in != nil {
 		g.AddEdge(in, node, BuildInputEdgeKind)
 	}
-	inputImage := buildutil.GetInputReference(node.BuildConfig.Spec.Strategy)
+	inputImage := buildapi.GetInputReference(node.BuildConfig.Spec.Strategy)
 	if input := imageRefNode(g, inputImage, node.BuildConfig); input != nil {
 		g.AddEdge(input, node, BuildInputImageEdgeKind)
 	}
@@ -108,7 +107,7 @@ func AddTriggerEdges(g osgraph.MutableUniqueGraph, node *buildgraph.BuildConfigN
 		}
 		from := trigger.ImageChange.From
 		if trigger.ImageChange.From == nil {
-			from = buildutil.GetInputReference(node.BuildConfig.Spec.Strategy)
+			from = buildapi.GetInputReference(node.BuildConfig.Spec.Strategy)
 		}
 		triggerNode := imageRefNode(g, from, node.BuildConfig)
 		g.AddEdge(triggerNode, node, BuildTriggerImageEdgeKind)

pkg/build/util/util.go

@@ -45,21 +45,6 @@ func GetBuildName(pod metav1.Object) string {
 	return pod.GetAnnotations()[buildapi.BuildAnnotation]
 }
 
-// GetInputReference returns the From ObjectReference associated with the
-// BuildStrategy.
-func GetInputReference(strategy buildapi.BuildStrategy) *kapi.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
-	}
-}
-
 // IsBuildComplete returns whether the provided build is complete or not
 func IsBuildComplete(build *buildapi.Build) bool {
 	return IsTerminalPhase(build.Status.Phase)

pkg/client/cache/index.go

@@ -5,7 +5,6 @@ import (
 
 	deployapi "github.com/openshift/origin/pkg/apps/apis/apps"
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
-	buildutil "github.com/openshift/origin/pkg/build/util"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 )
 
@@ -32,7 +31,7 @@ func ImageStreamReferenceIndexFunc(obj interface{}) ([]string, error) {
 			// explicit image reference, use the image referenced by the strategy
 			// because this is the default ICT.
 			if from == nil {
-				from = buildutil.GetInputReference(t.Spec.Strategy)
+				from = buildapi.GetInputReference(t.Spec.Strategy)
 				if from == nil || from.Kind != "ImageStreamTag" {
 					continue
 				}

pkg/generate/app/cmd/newapp.go

@@ -1017,7 +1017,7 @@ func (c *AppConfig) checkCircularReferences(objects app.Objects) error {
 		}
 
 		if bc, ok := obj.(*buildapi.BuildConfig); ok {
-			input := buildutil.GetInputReference(bc.Spec.Strategy)
+			input := buildapi.GetInputReference(bc.Spec.Strategy)
 			output := bc.Spec.Output.To
 
 			if output == nil || input == nil {

pkg/generate/app/imageref.go

@@ -14,11 +14,11 @@ import (
 	kvalidation "k8s.io/apimachinery/pkg/util/validation"
 	kapi "k8s.io/kubernetes/pkg/api"
 
+	"github.com/openshift/origin/pkg/api/apihelpers"
 	deployapi "github.com/openshift/origin/pkg/apps/apis/apps"
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	"github.com/openshift/origin/pkg/util/docker/dockerfile"
-	"github.com/openshift/origin/pkg/util/namer"
 )
 
 // ImageRefGenerator is an interface for generating ImageRefs
@@ -378,7 +378,7 @@ func (r *ImageRef) DeployableContainer() (container *kapi.Container, triggers []
 
 		// Create volume mounts with names based on container name
 		maxDigits := len(fmt.Sprintf("%d", len(r.Info.Config.Volumes)))
-		baseName := namer.GetName(container.Name, volumeNameInfix, kvalidation.LabelValueMaxLength-maxDigits-1)
+		baseName := apihelpers.GetName(container.Name, volumeNameInfix, kvalidation.LabelValueMaxLength-maxDigits-1)
 		i := 1
 		for volume := range r.Info.Config.Volumes {
 			r.HasEmptyDir = true