Merge pull request #11910 from guangxuli/openshift_refactor_generator

Merged by openshift-bot

pkg/authorization/registry/clusterpolicy/etcd/etcd.go

@@ -17,7 +17,7 @@ type REST struct {
 	*registry.Store
 }
 
-// NewStorage returns a RESTStorage object that will work against nodes.
+// NewStorage returns a RESTStorage object that will work against ClusterPolicy.
 func NewStorage(optsGetter restoptions.Getter) (*REST, error) {
 
 	store := &registry.Store{

pkg/authorization/registry/clusterpolicybinding/etcd/etcd.go

@@ -17,7 +17,7 @@ type REST struct {
 	*registry.Store
 }
 
-// NewStorage returns a RESTStorage object that will work against nodes.
+// NewStorage returns a RESTStorage object that will work against ClusterPolicyBinding.
 func NewStorage(optsGetter restoptions.Getter) (*REST, error) {
 
 	store := &registry.Store{

pkg/authorization/registry/policy/etcd/etcd.go

@@ -17,7 +17,7 @@ type REST struct {
 	*registry.Store
 }
 
-// NewStorage returns a RESTStorage object that will work against nodes.
+// NewStorage returns a RESTStorage object that will work against Policy objects.
 func NewStorage(optsGetter restoptions.Getter) (*REST, error) {
 	store := &registry.Store{
 		NewFunc:           func() runtime.Object { return &authorizationapi.Policy{} },

pkg/authorization/registry/policybinding/etcd/etcd.go

@@ -17,7 +17,7 @@ type REST struct {
 	*registry.Store
 }
 
-// NewStorage returns a RESTStorage object that will work against nodes.
+// NewStorage returns a RESTStorage object that will work against PolicyBinding objects.
 func NewStorage(optsGetter restoptions.Getter) (*REST, error) {
 	store := &registry.Store{
 		NewFunc:           func() runtime.Object { return &authorizationapi.PolicyBinding{} },

pkg/build/generator/generator.go

@@ -383,17 +383,13 @@ func (g *BuildGenerator) createBuild(ctx kapi.Context, build *buildapi.Build) (*
 // the current build strategy with a binary artifact for this specific build.
 // Takes a BuildConfig to base the build on, and an optional SourceRevision to build.
 func (g *BuildGenerator) generateBuildFromConfig(ctx kapi.Context, bc *buildapi.BuildConfig, revision *buildapi.SourceRevision, binary *buildapi.BinaryBuildSource) (*buildapi.Build, error) {
-	serviceAccount := bc.Spec.ServiceAccount
-	if len(serviceAccount) == 0 {
-		serviceAccount = g.DefaultServiceAccountName
-	}
-	if len(serviceAccount) == 0 {
-		serviceAccount = bootstrappolicy.BuilderServiceAccountName
-	}
+
 	// Need to copy the buildConfig here so that it doesn't share pointers with
 	// the build object which could be (will be) modified later.
+	buildName := getNextBuildName(bc)
 	obj, _ := kapi.Scheme.Copy(bc)
 	bcCopy := obj.(*buildapi.BuildConfig)
+	serviceAccount := getServiceAccount(bcCopy, g.DefaultServiceAccountName)
 	build := &buildapi.Build{
 		Spec: buildapi.BuildSpec{
 			CommonSpec: buildapi.CommonSpec{
@@ -409,70 +405,65 @@ func (g *BuildGenerator) generateBuildFromConfig(ctx kapi.Context, bc *buildapi.
 			},
 		},
 		ObjectMeta: kapi.ObjectMeta{
+			Name:   buildName,
 			Labels: bcCopy.Labels,
 		},
 		Status: buildapi.BuildStatus{
 			Phase: buildapi.BuildPhaseNew,
 			Config: &kapi.ObjectReference{
 				Kind:      "BuildConfig",
-				Name:      bc.Name,
-				Namespace: bc.Namespace,
+				Name:      bcCopy.Name,
+				Namespace: bcCopy.Namespace,
 			},
 		},
 	}
-	if binary != nil {
-		build.Spec.Source.Git = nil
-		build.Spec.Source.Binary = binary
-		if build.Spec.Source.Dockerfile != nil && binary.AsFile == "Dockerfile" {
-			build.Spec.Source.Dockerfile = nil
-		}
-	} else {
-		// must explicitly set this because we copied the source values from the buildconfig.
-		build.Spec.Source.Binary = nil
-	}
 
-	build.Name = getNextBuildName(bc)
-	if build.Annotations == nil {
-		build.Annotations = make(map[string]string)
-	}
-	build.Annotations[buildapi.BuildNumberAnnotation] = strconv.FormatInt(bc.Status.LastVersion, 10)
-	build.Annotations[buildapi.BuildConfigAnnotation] = bcCopy.Name
-	if build.Labels == nil {
-		build.Labels = make(map[string]string)
+	setBuildSource(binary, build)
+	setBuildAnnotationAndLabel(bcCopy, build)
+
+	var builderSecrets []kapi.Secret
+	var err error
+	if builderSecrets, err = g.FetchServiceAccountSecrets(bcCopy.Namespace, serviceAccount); err != nil {
+		return nil, err
 	}
-	build.Labels[buildapi.BuildConfigLabelDeprecated] = buildapi.LabelValue(bcCopy.Name)
-	build.Labels[buildapi.BuildConfigLabel] = buildapi.LabelValue(bcCopy.Name)
-	build.Labels[buildapi.BuildRunPolicyLabel] = string(bc.Spec.RunPolicy)
+	setBuildPushSecret(g.resolveImageSecret(ctx, builderSecrets, build.Spec.Output.To, bcCopy.Namespace), &build.Spec.Output)
 
-	builderSecrets, err := g.FetchServiceAccountSecrets(bc.Namespace, serviceAccount)
-	if err != nil {
+	// Resolve image source if present
+	if err = g.setBuildSourceImage(ctx, builderSecrets, bcCopy, &build.Spec.Source); err != nil {
 		return nil, err
 	}
-	if build.Spec.Output.PushSecret == nil {
-		build.Spec.Output.PushSecret = g.resolveImageSecret(ctx, builderSecrets, build.Spec.Output.To, bc.Namespace)
+	if err = g.setBaseImageAndPullSecretForBuildStrategy(ctx, builderSecrets, bcCopy, &build.Spec.Strategy); err != nil {
+		return nil, err
 	}
-	strategyImageChangeTrigger := getStrategyImageChangeTrigger(bc)
 
-	// Resolve image source if present
-	for i, sourceImage := range build.Spec.Source.Images {
+	return build, nil
+}
+
+// setBuildSourceImage set BuildSource Image item for new build
+func (g *BuildGenerator) setBuildSourceImage(ctx kapi.Context, builderSecrets []kapi.Secret, bcCopy *buildapi.BuildConfig, Source *buildapi.BuildSource) error {
+	var err error
+
+	strategyImageChangeTrigger := getStrategyImageChangeTrigger(bcCopy)
+	for i, sourceImage := range Source.Images {
 		if sourceImage.PullSecret == nil {
-			sourceImage.PullSecret = g.resolveImageSecret(ctx, builderSecrets, &sourceImage.From, bc.Namespace)
+			sourceImage.PullSecret = g.resolveImageSecret(ctx, builderSecrets, &sourceImage.From, bcCopy.Namespace)
 		}
 
 		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(bc.Spec.Strategy); strategyFrom != nil && reflect.DeepEqual(sourceImage.From, *strategyFrom) &&
+		if strategyFrom := buildutil.GetInputReference(bcCopy.Spec.Strategy); strategyFrom != nil &&
+			reflect.DeepEqual(sourceImage.From, *strategyFrom) &&
 			strategyImageChangeTrigger != nil {
 			sourceImageSpec = strategyImageChangeTrigger.LastTriggeredImageID
 		} else {
-			refImageChangeTrigger := getImageChangeTriggerForRef(bc, &sourceImage.From)
+			refImageChangeTrigger := getImageChangeTriggerForRef(bcCopy, &sourceImage.From)
 			// if there is no trigger associated with this imagesource, resolve the imagesource reference now.
 			// otherwise use the imageid from the imagesource trigger.
 			if refImageChangeTrigger == nil {
-				sourceImageSpec, err = g.resolveImageStreamReference(ctx, sourceImage.From, bc.Namespace)
+				sourceImageSpec, err = g.resolveImageStreamReference(ctx, sourceImage.From, bcCopy.Namespace)
 				if err != nil {
-					return nil, err
+					return err
 				}
 			} else {
 				sourceImageSpec = refImageChangeTrigger.LastTriggeredImageID
@@ -482,84 +473,90 @@ func (g *BuildGenerator) generateBuildFromConfig(ctx kapi.Context, bc *buildapi.
 		sourceImage.From.Kind = "DockerImage"
 		sourceImage.From.Name = sourceImageSpec
 		sourceImage.From.Namespace = ""
-		build.Spec.Source.Images[i] = sourceImage
+		Source.Images[i] = sourceImage
 	}
 
-	// If the Build is using a From reference instead of a resolved image, we need to resolve that From
-	// reference to a valid image so we can run the build.  Builds do not consume ImageStream references,
-	// only image specs.
+	return nil
+}
+
+// setBaseImageAndPullSecretForBuildStrategy sets base image and pullSecret items used in buildStragety for new builds
+func (g *BuildGenerator) setBaseImageAndPullSecretForBuildStrategy(ctx kapi.Context, builderSecrets []kapi.Secret, bcCopy *buildapi.BuildConfig, strategy *buildapi.BuildStrategy) error {
+	var err error
 	var image string
-	if strategyImageChangeTrigger != nil {
+
+	if strategyImageChangeTrigger := getStrategyImageChangeTrigger(bcCopy); strategyImageChangeTrigger != nil {
 		image = strategyImageChangeTrigger.LastTriggeredImageID
 	}
-
+	// If the Build is using a From reference instead of a resolved image, we need to resolve that From
+	// reference to a valid image so we can run the build.  Builds do not consume ImageStream references,
+	// only image specs.
 	switch {
-	case build.Spec.Strategy.SourceStrategy != nil:
+	case strategy.SourceStrategy != nil:
 		if image == "" {
-			image, err = g.resolveImageStreamReference(ctx, build.Spec.Strategy.SourceStrategy.From, build.Status.Config.Namespace)
+			image, err = g.resolveImageStreamReference(ctx, strategy.SourceStrategy.From, bcCopy.Namespace)
 			if err != nil {
-				return nil, err
+				return err
 			}
 		}
-		build.Spec.Strategy.SourceStrategy.From = kapi.ObjectReference{
+		strategy.SourceStrategy.From = kapi.ObjectReference{
 			Kind: "DockerImage",
 			Name: image,
 		}
-		if build.Spec.Strategy.SourceStrategy.RuntimeImage != nil {
-			runtimeImageName, err := g.resolveImageStreamReference(ctx, *build.Spec.Strategy.SourceStrategy.RuntimeImage, build.Status.Config.Namespace)
+		if strategy.SourceStrategy.RuntimeImage != nil {
+			runtimeImageName, err := g.resolveImageStreamReference(ctx, *strategy.SourceStrategy.RuntimeImage, bcCopy.Namespace)
 			if err != nil {
-				return nil, err
+				return err
 			}
-			build.Spec.Strategy.SourceStrategy.RuntimeImage = &kapi.ObjectReference{
+			strategy.SourceStrategy.RuntimeImage = &kapi.ObjectReference{
 				Kind: "DockerImage",
 				Name: runtimeImageName,
 			}
 		}
-		if build.Spec.Strategy.SourceStrategy.PullSecret == nil {
+		if strategy.SourceStrategy.PullSecret == nil {
 			// we have 3 different variations:
 			// 1) builder and runtime images use the same secret => use builder image secret
 			// 2) builder and runtime images use different secrets => use builder image secret
 			// 3) builder doesn't need a secret but runtime image requires it => use runtime image secret
 			// The case when both of the images don't use secret (equals to nil) is covered by the first variant.
-			pullSecret := g.resolveImageSecret(ctx, builderSecrets, &build.Spec.Strategy.SourceStrategy.From, bc.Namespace)
+			pullSecret := g.resolveImageSecret(ctx, builderSecrets, &strategy.SourceStrategy.From, bcCopy.Namespace)
 			if pullSecret == nil {
-				pullSecret = g.resolveImageSecret(ctx, builderSecrets, build.Spec.Strategy.SourceStrategy.RuntimeImage, bc.Namespace)
+				pullSecret = g.resolveImageSecret(ctx, builderSecrets, strategy.SourceStrategy.RuntimeImage, bcCopy.Namespace)
 			}
 
-			build.Spec.Strategy.SourceStrategy.PullSecret = pullSecret
+			strategy.SourceStrategy.PullSecret = pullSecret
 		}
-	case build.Spec.Strategy.DockerStrategy != nil &&
-		build.Spec.Strategy.DockerStrategy.From != nil:
+	case strategy.DockerStrategy != nil &&
+		strategy.DockerStrategy.From != nil:
 		if image == "" {
-			image, err = g.resolveImageStreamReference(ctx, *build.Spec.Strategy.DockerStrategy.From, build.Status.Config.Namespace)
+			image, err = g.resolveImageStreamReference(ctx, *strategy.DockerStrategy.From, bcCopy.Namespace)
 			if err != nil {
-				return nil, err
+				return err
 			}
 		}
-		build.Spec.Strategy.DockerStrategy.From = &kapi.ObjectReference{
+		strategy.DockerStrategy.From = &kapi.ObjectReference{
 			Kind: "DockerImage",
 			Name: image,
 		}
-		if build.Spec.Strategy.DockerStrategy.PullSecret == nil {
-			build.Spec.Strategy.DockerStrategy.PullSecret = g.resolveImageSecret(ctx, builderSecrets, build.Spec.Strategy.DockerStrategy.From, bc.Namespace)
+		if strategy.DockerStrategy.PullSecret == nil {
+			strategy.DockerStrategy.PullSecret = g.resolveImageSecret(ctx, builderSecrets, strategy.DockerStrategy.From, bcCopy.Namespace)
 		}
-	case build.Spec.Strategy.CustomStrategy != nil:
+	case strategy.CustomStrategy != nil:
 		if image == "" {
-			image, err = g.resolveImageStreamReference(ctx, build.Spec.Strategy.CustomStrategy.From, build.Status.Config.Namespace)
+			image, err = g.resolveImageStreamReference(ctx, strategy.CustomStrategy.From, bcCopy.Namespace)
 			if err != nil {
-				return nil, err
+				return err
 			}
 		}
-		build.Spec.Strategy.CustomStrategy.From = kapi.ObjectReference{
+		strategy.CustomStrategy.From = kapi.ObjectReference{
 			Kind: "DockerImage",
 			Name: image,
 		}
-		if build.Spec.Strategy.CustomStrategy.PullSecret == nil {
-			build.Spec.Strategy.CustomStrategy.PullSecret = g.resolveImageSecret(ctx, builderSecrets, &build.Spec.Strategy.CustomStrategy.From, bc.Namespace)
+		if strategy.CustomStrategy.PullSecret == nil {
+			strategy.CustomStrategy.PullSecret = g.resolveImageSecret(ctx, builderSecrets, &strategy.CustomStrategy.From, bcCopy.Namespace)
 		}
-		updateCustomImageEnv(build.Spec.Strategy.CustomStrategy, image)
+		updateCustomImageEnv(strategy.CustomStrategy, image)
 	}
-	return build, nil
+	return nil
 }
 
 // resolveImageStreamReference looks up the ImageStream[Tag/Image] and converts it to a
@@ -804,3 +801,52 @@ func getImageChangeTriggerForRef(bc *buildapi.BuildConfig, ref *kapi.ObjectRefer
 	}
 	return nil
 }
+
+//getServiceAccount returns serviceaccount used by new build
+func getServiceAccount(buildConfig *buildapi.BuildConfig, defaultServiceAccount string) string {
+	serviceAccount := buildConfig.Spec.ServiceAccount
+	if len(serviceAccount) == 0 {
+		serviceAccount = defaultServiceAccount
+	}
+	if len(serviceAccount) == 0 {
+		serviceAccount = bootstrappolicy.BuilderServiceAccountName
+	}
+	return serviceAccount
+}
+
+//setBuildSource update build source by bianry status
+func setBuildSource(binary *buildapi.BinaryBuildSource, build *buildapi.Build) {
+	if binary != nil {
+		build.Spec.Source.Git = nil
+		build.Spec.Source.Binary = binary
+		if build.Spec.Source.Dockerfile != nil && binary.AsFile == "Dockerfile" {
+			build.Spec.Source.Dockerfile = nil
+		}
+	} else {
+		// must explicitly set this because we copied the source values from the buildconfig.
+		build.Spec.Source.Binary = nil
+	}
+}
+
+//setBuildAnnotationAndLabel set annotations and label info of this build
+func setBuildAnnotationAndLabel(bcCopy *buildapi.BuildConfig, build *buildapi.Build) {
+	if build.Annotations == nil {
+		build.Annotations = make(map[string]string)
+	}
+	//bcCopy.Status.LastVersion has been increased
+	build.Annotations[buildapi.BuildNumberAnnotation] = strconv.FormatInt(bcCopy.Status.LastVersion, 10)
+	build.Annotations[buildapi.BuildConfigAnnotation] = bcCopy.Name
+	if build.Labels == nil {
+		build.Labels = make(map[string]string)
+	}
+	build.Labels[buildapi.BuildConfigLabelDeprecated] = buildapi.LabelValue(bcCopy.Name)
+	build.Labels[buildapi.BuildConfigLabel] = buildapi.LabelValue(bcCopy.Name)
+	build.Labels[buildapi.BuildRunPolicyLabel] = string(bcCopy.Spec.RunPolicy)
+}
+
+// setBuildPushSecret set push secret for new build
+func setBuildPushSecret(pushSecret *kapi.LocalObjectReference, output *buildapi.BuildOutput) {
+	if output.PushSecret == nil {
+		output.PushSecret = pushSecret
+	}
+}

pkg/quota/registry/clusterresourcequota/etcd.go

@@ -18,7 +18,7 @@ type REST struct {
 	*registry.Store
 }
 
-// NewStorage returns a RESTStorage object that will work against nodes.
+// NewStorage returns a RESTStorage object that will work against ClusterResourceQuota objects.
 func NewStorage(optsGetter restoptions.Getter) (*REST, error) {
 	store, err := makeStore(optsGetter)
 	if err != nil {