Skip to content

Commit

Permalink
Set default for DeploymentConfigSpec.RevisionHistoryLimit in apps/v1 …
Browse files Browse the repository at this point in the history 
…to 10
  • Loading branch information
@tnozicka
tnozicka committed Aug 4, 2017
1 parent dc4d146 commit 5bd21ec
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 32 deletions.
2 changes: 2 additions & 0 deletions pkg/deploy/apis/apps/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const (
// This is set as the default value for ActiveDeadlineSeconds for the deployer pod.
// Currently set to 6 hours.
MaxDeploymentDurationSeconds int64 = 21600
// DefaultRevisionHistoryLimit is the number of old ReplicationControllers to retain to allow for rollbacks.
DefaultRevisionHistoryLimit int32 = 10
)

// These constants represent keys used for correlating objects related to deployments.
Expand Down
26 changes: 17 additions & 9 deletions pkg/deploy/apis/apps/v1/defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ func SetDefaults_DeploymentConfigSpec(obj *DeploymentConfigSpec) {
defaultHookContainerName(p.Post, containerName)
}
}

if obj.RevisionHistoryLimit == nil {
obj.RevisionHistoryLimit = newInt32(deployapi.DefaultRevisionHistoryLimit)
}
}

func SetDefaults_DeploymentStrategy(obj *DeploymentStrategy) {
Expand All @@ -55,37 +59,37 @@ func SetDefaults_DeploymentStrategy(obj *DeploymentStrategy) {

if obj.Type == DeploymentStrategyTypeRolling && obj.RollingParams == nil {
obj.RollingParams = &RollingDeploymentStrategyParams{
IntervalSeconds: mkintp(deployapi.DefaultRollingIntervalSeconds),
UpdatePeriodSeconds: mkintp(deployapi.DefaultRollingUpdatePeriodSeconds),
TimeoutSeconds: mkintp(deployapi.DefaultRollingTimeoutSeconds),
IntervalSeconds: newInt64(deployapi.DefaultRollingIntervalSeconds),
UpdatePeriodSeconds: newInt64(deployapi.DefaultRollingUpdatePeriodSeconds),
TimeoutSeconds: newInt64(deployapi.DefaultRollingTimeoutSeconds),
}
}
if obj.Type == DeploymentStrategyTypeRecreate && obj.RecreateParams == nil {
obj.RecreateParams = &RecreateDeploymentStrategyParams{}
}

if obj.ActiveDeadlineSeconds == nil {
obj.ActiveDeadlineSeconds = mkintp(deployapi.MaxDeploymentDurationSeconds)
obj.ActiveDeadlineSeconds = newInt64(deployapi.MaxDeploymentDurationSeconds)
}
}

func SetDefaults_RecreateDeploymentStrategyParams(obj *RecreateDeploymentStrategyParams) {
if obj.TimeoutSeconds == nil {
obj.TimeoutSeconds = mkintp(deployapi.DefaultRecreateTimeoutSeconds)
obj.TimeoutSeconds = newInt64(deployapi.DefaultRecreateTimeoutSeconds)
}
}

func SetDefaults_RollingDeploymentStrategyParams(obj *RollingDeploymentStrategyParams) {
if obj.IntervalSeconds == nil {
obj.IntervalSeconds = mkintp(deployapi.DefaultRollingIntervalSeconds)
obj.IntervalSeconds = newInt64(deployapi.DefaultRollingIntervalSeconds)
}

if obj.UpdatePeriodSeconds == nil {
obj.UpdatePeriodSeconds = mkintp(deployapi.DefaultRollingUpdatePeriodSeconds)
obj.UpdatePeriodSeconds = newInt64(deployapi.DefaultRollingUpdatePeriodSeconds)
}

if obj.TimeoutSeconds == nil {
obj.TimeoutSeconds = mkintp(deployapi.DefaultRollingTimeoutSeconds)
obj.TimeoutSeconds = newInt64(deployapi.DefaultRollingTimeoutSeconds)
}

if obj.MaxUnavailable == nil && obj.MaxSurge == nil {
Expand Down Expand Up @@ -122,6 +126,10 @@ func SetDefaults_DeploymentConfig(obj *DeploymentConfig) {
}
}

func mkintp(i int64) *int64 {
func newInt64(i int64) *int64 {
return &i
}

func newInt32(val int32) *int32 {
return &val
}
53 changes: 32 additions & 21 deletions pkg/deploy/apis/apps/v1/defaults_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,10 @@ import (
deployv1 "github.com/openshift/origin/pkg/deploy/apis/apps/v1"
)

func mkintp(i int64) *int64 {
return &i
}

func TestDefaults(t *testing.T) {
defaultIntOrString := intstr.FromString("25%")
differentIntOrString := intstr.FromInt(5)
nonDefaultRevisionHistoryLimit := newInt32(deployapi.DefaultRevisionHistoryLimit + 42)
tests := []struct {
original *deployv1.DeploymentConfig
expected *deployv1.DeploymentConfig
Expand All @@ -49,6 +46,7 @@ func TestDefaults(t *testing.T) {
Type: deployv1.DeploymentTriggerOnConfigChange,
},
},
RevisionHistoryLimit: newInt32(deployapi.DefaultRevisionHistoryLimit),
},
},
},
Expand Down Expand Up @@ -139,7 +137,7 @@ func TestDefaults(t *testing.T) {
Spec: kapiv1.PodSpec{
SecurityContext: &kapiv1.PodSecurityContext{},
RestartPolicy: kapiv1.RestartPolicyAlways,
TerminationGracePeriodSeconds: mkintp(30),
TerminationGracePeriodSeconds: newInt64(30),
DNSPolicy: kapiv1.DNSClusterFirst,
Containers: []kapiv1.Container{
{
Expand All @@ -155,6 +153,7 @@ func TestDefaults(t *testing.T) {
SchedulerName: kapiv1.DefaultSchedulerName,
},
},
RevisionHistoryLimit: newInt32(deployapi.DefaultRevisionHistoryLimit),
},
},
},
Expand All @@ -176,6 +175,7 @@ func TestDefaults(t *testing.T) {
Type: deployv1.DeploymentTriggerOnImageChange,
},
},
RevisionHistoryLimit: nonDefaultRevisionHistoryLimit,
},
},
expected: &deployv1.DeploymentConfig{
Expand All @@ -196,6 +196,7 @@ func TestDefaults(t *testing.T) {
Type: deployv1.DeploymentTriggerOnImageChange,
},
},
RevisionHistoryLimit: nonDefaultRevisionHistoryLimit,
},
},
},
Expand All @@ -216,6 +217,7 @@ func TestDefaults(t *testing.T) {
Type: deployv1.DeploymentTriggerOnImageChange,
},
},
RevisionHistoryLimit: nonDefaultRevisionHistoryLimit,
},
},
expected: &deployv1.DeploymentConfig{
Expand All @@ -236,6 +238,7 @@ func TestDefaults(t *testing.T) {
Type: deployv1.DeploymentTriggerOnImageChange,
},
},
RevisionHistoryLimit: nonDefaultRevisionHistoryLimit,
},
},
},
Expand Down Expand Up @@ -272,6 +275,7 @@ func TestDefaults(t *testing.T) {
Triggers: []deployv1.DeploymentTriggerPolicy{
{},
},
RevisionHistoryLimit: newInt32(deployapi.DefaultRevisionHistoryLimit),
},
},
},
Expand Down Expand Up @@ -303,6 +307,7 @@ func TestDefaults(t *testing.T) {
Triggers: []deployv1.DeploymentTriggerPolicy{
{},
},
RevisionHistoryLimit: newInt32(deployapi.DefaultRevisionHistoryLimit),
},
},
},
Expand Down Expand Up @@ -331,6 +336,7 @@ func TestDefaults(t *testing.T) {
Triggers: []deployv1.DeploymentTriggerPolicy{
{},
},
RevisionHistoryLimit: newInt32(deployapi.DefaultRevisionHistoryLimit),
},
},
},
Expand Down Expand Up @@ -358,6 +364,7 @@ func TestDefaults(t *testing.T) {
Triggers: []deployv1.DeploymentTriggerPolicy{
{},
},
RevisionHistoryLimit: newInt32(deployapi.DefaultRevisionHistoryLimit),
},
},
},
Expand Down Expand Up @@ -406,7 +413,7 @@ func TestDefaults(t *testing.T) {
},
},
RestartPolicy: kapiv1.RestartPolicyAlways,
TerminationGracePeriodSeconds: mkintp(30),
TerminationGracePeriodSeconds: newInt64(30),
SecurityContext: &kapiv1.PodSecurityContext{},
DNSPolicy: kapiv1.DNSClusterFirst,
SchedulerName: kapiv1.DefaultSchedulerName,
Expand Down Expand Up @@ -434,6 +441,7 @@ func TestDefaults(t *testing.T) {
Triggers: []deployv1.DeploymentTriggerPolicy{
{},
},
RevisionHistoryLimit: newInt32(deployapi.DefaultRevisionHistoryLimit),
},
},
},
Expand Down Expand Up @@ -478,7 +486,7 @@ func TestDefaults(t *testing.T) {
},
},
RestartPolicy: kapiv1.RestartPolicyAlways,
TerminationGracePeriodSeconds: mkintp(30),
TerminationGracePeriodSeconds: newInt64(30),
SecurityContext: &kapiv1.PodSecurityContext{},
DNSPolicy: kapiv1.DNSClusterFirst,
SchedulerName: kapiv1.DefaultSchedulerName,
Expand Down Expand Up @@ -506,6 +514,7 @@ func TestDefaults(t *testing.T) {
Triggers: []deployv1.DeploymentTriggerPolicy{
{},
},
RevisionHistoryLimit: newInt32(deployapi.DefaultRevisionHistoryLimit),
},
},
},
Expand Down Expand Up @@ -543,25 +552,27 @@ func TestDefaults(t *testing.T) {
},
},
},
RevisionHistoryLimit: newInt32(deployapi.DefaultRevisionHistoryLimit),
},
},
},
}

