set default build prune limits for group api

Author: bparees

pkg/build/api/types.go

@@ -82,9 +82,21 @@ const (
 	BuildCancelledEventMessage = "Build %s/%s has been cancelled"
 )
 
-// WhitelistEnvVarNames is a list of environment variable keys that are allowed to be set by the
-// user on the build pod.
-var WhitelistEnvVarNames = [2]string{"BUILD_LOGLEVEL", "GIT_SSL_NO_VERIFY"}
+var (
+	// WhitelistEnvVarNames is a list of environment variable keys that are allowed to be set by the
+	// user on the build pod.
+	WhitelistEnvVarNames = [2]string{"BUILD_LOGLEVEL", "GIT_SSL_NO_VERIFY"}
+
+	// DefaultSuccessfulBuildsHistoryLimit is the default number of successful builds to retain
+	// if the buildconfig does not specify a value.  This only applies to buildconfigs created
+	// via the new group api resource, not the legacy resource.
+	DefaultSuccessfulBuildsHistoryLimit = int32(5)
+
+	// DefaultFailedBuildsHistoryLimit is the default number of failed builds to retain
+	// if the buildconfig does not specify a value.  This only applies to buildconfigs created
+	// via the new group api resource, not the legacy resource.
+	DefaultFailedBuildsHistoryLimit = int32(5)
+)
 
 // +genclient=true
 

pkg/build/registry/buildconfig/strategy.go

