Skip to content

Commit

Permalink
DeploymentConfig replicas should be optional, other fields too
Browse files Browse the repository at this point in the history
Make a set of fields truly optional in openapi, and also make replicas a
pointer so we can default it.
  • Loading branch information
smarterclayton committed Oct 25, 2017
1 parent 951a379 commit 9cfe5b4
Show file tree
Hide file tree
Showing 15 changed files with 243 additions and 203 deletions.

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

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

3 changes: 1 addition & 2 deletions api/swagger-spec/oapi-v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -23170,8 +23170,7 @@
"id": "v1.DeploymentConfig",
"description": "Deployment Configs define the template for a pod and manages deploying new images or configuration changes. A single deployment configuration is usually analogous to a single micro-service. Can support many different deployment patterns, including full restart, customizable rolling updates, and fully custom behaviors, as well as pre- and post- deployment hooks. Each individual deployment is represented as a replication controller.\n\nA deployment is \"triggered\" when its configuration is changed or a tag in an Image Stream is changed. Triggers can be disabled to allow manual control over a deployment. The \"strategy\" determines how the deployment is carried out and may be changed at any time. The `latestVersion` field is updated when a new deployment is triggered by any means.",
"required": [
"spec",
"status"
"spec"
],
"properties": {
"kind": {
Expand Down
13 changes: 2 additions & 11 deletions api/swagger-spec/openshift-openapi-spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -87032,8 +87032,7 @@
"com.github.openshift.origin.pkg.apps.apis.apps.v1.DeploymentConfig": {
"description": "Deployment Configs define the template for a pod and manages deploying new images or configuration changes. A single deployment configuration is usually analogous to a single micro-service. Can support many different deployment patterns, including full restart, customizable rolling updates, and fully custom behaviors, as well as pre- and post- deployment hooks. Each individual deployment is represented as a replication controller.\n\nA deployment is \"triggered\" when its configuration is changed or a tag in an Image Stream is changed. Triggers can be disabled to allow manual control over a deployment. The \"strategy\" determines how the deployment is carried out and may be changed at any time. The `latestVersion` field is updated when a new deployment is triggered by any means.",
"required": [
"spec",
"status"
"spec"
],
"properties": {
"apiVersion": {
Expand Down Expand Up @@ -87192,12 +87191,6 @@
},
"com.github.openshift.origin.pkg.apps.apis.apps.v1.DeploymentConfigSpec": {
"description": "DeploymentConfigSpec represents the desired state of the deployment.",
"required": [
"strategy",
"triggers",
"replicas",
"test"
],
"properties": {
"minReadySeconds": {
"description": "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)",
Expand Down Expand Up @@ -90677,9 +90670,7 @@
"com.github.openshift.origin.pkg.image.apis.image.v1.TagReference": {
"description": "TagReference specifies optional annotations for images using this tag and an optional reference to an ImageStreamTag, ImageStreamImage, or DockerImage this tag should track.",
"required": [
"name",
"annotations",
"generation"
"name"
],
"properties": {
"annotations": {
Expand Down
4 changes: 4 additions & 0 deletions pkg/apps/apis/apps/v1/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ func SetDefaults_DeploymentConfigSpec(obj *DeploymentConfigSpec) {
if len(obj.Selector) == 0 && obj.Template != nil {
obj.Selector = obj.Template.Labels
}
if obj.Replicas == nil {
one := int32(1)
obj.Replicas = &one
}

// if you only specify a single container, default the TagImages hook to the container name
if obj.Template != nil && len(obj.Template.Spec.Containers) == 1 {
Expand Down
40 changes: 27 additions & 13 deletions pkg/apps/apis/apps/v1/defaults_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package v1

import (
"fmt"
"reflect"
"testing"

Expand All @@ -14,6 +15,7 @@ import (
)

func TestDefaults(t *testing.T) {
one := int32(1)
defaultIntOrString := intstr.FromString("25%")
differentIntOrString := intstr.FromInt(5)
tests := []struct {
Expand All @@ -24,6 +26,7 @@ func TestDefaults(t *testing.T) {
original: &DeploymentConfig{},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Strategy: DeploymentStrategy{
Type: DeploymentStrategyTypeRolling,
RollingParams: &RollingDeploymentStrategyParams{
Expand Down Expand Up @@ -92,6 +95,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Strategy: DeploymentStrategy{
Type: DeploymentStrategyTypeRecreate,
RecreateParams: &RecreateDeploymentStrategyParams{
Expand Down Expand Up @@ -171,6 +175,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Strategy: DeploymentStrategy{
Type: DeploymentStrategyTypeRolling,
RollingParams: &RollingDeploymentStrategyParams{
Expand Down Expand Up @@ -211,6 +216,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Strategy: DeploymentStrategy{
Type: DeploymentStrategyTypeRolling,
RollingParams: &RollingDeploymentStrategyParams{
Expand Down Expand Up @@ -249,6 +255,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Strategy: DeploymentStrategy{
Type: DeploymentStrategyTypeRolling,
RollingParams: &RollingDeploymentStrategyParams{
Expand Down Expand Up @@ -280,6 +287,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Strategy: DeploymentStrategy{
Type: DeploymentStrategyTypeRolling,
RollingParams: &RollingDeploymentStrategyParams{
Expand Down Expand Up @@ -312,6 +320,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Strategy: DeploymentStrategy{
Type: DeploymentStrategyTypeRecreate,
RecreateParams: &RecreateDeploymentStrategyParams{
Expand Down Expand Up @@ -339,6 +348,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Strategy: DeploymentStrategy{
Type: DeploymentStrategyTypeRecreate,
RecreateParams: &RecreateDeploymentStrategyParams{
Expand Down Expand Up @@ -386,6 +396,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Template: &kapiv1.PodTemplateSpec{
Spec: kapiv1.PodSpec{
Containers: []kapiv1.Container{
Expand Down Expand Up @@ -458,6 +469,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Template: &kapiv1.PodTemplateSpec{
Spec: kapiv1.PodSpec{
Containers: []kapiv1.Container{
Expand Down Expand Up @@ -513,6 +525,7 @@ func TestDefaults(t *testing.T) {
},
expected: &DeploymentConfig{
Spec: DeploymentConfigSpec{
Replicas: &one,
Strategy: DeploymentStrategy{
Type: DeploymentStrategyTypeRolling,
RollingParams: &RollingDeploymentStrategyParams{
Expand Down Expand Up @@ -540,19 +553,20 @@ func TestDefaults(t *testing.T) {
}

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.(*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 !equality.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))
}
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
t.Logf("test %d", i)
original := test.original
expected := test.expected
obj2 := roundTrip(t, runtime.Object(original))
got, ok := obj2.(*DeploymentConfig)
if !ok {
t.Fatalf("unexpected object: %v", got)
}
// TODO(rebase): check that there are no fields which have different semantics for nil and []
if !equality.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
Loading

0 comments on commit 9cfe5b4

Please sign in to comment.