for i, test := range tests {
t.Logf("test %d", i)
original := test.original
expected := test.expected
obj2 := roundTrip(t, runtime.Object(original))
got, ok := obj2.(*deployv1.DeploymentConfig)
if !ok {
t.Errorf("unexpected object: %v", got)
t.FailNow()
}
// TODO(rebase): check that there are no fields which have different semantics for nil and []
if !kapihelper.Semantic.DeepEqual(got.Spec, expected.Spec) {
t.Errorf("got different than expected:\nA:\t%#v\nB:\t%#v\n\nDiff:\n%s\n\n%s", got, expected, diff.ObjectDiff(expected, got), diff.ObjectGoPrintSideBySide(expected, got))
}
for _, test := range tests {
t.Run("", func(t *testing.T) {
original := test.original
expected := test.expected
obj2 := roundTrip(t, runtime.Object(original))
got, ok := obj2.(*deployv1.DeploymentConfig)
if !ok {
t.Errorf("unexpected object: %v", got)
t.FailNow()
}
// TODO(rebase): check that there are no fields which have different semantics for nil and []
if !kapihelper.Semantic.DeepEqual(got.Spec, expected.Spec) {
t.Errorf("got different than expected:\nA:\t%#v\nB:\t%#v\n\nDiff:\n%s\n\n%s", got, expected, diff.ObjectDiff(expected, got), diff.ObjectGoPrintSideBySide(expected, got))
}
})
}
}

