Skip to content

Commit a7a5b81

Browse files
tnozickatnozicka
committed
TMP: Addreses Clayton's comment and adds integration test
1 parent b5dedb9 commit a7a5b81

File tree

3 files changed

+239
-5
lines changed

3 files changed

+239
-5
lines changed

‎pkg/deploy/apis/apps/v1/defaults.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@ import (
66
deployapi "github.com/openshift/origin/pkg/deploy/apis/apps"
77
)
88

9+
// Applies defaults only for API group "apps.openshift.io" and not for the legacy API.
10+
// This function is called from storage layer where differentiation
11+
// between legacy and group API can be made and is not related to other functions here
12+
// which are called fom auto-generated code.
13+
func AppsV1DeploymentConfigLayeredDefaults(dc *deployapi.DeploymentConfig) {
14+
if dc.Spec.RevisionHistoryLimit == nil {
15+
v := deployapi.DefaultRevisionHistoryLimit
16+
dc.Spec.RevisionHistoryLimit = &v
17+
}
18+
}
19+
920
// Keep this in sync with pkg/api/serialization_test.go#defaultHookContainerName
1021
func defaultHookContainerName(hook *LifecycleHook, containerName string) {
1122
if hook == nil {

‎pkg/deploy/registry/deployconfig/strategy.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
kapi "k8s.io/kubernetes/pkg/api"
1717

1818
deployapi "github.com/openshift/origin/pkg/deploy/apis/apps"
19+
deployapiv1 "github.com/openshift/origin/pkg/deploy/apis/apps/v1"
1920
"github.com/openshift/origin/pkg/deploy/apis/apps/validation"
2021
)
2122

@@ -139,11 +140,7 @@ func (s groupStrategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Obje
139140
s.strategy.PrepareForCreate(ctx, obj)
140141

141142
dc := obj.(*deployapi.DeploymentConfig)
142-
143-
if dc.Spec.RevisionHistoryLimit == nil {
144-
v := deployapi.DefaultRevisionHistoryLimit
145-
dc.Spec.RevisionHistoryLimit = &v
146-
}
143+
deployapiv1.AppsV1DeploymentConfigLayeredDefaults(dc)
147144
}
148145

149146
// statusStrategy implements behavior for DeploymentConfig status updates.

‎test/integration/deploy_defaults_test.go

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
package integration
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
"time"
7+
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/apimachinery/pkg/util/diff"
10+
"k8s.io/apimachinery/pkg/util/intstr"
11+
kapi "k8s.io/kubernetes/pkg/api"
12+
13+
"github.com/openshift/origin/pkg/client"
14+
deployapi "github.com/openshift/origin/pkg/deploy/apis/apps"
15+
deployapiv1 "github.com/openshift/origin/pkg/deploy/apis/apps/v1"
16+
"github.com/openshift/origin/pkg/deploy/generated/clientset"
17+
testutil "github.com/openshift/origin/test/util"
18+
testserver "github.com/openshift/origin/test/util/server"
19+
)
20+
21+
var (
22+
nonDefaultRevisionHistoryLimit = deployapi.DefaultRevisionHistoryLimit + 42
23+
)
24+
25+
func minimalDC(name string, generation int64) *deployapi.DeploymentConfig {
26+
return &deployapi.DeploymentConfig{
27+
ObjectMeta: metav1.ObjectMeta{
28+
Name: name,
29+
Generation: generation,
30+
},
31+
Spec: deployapi.DeploymentConfigSpec{
32+
Selector: map[string]string{
33+
"app": name,
34+
},
35+
Template: &kapi.PodTemplateSpec{
36+
ObjectMeta: metav1.ObjectMeta{
37+
Labels: map[string]string{
38+
"app": name,
39+
},
40+
},
41+
Spec: kapi.PodSpec{
42+
Containers: []kapi.Container{
43+
{
44+
Name: "a",
45+
Image: " ",
46+
},
47+
},
48+
},
49+
},
50+
},
51+
}
52+
}
53+
54+
func int64ptr(v int64) *int64 {
55+
return &v
56+
}
57+
58+
func int32ptr(v int32) *int32 {
59+
return &v
60+
}
61+
62+
func setEssentialDefaults(dc *deployapi.DeploymentConfig) *deployapi.DeploymentConfig {
63+
dc.Spec.Strategy.Type = deployapi.DeploymentStrategyTypeRolling
64+
dc.Spec.Strategy.RollingParams = &deployapi.RollingDeploymentStrategyParams{
65+
IntervalSeconds: int64ptr(1),
66+
UpdatePeriodSeconds: int64ptr(1),
67+
TimeoutSeconds: int64ptr(600),
68+
MaxUnavailable: intstr.FromString("25%"),
69+
MaxSurge: intstr.FromString("25%"),
70+
}
71+
dc.Spec.Strategy.ActiveDeadlineSeconds = int64ptr(21600)
72+
dc.Spec.Triggers = []deployapi.DeploymentTriggerPolicy{}
73+
dc.Spec.Template.Spec.Containers[0].TerminationMessagePath = "/dev/termination-log"
74+
dc.Spec.Template.Spec.Containers[0].TerminationMessagePolicy = "File"
75+
dc.Spec.Template.Spec.Containers[0].ImagePullPolicy = "IfNotPresent"
76+
dc.Spec.Template.Spec.RestartPolicy = "Always"
77+
dc.Spec.Template.Spec.TerminationGracePeriodSeconds = int64ptr(30)
78+
dc.Spec.Template.Spec.DNSPolicy = "ClusterFirst"
79+
dc.Spec.Template.Spec.SecurityContext = &kapi.PodSecurityContext{
80+
HostNetwork: false,
81+
HostPID: false,
82+
HostIPC: false,
83+
}
84+
dc.Spec.Template.Spec.SchedulerName = "default-scheduler"
85+
86+
return dc
87+
}
88+
89+
func clearTransient(dc *deployapi.DeploymentConfig) {
90+
dc.ObjectMeta.Namespace = ""
91+
dc.ObjectMeta.SelfLink = ""
92+
dc.ObjectMeta.UID = ""
93+
dc.ObjectMeta.ResourceVersion = ""
94+
dc.ObjectMeta.CreationTimestamp.Time = time.Time{}
95+
}
96+
97+
func TestDeploymentConfigDefaults(t *testing.T) {
98+
masterConfig, clusterAdminKubeConfig, err := testserver.StartTestMaster()
99+
if err != nil {
100+
t.Fatalf("Failed to start master: %v", err)
101+
}
102+
defer testserver.CleanupMasterEtcd(t, masterConfig)
103+
104+
namespace := "default"
105+
106+
adminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
107+
if err != nil {
108+
t.Fatalf("Failed to get cluster admin client config: %v", err)
109+
}
110+
111+
legacyClient, err := client.New(adminClientConfig)
112+
if err != nil {
113+
t.Fatalf("Failed to create legacyClient: %v", err)
114+
}
115+
116+
appsClient, err := clientset.NewForConfig(adminClientConfig)
117+
if err != nil {
118+
t.Fatalf("Failed to create appsClient: %v", err)
119+
}
120+
121+
ttLegacy := []struct {
122+
obj *deployapi.DeploymentConfig
123+
legacy *deployapi.DeploymentConfig
124+
}{
125+
{
126+
obj: func() *deployapi.DeploymentConfig {
127+
dc := minimalDC("test-legacy-01", 0)
128+
dc.Spec.RevisionHistoryLimit = nil
129+
return dc
130+
}(),
131+
legacy: func() *deployapi.DeploymentConfig {
132+
dc := minimalDC("test-legacy-01", 1)
133+
setEssentialDefaults(dc)
134+
// Legacy API shall not default RevisionHistoryLimit to maintain backwards compatibility
135+
dc.Spec.RevisionHistoryLimit = nil
136+
return dc
137+
}(),
138+
},
139+
{
140+
obj: func() *deployapi.DeploymentConfig {
141+
dc := minimalDC("test-legacy-02", 0)
142+
dc.Spec.RevisionHistoryLimit = &nonDefaultRevisionHistoryLimit
143+
return dc
144+
}(),
145+
legacy: func() *deployapi.DeploymentConfig {
146+
dc := minimalDC("test-legacy-02", 1)
147+
setEssentialDefaults(dc)
148+
dc.Spec.RevisionHistoryLimit = &nonDefaultRevisionHistoryLimit
149+
return dc
150+
}(),
151+
},
152+
}
153+
t.Run("Legacy API", func(t *testing.T) {
154+
for _, tc := range ttLegacy {
155+
t.Run("", func(t *testing.T) {
156+
legacyDC, err := legacyClient.DeploymentConfigs(namespace).Create(tc.obj)
157+
if err != nil {
158+
t.Fatalf("Failed to create DC: %v", err)
159+
}
160+
161+
clearTransient(legacyDC)
162+
if !reflect.DeepEqual(legacyDC, tc.legacy) {
163+
t.Errorf("Legacy DC differs from expected output: %s", diff.ObjectReflectDiff(legacyDC, tc.legacy))
164+
}
165+
})
166+
}
167+
})
168+
169+
ttApps := []struct {
170+
obj *deployapi.DeploymentConfig
171+
apps *deployapi.DeploymentConfig
172+
}{
173+
{
174+
obj: func() *deployapi.DeploymentConfig {
175+
dc := minimalDC("test-apps-01", 0)
176+
dc.Spec.RevisionHistoryLimit = nil
177+
return dc
178+
}(),
179+
apps: func() *deployapi.DeploymentConfig {
180+
dc := minimalDC("test-apps-01", 1)
181+
setEssentialDefaults(dc)
182+
// Group API should default RevisionHistoryLimit
183+
dc.Spec.RevisionHistoryLimit = int32ptr(10)
184+
return dc
185+
}(),
186+
},
187+
{
188+
obj: func() *deployapi.DeploymentConfig {
189+
dc := minimalDC("test-apps-02", 0)
190+
dc.Spec.RevisionHistoryLimit = &nonDefaultRevisionHistoryLimit
191+
return dc
192+
}(),
193+
apps: func() *deployapi.DeploymentConfig {
194+
dc := minimalDC("test-apps-02", 1)
195+
setEssentialDefaults(dc)
196+
dc.Spec.RevisionHistoryLimit = &nonDefaultRevisionHistoryLimit
197+
return dc
198+
}(),
199+
},
200+
}
201+
t.Run("apps.openshift.io", func(t *testing.T) {
202+
for _, tc := range ttApps {
203+
t.Run("", func(t *testing.T) {
204+
var objV1 deployapiv1.DeploymentConfig
205+
err := kapi.Scheme.Convert(tc.obj, &objV1, nil)
206+
if err != nil {
207+
t.Fatalf("Failed to convert internal DC to v1: %v", err)
208+
}
209+
appsDCV1, err := appsClient.AppsV1().DeploymentConfigs(namespace).Create(&objV1)
210+
if err != nil {
211+
t.Fatalf("Failed to create DC: %v", err)
212+
}
213+
214+
var appsDC deployapi.DeploymentConfig
215+
err = kapi.Scheme.Convert(appsDCV1, &appsDC, nil)
216+
if err != nil {
217+
t.Fatalf("Failed to convert v1 to internal DC: %v", err)
218+
}
219+
clearTransient(&appsDC)
220+
if !reflect.DeepEqual(&appsDC, tc.apps) {
221+
t.Errorf("Apps DC differs from expected output: %s", diff.ObjectReflectDiff(&appsDC, tc.apps))
222+
}
223+
})
224+
}
225+
})
226+
}

0 commit comments

Comments
 (0)