Skip to content

Commit c60ad43

Browse files
bpareesbparees
committed
set default build prune limits for group api
1 parent 7461c10 commit c60ad43

File tree

9 files changed

+172
-51
lines changed

9 files changed

+172
-51
lines changed

‎pkg/build/api/types.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,21 @@ const (
8282
BuildCancelledEventMessage = "Build %s/%s has been cancelled"
8383
)
8484

85-
// WhitelistEnvVarNames is a list of environment variable keys that are allowed to be set by the
86-
// user on the build pod.
87-
var WhitelistEnvVarNames = [2]string{"BUILD_LOGLEVEL", "GIT_SSL_NO_VERIFY"}
85+
var (
86+
// WhitelistEnvVarNames is a list of environment variable keys that are allowed to be set by the
87+
// user on the build pod.
88+
WhitelistEnvVarNames = [2]string{"BUILD_LOGLEVEL", "GIT_SSL_NO_VERIFY"}
89+
90+
// DefaultSuccessfulBuildsHistoryLimit is the default number of successful builds to retain
91+
// if the buildconfig does not specify a value. This only applies to buildconfigs created
92+
// via the new group api resource, not the legacy resource.
93+
DefaultSuccessfulBuildsHistoryLimit = int32(5)
94+
95+
// DefaultFailedBuildsHistoryLimit is the default number of failed builds to retain
96+
// if the buildconfig does not specify a value. This only applies to buildconfigs created
97+
// via the new group api resource, not the legacy resource.
98+
DefaultFailedBuildsHistoryLimit = int32(5)
99+
)
88100

89101
// +genclient=true
90102

‎pkg/build/registry/buildconfig/strategy.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,14 @@ import (
2121
type strategy struct {
2222
runtime.ObjectTyper
2323
names.NameGenerator
24+
legacy bool
2425
}
2526

2627
// Strategy is the default logic that applies when creating and updating BuildConfig objects.
27-
var Strategy = strategy{kapi.Scheme, names.SimpleNameGenerator}
28+
var Strategy = strategy{kapi.Scheme, names.SimpleNameGenerator, false}
29+
30+
// LegacyStrategy is the default logic that applies when creating and updating BuildConfig objects.
31+
var LegacyStrategy = strategy{kapi.Scheme, names.SimpleNameGenerator, true}
2832

2933
func (strategy) NamespaceScoped() bool {
3034
return true
@@ -40,9 +44,19 @@ func (strategy) AllowUnconditionalUpdate() bool {
4044
}
4145

4246
// PrepareForCreate clears fields that are not allowed to be set by end users on creation.
43-
func (strategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) {
47+
func (s strategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) {
4448
bc := obj.(*api.BuildConfig)
4549
dropUnknownTriggers(bc)
50+
51+
// legacy buildconfig api does not apply default pruning values.
52+
if !s.legacy {
53+
if bc.Spec.SuccessfulBuildsHistoryLimit == nil {
54+
bc.Spec.SuccessfulBuildsHistoryLimit = &api.DefaultSuccessfulBuildsHistoryLimit
55+
}
56+
if bc.Spec.FailedBuildsHistoryLimit == nil {
57+
bc.Spec.FailedBuildsHistoryLimit = &api.DefaultFailedBuildsHistoryLimit
58+
}
59+
}
4660
}
4761

4862
// Canonicalize normalizes the object after validation.

‎pkg/cmd/server/origin/legacy.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"k8s.io/apimachinery/pkg/util/sets"
66
"k8s.io/apiserver/pkg/registry/rest"
77

8+
buildconfig "github.com/openshift/origin/pkg/build/registry/buildconfig"
89
buildconfigetcd "github.com/openshift/origin/pkg/build/registry/buildconfig/etcd"
910
deploymentconfigetcd "github.com/openshift/origin/pkg/deploy/registry/deployconfig/etcd"
1011
)
@@ -198,6 +199,7 @@ func LegacyStorage(storage map[schema.GroupVersion]map[string]rest.Storage) map[
198199
restStorage := s.(*buildconfigetcd.REST)
199200
store := *restStorage.Store
200201
store.DeleteStrategy = orphanByDefault(store.DeleteStrategy)
202+
store.CreateStrategy = buildconfig.LegacyStrategy
201203
legacyStorage[resource] = &buildconfigetcd.REST{Store: &store}
202204
case "deploymentConfigs":
203205
restStorage := s.(*deploymentconfigetcd.REST)

‎test/extended/builds/build_pruning.go

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import (
77
g "github.com/onsi/ginkgo"
88
o "github.com/onsi/gomega"
99

10-
exutil "github.com/openshift/origin/test/extended/util"
11-
1210
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
12+
buildapi "github.com/openshift/origin/pkg/build/api"
13+
exutil "github.com/openshift/origin/test/extended/util"
1314
)
1415

1516
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
1920
successfulBuildConfig = filepath.Join(buildPruningBaseDir, "successful-build-config.yaml")
2021
failedBuildConfig = filepath.Join(buildPruningBaseDir, "failed-build-config.yaml")
2122
erroredBuildConfig = filepath.Join(buildPruningBaseDir, "errored-build-config.yaml")
23+
legacyBuildConfig = filepath.Join(buildPruningBaseDir, "default-legacy-build-config.yaml")
24+
groupBuildConfig = filepath.Join(buildPruningBaseDir, "default-group-build-config.yaml")
2225
oc = exutil.NewCLI("build-pruning", exutil.KubeConfigPath())
2326
)
2427

@@ -149,4 +152,34 @@ var _ = g.Describe("[builds][pruning] prune builds based on settings in the buil
149152

150153
})
151154

155+
g.It("buildconfigs should have a default history limit set when created via the group api", func() {
156+
157+
g.By("creating a build config with the group api")
158+
err := oc.Run("create").Args("-f", groupBuildConfig).Execute()
159+
o.Expect(err).NotTo(o.HaveOccurred())
160+
161+
buildConfig, err := oc.Client().BuildConfigs(oc.Namespace()).Get("myphp", metav1.GetOptions{})
162+
if err != nil {
163+
fmt.Fprintf(g.GinkgoWriter, "%v", err)
164+
}
165+
o.Expect(buildConfig.Spec.SuccessfulBuildsHistoryLimit).NotTo(o.BeNil(), "the buildconfig should have the default successful history limit set")
166+
o.Expect(buildConfig.Spec.FailedBuildsHistoryLimit).NotTo(o.BeNil(), "the buildconfig should have the default failed history limit set")
167+
o.Expect(*buildConfig.Spec.SuccessfulBuildsHistoryLimit).To(o.Equal(buildapi.DefaultSuccessfulBuildsHistoryLimit), "the buildconfig should have the default successful history limit set")
168+
o.Expect(*buildConfig.Spec.FailedBuildsHistoryLimit).To(o.Equal(buildapi.DefaultFailedBuildsHistoryLimit), "the buildconfig should have the default failed history limit set")
169+
})
170+
171+
g.It("buildconfigs should not have a default history limit set when created via the legacy api", func() {
172+
173+
g.By("creating a build config with the legacy api")
174+
err := oc.Run("create").Args("-f", legacyBuildConfig).Execute()
175+
o.Expect(err).NotTo(o.HaveOccurred())
176+
177+
buildConfig, err := oc.Client().BuildConfigs(oc.Namespace()).Get("myphp", metav1.GetOptions{})
178+
if err != nil {
179+
fmt.Fprintf(g.GinkgoWriter, "%v", err)
180+
}
181+
o.Expect(buildConfig.Spec.SuccessfulBuildsHistoryLimit).To(o.BeNil(), "the buildconfig should not have the default successful history limit set")
182+
o.Expect(buildConfig.Spec.FailedBuildsHistoryLimit).To(o.BeNil(), "the buildconfig should not have the default failed history limit set")
183+
})
184+
152185
})