@@ -21,10 +21,14 @@ import (
 type strategy struct {
 	runtime.ObjectTyper
 	names.NameGenerator
+	legacy bool
 }
 
 // Strategy is the default logic that applies when creating and updating BuildConfig objects.
-var Strategy = strategy{kapi.Scheme, names.SimpleNameGenerator}
+var Strategy = strategy{kapi.Scheme, names.SimpleNameGenerator, false}
+
+// LegacyStrategy is the default logic that applies when creating and updating BuildConfig objects.
+var LegacyStrategy = strategy{kapi.Scheme, names.SimpleNameGenerator, true}
 
 func (strategy) NamespaceScoped() bool {
 	return true
@@ -40,9 +44,19 @@ func (strategy) AllowUnconditionalUpdate() bool {
 }
 
 // PrepareForCreate clears fields that are not allowed to be set by end users on creation.
-func (strategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) {
+func (s strategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) {
 	bc := obj.(*api.BuildConfig)
 	dropUnknownTriggers(bc)
+
+	// legacy buildconfig api does not apply default pruning values.
+	if !s.legacy {
+		if bc.Spec.SuccessfulBuildsHistoryLimit == nil {
+			bc.Spec.SuccessfulBuildsHistoryLimit = &api.DefaultSuccessfulBuildsHistoryLimit
+		}
+		if bc.Spec.FailedBuildsHistoryLimit == nil {
+			bc.Spec.FailedBuildsHistoryLimit = &api.DefaultFailedBuildsHistoryLimit
+		}
+	}
 }
 
 // Canonicalize normalizes the object after validation.

pkg/cmd/server/origin/legacy.go

@@ -5,6 +5,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/registry/rest"
 
+	buildconfig "github.com/openshift/origin/pkg/build/registry/buildconfig"
 	buildconfigetcd "github.com/openshift/origin/pkg/build/registry/buildconfig/etcd"
 	deploymentconfigetcd "github.com/openshift/origin/pkg/deploy/registry/deployconfig/etcd"
 )
@@ -198,6 +199,7 @@ func LegacyStorage(storage map[schema.GroupVersion]map[string]rest.Storage) map[
 					restStorage := s.(*buildconfigetcd.REST)
 					store := *restStorage.Store
 					store.DeleteStrategy = orphanByDefault(store.DeleteStrategy)
+					store.CreateStrategy = buildconfig.LegacyStrategy
 					legacyStorage[resource] = &buildconfigetcd.REST{Store: &store}
 				case "deploymentConfigs":
 					restStorage := s.(*deploymentconfigetcd.REST)

test/extended/builds/build_pruning.go

@@ -7,9 +7,10 @@ import (
 	g "github.com/onsi/ginkgo"
 	o "github.com/onsi/gomega"
 
-	exutil "github.com/openshift/origin/test/extended/util"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	buildapi "github.com/openshift/origin/pkg/build/api"
+	exutil "github.com/openshift/origin/test/extended/util"
 )
 
 var _ = g.Describe("[builds][pruning] prune builds based on settings in the buildconfig", func() {
@@ -19,6 +20,8 @@ var _ = g.Describe("[builds][pruning] prune builds based on settings in the buil
 		successfulBuildConfig = filepath.Join(buildPruningBaseDir, "successful-build-config.yaml")
 		failedBuildConfig     = filepath.Join(buildPruningBaseDir, "failed-build-config.yaml")
 		erroredBuildConfig    = filepath.Join(buildPruningBaseDir, "errored-build-config.yaml")
+		legacyBuildConfig     = filepath.Join(buildPruningBaseDir, "default-legacy-build-config.yaml")
+		groupBuildConfig      = filepath.Join(buildPruningBaseDir, "default-group-build-config.yaml")
 		oc                    = exutil.NewCLI("build-pruning", exutil.KubeConfigPath())
 	)
 
@@ -149,4 +152,34 @@ var _ = g.Describe("[builds][pruning] prune builds based on settings in the buil
 
 	})
 
+	g.It("buildconfigs should have a default history limit set when created via the group api", func() {
+
+		g.By("creating a build config with the group api")
+		err := oc.Run("create").Args("-f", groupBuildConfig).Execute()
+		o.Expect(err).NotTo(o.HaveOccurred())
+
+		buildConfig, err := oc.Client().BuildConfigs(oc.Namespace()).Get("myphp", metav1.GetOptions{})
+		if err != nil {
+			fmt.Fprintf(g.GinkgoWriter, "%v", err)
+		}
+		o.Expect(buildConfig.Spec.SuccessfulBuildsHistoryLimit).NotTo(o.BeNil(), "the buildconfig should have the default successful history limit set")
+		o.Expect(buildConfig.Spec.FailedBuildsHistoryLimit).NotTo(o.BeNil(), "the buildconfig should have the default failed history limit set")
+		o.Expect(*buildConfig.Spec.SuccessfulBuildsHistoryLimit).To(o.Equal(buildapi.DefaultSuccessfulBuildsHistoryLimit), "the buildconfig should have the default successful history limit set")
+		o.Expect(*buildConfig.Spec.FailedBuildsHistoryLimit).To(o.Equal(buildapi.DefaultFailedBuildsHistoryLimit), "the buildconfig should have the default failed history limit set")
+	})
+
+	g.It("buildconfigs should not have a default history limit set when created via the legacy api", func() {
+
+		g.By("creating a build config with the legacy api")
+		err := oc.Run("create").Args("-f", legacyBuildConfig).Execute()
+		o.Expect(err).NotTo(o.HaveOccurred())
+
+		buildConfig, err := oc.Client().BuildConfigs(oc.Namespace()).Get("myphp", metav1.GetOptions{})
+		if err != nil {
+			fmt.Fprintf(g.GinkgoWriter, "%v", err)
+		}
+		o.Expect(buildConfig.Spec.SuccessfulBuildsHistoryLimit).To(o.BeNil(), "the buildconfig should not have the default successful history limit set")
+		o.Expect(buildConfig.Spec.FailedBuildsHistoryLimit).To(o.BeNil(), "the buildconfig should not have the default failed history limit set")
+	})
+
 })

test/extended/testdata/bindata.go

@@ -0,0 +1,16 @@
+apiVersion: build.openshift.io/v1
+kind: BuildConfig
+metadata:
+  name: myphp
+spec:
+  source:
+    type: Git
+    git:
+      uri: 'https://github.com/openshift/cakephp-ex.git'
+  strategy:
+    type: Source
+    sourceStrategy:
+      from:
+        kind: ImageStreamTag
+        namespace: openshift
+        name: 'php:7.0'

test/extended/testdata/build-pruning/default-group-build-config.yaml

@@ -0,0 +1,16 @@
+apiVersion: v1
+kind: BuildConfig
+metadata:
+  name: myphp
+spec:
+  source:
+    type: Git
+    git:
+      uri: 'https://github.com/openshift/cakephp-ex.git'
+  strategy:
+    type: Source
+    sourceStrategy:
+      from:
+        kind: ImageStreamTag
+        namespace: openshift
+        name: 'php:7.0'

test/extended/testdata/build-pruning/default-legacy-build-config.yaml

@@ -8,25 +8,14 @@ metadata:
     openshift.io/generated-by: OpenShiftWebConsole
 spec:
   failedBuildsHistoryLimit: 2
-  triggers: {}
-  runPolicy: Serial
   source:
     type: Git
     git:
       uri: 'https://github.com/openshift/non-working-example.git'
-      ref: master
   strategy:
     type: Source
     sourceStrategy:
       from:
         kind: ImageStreamTag
         namespace: openshift
         name: 'php:7.0'
-  output:
-    to:
-      kind: ImageStreamTag
-      name: 'myphp:latest'
-  resources: {}
-  postCommit: {}
-  nodeSelector: null
-status:

test/extended/testdata/build-pruning/failed-build-config.yaml

@@ -8,25 +8,14 @@ metadata:
     openshift.io/generated-by: OpenShiftWebConsole
 spec:
   successfulBuildsHistoryLimit: 2
-  triggers: {}
-  runPolicy: Serial
   source:
     type: Git
     git:
       uri: 'https://github.com/openshift/cakephp-ex.git'
-      ref: master
   strategy:
     type: Source
     sourceStrategy:
       from:
         kind: ImageStreamTag
         namespace: openshift
         name: 'php:7.0'
-  output:
-    to:
-      kind: ImageStreamTag
-      name: 'myphp:latest'
-  resources: {}
-  postCommit: {}
-  nodeSelector: null
-status: