set default build prune limits for group api

pkg/build/api/types.go

@@ -80,11 +80,23 @@ const (
 	BuildCancelledEventReason = "BuildCancelled"
 	// BuildCancelledEventMessage is the message associated with the event registered when build is cancelled.
 	BuildCancelledEventMessage = "Build %s/%s has been cancelled"
+
+	// 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)
 )
 
-// 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"}
+)
 
 // +genclient=true
 

pkg/build/registry/buildconfig/etcd/etcd.go

@@ -24,9 +24,9 @@ func NewREST(optsGetter restoptions.Getter) (*REST, error) {
 		QualifiedResource: api.Resource("buildconfigs"),
 		PredicateFunc:     buildconfig.Matcher,
 
-		CreateStrategy: buildconfig.Strategy,
-		UpdateStrategy: buildconfig.Strategy,
-		DeleteStrategy: buildconfig.Strategy,
+		CreateStrategy: buildconfig.GroupStrategy,
+		UpdateStrategy: buildconfig.GroupStrategy,
+		DeleteStrategy: buildconfig.GroupStrategy,
 	}
 
 	options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: buildconfig.GetAttrs}

pkg/build/registry/buildconfig/strategy.go

@@ -9,6 +9,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	apirequest "k8s.io/apiserver/pkg/endpoints/request"
+	"k8s.io/apiserver/pkg/registry/rest"
 	kstorage "k8s.io/apiserver/pkg/storage"
 	"k8s.io/apiserver/pkg/storage/names"
 	kapi "k8s.io/kubernetes/pkg/api"
@@ -17,15 +18,28 @@ import (
 	"github.com/openshift/origin/pkg/build/api/validation"
 )
 
-// strategy implements behavior for BuildConfig objects
+var (
+	// GroupStrategy is the logic that applies when creating and updating BuildConfig objects
+	// in the Group api.
+	// This differs from the LegacyStrategy in that on create it will set a default build
+	// pruning limit value for both successful and failed builds.  This is new behavior that
+	// can only be introduced to users consuming the new group based api.
+	GroupStrategy = groupStrategy{strategy{kapi.Scheme, names.SimpleNameGenerator}}
+
+	// LegacyStrategy is the default logic that applies when creating BuildConfig objects.
+	// Specifically it will not set the default build pruning limit values because that was not
+	// part of the legacy api.
+	LegacyStrategy = legacyStrategy{strategy{kapi.Scheme, names.SimpleNameGenerator}}
+)
+
+// strategy implements most of the behavior for BuildConfig objects
+// It does not provide a PrepareForCreate implementation, that is expected
+// to be implemented by the child implementation.
 type strategy struct {
 	runtime.ObjectTyper
 	names.NameGenerator
 }
 
-// Strategy is the default logic that applies when creating and updating BuildConfig objects.
-var Strategy = strategy{kapi.Scheme, names.SimpleNameGenerator}
-
 func (strategy) NamespaceScoped() bool {
 	return true
 }
@@ -39,16 +53,17 @@ func (strategy) AllowUnconditionalUpdate() bool {
 	return false
 }
 
-// PrepareForCreate clears fields that are not allowed to be set by end users on creation.
-func (strategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) {
-	bc := obj.(*api.BuildConfig)
-	dropUnknownTriggers(bc)
-}
-
 // Canonicalize normalizes the object after validation.
 func (strategy) Canonicalize(obj runtime.Object) {
 }
 
+// PrepareForCreateCommon clears fields that are not allowed to be set by end users on creation.
+// This is shared by the Group and Legacy strategies.
+func (s strategy) PrepareForCreateCommon(ctx apirequest.Context, obj runtime.Object) {
+	bc := obj.(*api.BuildConfig)
+	dropUnknownTriggers(bc)
+}
+
 // PrepareForUpdate clears fields that are not allowed to be set by end users on update.
 func (strategy) PrepareForUpdate(ctx apirequest.Context, obj, old runtime.Object) {
 	newBC := obj.(*api.BuildConfig)
@@ -71,6 +86,45 @@ func (strategy) ValidateUpdate(ctx apirequest.Context, obj, old runtime.Object)
 	return validation.ValidateBuildConfigUpdate(obj.(*api.BuildConfig), old.(*api.BuildConfig))
 }
 
+// groupStrategy implements behavior for BuildConfig objects in the Group api
+type groupStrategy struct {
+	strategy
+}
+
+// PrepareForCreate delegates to the common strategy and sets default pruning limits
+func (s groupStrategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) {
+	s.PrepareForCreateCommon(ctx, obj)
+
+	bc := obj.(*api.BuildConfig)
+	if bc.Spec.SuccessfulBuildsHistoryLimit == nil {
+		v := api.DefaultSuccessfulBuildsHistoryLimit
+		bc.Spec.SuccessfulBuildsHistoryLimit = &v
+	}
+	if bc.Spec.FailedBuildsHistoryLimit == nil {
+		v := api.DefaultFailedBuildsHistoryLimit
+		bc.Spec.FailedBuildsHistoryLimit = &v
+	}
+}
+
+// legacyStrategy implements behavior for BuildConfig objects in the legacy api
+type legacyStrategy struct {
+	strategy
+}
+
+// PrepareForCreate delegates to the common strategy.
+func (s legacyStrategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) {
+	s.PrepareForCreateCommon(ctx, obj)
+
+	// legacy buildconfig api does not apply default pruning values, to maintain
+	// backwards compatibility.
+
+}
+
+// DefaultGarbageCollectionPolicy for legacy buildconfigs will orphan dependents.
+func (s legacyStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy {
+	return rest.OrphanDependents
+}
+
 // GetAttrs returns labels and fields of a given object for filtering purposes
 func GetAttrs(obj runtime.Object) (labels.Set, fields.Set, error) {
 	buildConfig, ok := obj.(*api.BuildConfig)

pkg/cmd/cli/cmd/exporter.go

@@ -148,7 +148,7 @@ func (e *DefaultExporter) Export(obj runtime.Object, exact bool) error {
 		return deployrest.Strategy.Export(ctx, obj, exact)
 
 	case *buildapi.BuildConfig:
-		buildconfigrest.Strategy.PrepareForCreate(ctx, obj)
+		buildconfigrest.GroupStrategy.PrepareForCreate(ctx, obj)
 		// TODO: should be handled by prepare for create
 		t.Status.LastVersion = 0
 		for i := range t.Spec.Triggers {

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"
 )
@@ -197,7 +198,8 @@ func LegacyStorage(storage map[schema.GroupVersion]map[string]rest.Storage) map[
 				case "buildConfigs":
 					restStorage := s.(*buildconfigetcd.REST)
 					store := *restStorage.Store
-					store.DeleteStrategy = orphanByDefault(store.DeleteStrategy)
+					store.DeleteStrategy = buildconfig.LegacyStrategy
+					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")
+	})
+
 })