‎test/extended/testdata/bindata.go

Lines changed: 72 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
apiVersion: build.openshift.io/v1
2+
kind: BuildConfig
3+
metadata:
4+
name: myphp
5+
spec:
6+
source:
7+
type: Git
8+
git:
9+
uri: 'https://github.com/openshift/cakephp-ex.git'
10+
strategy:
11+
type: Source
12+
sourceStrategy:
13+
from:
14+
kind: ImageStreamTag
15+
namespace: openshift
16+
name: 'php:7.0'

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
apiVersion: v1
2+
kind: BuildConfig
3+
metadata:
4+
name: myphp
5+
spec:
6+
source:
7+
type: Git
8+
git:
9+
uri: 'https://github.com/openshift/cakephp-ex.git'
10+
strategy:
11+
type: Source
12+
sourceStrategy:
13+
from:
14+
kind: ImageStreamTag
15+
namespace: openshift
16+
name: 'php:7.0'

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,14 @@ metadata:
88
openshift.io/generated-by: OpenShiftWebConsole
99
spec:
1010
failedBuildsHistoryLimit: 2
11-
triggers: {}
12-
runPolicy: Serial
1311
source:
1412
type: Git
1513
git:
1614
uri: 'https://github.com/openshift/non-working-example.git'
17-
ref: master
1815
strategy:
1916
type: Source
2017
sourceStrategy:
2118
from:
2219
kind: ImageStreamTag
2320
namespace: openshift
2421
name: 'php:7.0'
25-
output:
26-
to:
27-
kind: ImageStreamTag
28-
name: 'myphp:latest'
29-
resources: {}
30-
postCommit: {}
31-
nodeSelector: null
32-
status:

‎test/extended/testdata/build-pruning/successful-build-config.yaml

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,14 @@ metadata:
88
openshift.io/generated-by: OpenShiftWebConsole
99
spec:
1010
successfulBuildsHistoryLimit: 2
11-
triggers: {}
12-
runPolicy: Serial
1311
source:
1412
type: Git
1513
git:
1614
uri: 'https://github.com/openshift/cakephp-ex.git'
17-
ref: master
1815
strategy:
1916
type: Source
2017
sourceStrategy:
2118
from:
2219
kind: ImageStreamTag
2320
namespace: openshift
2421
name: 'php:7.0'
25-
output:
26-
to:
27-
kind: ImageStreamTag
28-
name: 'myphp:latest'
29-
resources: {}
30-
postCommit: {}
31-
nodeSelector: null
32-
status:

0 commit comments

Comments
 (0)