Expand Down
1 change: 1 addition & 0 deletions pkg/deploy/apis/apps/v1/generated.proto
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/deploy/apis/apps/v1/swagger_doc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ var map_DeploymentConfigSpec = map[string]string{
"minReadySeconds": "MinReadySeconds is the minimum number of seconds for which a newly created pod should be ready without any of its container crashing, for it to be considered available. Defaults to 0 (pod will be considered available as soon as it is ready)",
"triggers": "Triggers determine how updates to a DeploymentConfig result in new deployments. If no triggers are defined, a new deployment can only occur as a result of an explicit client update to the DeploymentConfig with a new LatestVersion. If null, defaults to having a config change trigger.",
"replicas": "Replicas is the number of desired replicas.",
"revisionHistoryLimit": "RevisionHistoryLimit is the number of old ReplicationControllers to retain to allow for rollbacks. This field is a pointer to allow for differentiation between an explicit zero and not specified.",
"revisionHistoryLimit": "RevisionHistoryLimit is the number of old ReplicationControllers to retain to allow for rollbacks. This field is a pointer to allow for differentiation between an explicit zero and not specified. Defaults to 10.",
"test": "Test ensures that this deployment config will have zero replicas except while a deployment is running. This allows the deployment config to be used as a continuous deployment test - triggering on images, running the deployment, and then succeeding or failing. Post strategy hooks and After actions can be used to integrate successful deployment with an action.",
"paused": "Paused indicates that the deployment config is paused resulting in no new deployments on template changes or changes in the template caused by other triggers.",
"selector": "Selector is a label query over pods that should match the Replicas count.",
Expand Down
1 change: 1 addition & 0 deletions pkg/deploy/apis/apps/v1/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type DeploymentConfigSpec struct {

// RevisionHistoryLimit is the number of old ReplicationControllers to retain to allow for rollbacks.
// This field is a pointer to allow for differentiation between an explicit zero and not specified.
// Defaults to 10.
RevisionHistoryLimit *int32 `json:"revisionHistoryLimit,omitempty" protobuf:"varint,4,opt,name=revisionHistoryLimit"`

// Test ensures that this deployment config will have zero replicas except while a deployment is running. This allows the
Expand Down
2 changes: 1 addition & 1 deletion pkg/openapi/zz_generated.openapi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4669,7 +4669,7 @@ func GetOpenAPIDefinitions(ref openapi.ReferenceCallback) map[string]openapi.Ope
},
"revisionHistoryLimit": {
SchemaProps: spec.SchemaProps{
Description: "RevisionHistoryLimit is the number of old ReplicationControllers to retain to allow for rollbacks. This field is a pointer to allow for differentiation between an explicit zero and not specified.",
Description: "RevisionHistoryLimit is the number of old ReplicationControllers to retain to allow for rollbacks. This field is a pointer to allow for differentiation between an explicit zero and not specified. Defaults to 10.",
Type: []string{"integer"},
Format: "int32",
},
Expand Down

0 comments on commit 5bd21ec

Please sign in to comment.