Browse code

Always do a canary check during deployment

Always perform canary checking during deployments, regardless of the
desired replica count. Increase the default canary timeout to 10 seconds
to reduce the possibility of false negative startup canary checks.

Dan Mace authored on 2015/06/19 01:01:12
Showing 7 changed files
... ...
@@ -124,6 +124,15 @@ type RollingDeploymentStrategyParams struct {
124 124
 	Post *LifecycleHook `json:"post,omitempty" description:"a hook executed after the strategy finishes the deployment"`
125 125
 }
126 126
 
127
+const (
128
+	// DefaultRollingTimeoutSeconds is the default TimeoutSeconds for RollingDeploymentStrategyParams.
129
+	DefaultRollingTimeoutSeconds int64 = 10 * 60
130
+	// DefaultRollingIntervalSeconds is the default IntervalSeconds for RollingDeploymentStrategyParams.
131
+	DefaultRollingIntervalSeconds int64 = 1
132
+	// DefaultRollingUpdatePeriodSeconds is the default PeriodSeconds for RollingDeploymentStrategyParams.
133
+	DefaultRollingUpdatePeriodSeconds int64 = 1
134
+)
135
+
127 136
 // These constants represent keys used for correlating objects related to deployments.
128 137
 const (
129 138
 	// DeploymentConfigAnnotation is an annotation name used to correlate a deployment with the
... ...
@@ -2,12 +2,13 @@ package v1
2 2
 
3 3
 import (
4 4
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
5
+
6
+	deployapi "github.com/openshift/origin/pkg/deploy/api"
5 7
 )
6 8
 
7 9
 func init() {
8
-	mkintp := func(i int) *int64 {
9
-		v := int64(i)
10
-		return &v
10
+	mkintp := func(i int64) *int64 {
11
+		return &i
11 12
 	}
12 13
 
13 14
 	err := api.Scheme.AddDefaultingFuncs(
... ...
@@ -18,23 +19,23 @@ func init() {
18 18
 
19 19
 			if obj.Type == DeploymentStrategyTypeRolling && obj.RollingParams == nil {
20 20
 				obj.RollingParams = &RollingDeploymentStrategyParams{
21
-					IntervalSeconds:     mkintp(1),
22
-					UpdatePeriodSeconds: mkintp(1),
23
-					TimeoutSeconds:      mkintp(120),
21
+					IntervalSeconds:     mkintp(deployapi.DefaultRollingIntervalSeconds),
22
+					UpdatePeriodSeconds: mkintp(deployapi.DefaultRollingUpdatePeriodSeconds),
23
+					TimeoutSeconds:      mkintp(deployapi.DefaultRollingTimeoutSeconds),
24 24
 				}
25 25
 			}
26 26
 		},
27 27
 		func(obj *RollingDeploymentStrategyParams) {
28 28
 			if obj.IntervalSeconds == nil {
29
-				obj.IntervalSeconds = mkintp(1)
29
+				obj.IntervalSeconds = mkintp(deployapi.DefaultRollingIntervalSeconds)
30 30
 			}
31 31
 
32 32
 			if obj.UpdatePeriodSeconds == nil {
33
-				obj.UpdatePeriodSeconds = mkintp(1)
33
+				obj.UpdatePeriodSeconds = mkintp(deployapi.DefaultRollingUpdatePeriodSeconds)
34 34
 			}
35 35
 
36 36
 			if obj.TimeoutSeconds == nil {
37
-				obj.TimeoutSeconds = mkintp(120)
37
+				obj.TimeoutSeconds = mkintp(deployapi.DefaultRollingTimeoutSeconds)
38 38
 			}
39 39
 		},
40 40
 	)
... ...
@@ -7,6 +7,7 @@ import (
7 7
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
8 8
 
9 9
 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
10
+	deployapi "github.com/openshift/origin/pkg/deploy/api"
10 11
 	current "github.com/openshift/origin/pkg/deploy/api/v1"
11 12
 )
12 13
 
... ...
@@ -38,13 +39,13 @@ func TestDefaults_rollingParams(t *testing.T) {
38 38
 	if e, a := current.DeploymentStrategyTypeRolling, strat.Type; e != a {
39 39
 		t.Errorf("expected strategy type %s, got %s", e, a)
40 40
 	}
41
-	if e, a := int64(1), *strat.RollingParams.UpdatePeriodSeconds; e != a {
41
+	if e, a := deployapi.DefaultRollingUpdatePeriodSeconds, *strat.RollingParams.UpdatePeriodSeconds; e != a {
42 42
 		t.Errorf("expected UpdatePeriodSeconds %d, got %d", e, a)
43 43
 	}
44
-	if e, a := int64(1), *strat.RollingParams.IntervalSeconds; e != a {
44
+	if e, a := deployapi.DefaultRollingIntervalSeconds, *strat.RollingParams.IntervalSeconds; e != a {
45 45
 		t.Errorf("expected IntervalSeconds %d, got %d", e, a)
46 46
 	}
47
-	if e, a := int64(120), *strat.RollingParams.TimeoutSeconds; e != a {
47
+	if e, a := deployapi.DefaultRollingTimeoutSeconds, *strat.RollingParams.TimeoutSeconds; e != a {
48 48
 		t.Errorf("expected UpdatePeriodSeconds %d, got %d", e, a)
49 49
 	}
50 50
 }
... ...
@@ -2,12 +2,13 @@ package v1beta3
2 2
 
3 3
 import (
4 4
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
5
+
6
+	deployapi "github.com/openshift/origin/pkg/deploy/api"
5 7
 )
6 8
 
7 9
 func init() {
8
-	mkintp := func(i int) *int64 {
9
-		v := int64(i)
10
-		return &v
10
+	mkintp := func(i int64) *int64 {
11
+		return &i
11 12
 	}
12 13
 
13 14
 	err := api.Scheme.AddDefaultingFuncs(
... ...
@@ -18,23 +19,23 @@ func init() {
18 18
 
19 19
 			if obj.Type == DeploymentStrategyTypeRolling && obj.RollingParams == nil {
20 20
 				obj.RollingParams = &RollingDeploymentStrategyParams{
21
-					IntervalSeconds:     mkintp(1),
22
-					UpdatePeriodSeconds: mkintp(1),
23
-					TimeoutSeconds:      mkintp(120),
21
+					IntervalSeconds:     mkintp(deployapi.DefaultRollingIntervalSeconds),
22
+					UpdatePeriodSeconds: mkintp(deployapi.DefaultRollingUpdatePeriodSeconds),
23
+					TimeoutSeconds:      mkintp(deployapi.DefaultRollingTimeoutSeconds),
24 24
 				}
25 25
 			}
26 26
 		},
27 27
 		func(obj *RollingDeploymentStrategyParams) {
28 28
 			if obj.IntervalSeconds == nil {
29
-				obj.IntervalSeconds = mkintp(1)
29
+				obj.IntervalSeconds = mkintp(deployapi.DefaultRollingIntervalSeconds)
30 30
 			}
31 31
 
32 32
 			if obj.UpdatePeriodSeconds == nil {
33
-				obj.UpdatePeriodSeconds = mkintp(1)
33
+				obj.UpdatePeriodSeconds = mkintp(deployapi.DefaultRollingUpdatePeriodSeconds)
34 34
 			}
35 35
 
36 36
 			if obj.TimeoutSeconds == nil {
37
-				obj.TimeoutSeconds = mkintp(120)
37
+				obj.TimeoutSeconds = mkintp(deployapi.DefaultRollingTimeoutSeconds)
38 38
 			}
39 39
 		},
40 40
 	)
... ...
@@ -7,6 +7,7 @@ import (
7 7
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
8 8
 
9 9
 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
10
+	deployapi "github.com/openshift/origin/pkg/deploy/api"
10 11
 	current "github.com/openshift/origin/pkg/deploy/api/v1beta3"
11 12
 )
12 13
 
... ...
@@ -38,13 +39,13 @@ func TestDefaults_rollingParams(t *testing.T) {
38 38
 	if e, a := current.DeploymentStrategyTypeRolling, strat.Type; e != a {
39 39
 		t.Errorf("expected strategy type %s, got %s", e, a)
40 40
 	}
41
-	if e, a := int64(1), *strat.RollingParams.UpdatePeriodSeconds; e != a {
41
+	if e, a := deployapi.DefaultRollingUpdatePeriodSeconds, *strat.RollingParams.UpdatePeriodSeconds; e != a {
42 42
 		t.Errorf("expected UpdatePeriodSeconds %d, got %d", e, a)
43 43
 	}
44
-	if e, a := int64(1), *strat.RollingParams.IntervalSeconds; e != a {
44
+	if e, a := deployapi.DefaultRollingIntervalSeconds, *strat.RollingParams.IntervalSeconds; e != a {
45 45
 		t.Errorf("expected IntervalSeconds %d, got %d", e, a)
46 46
 	}
47
-	if e, a := int64(120), *strat.RollingParams.TimeoutSeconds; e != a {
47
+	if e, a := deployapi.DefaultRollingTimeoutSeconds, *strat.RollingParams.TimeoutSeconds; e != a {
48 48
 		t.Errorf("expected UpdatePeriodSeconds %d, got %d", e, a)
49 49
 	}
50 50
 }
... ...
@@ -104,29 +104,32 @@ func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationCo
104 104
 		}
105 105
 	}
106 106
 
107
-	// If an UpdateAcceptor is provided and we're trying to scale up to more
108
-	// than one replica, scale up to 1 and validate the replica, aborting if the
109
-	// replica isn't acceptable.
110
-	if updateAcceptor != nil && desiredReplicas > 1 {
111
-		glog.Infof("Scaling %s to 1 before validating first replica", deployutil.LabelForDeployment(to))
112
-		updatedTo, err := s.scaleAndWait(to, 1, retryParams, waitParams)
113
-		if err != nil {
114
-			return fmt.Errorf("couldn't scale %s to 1: %v", deployutil.LabelForDeployment(to), err)
107
+	// Scale up the to deployment.
108
+	if desiredReplicas > 0 {
109
+		// If an UpdateAcceptor is provided, scale up to 1 and validate the replica,
110
+		// aborting if the replica isn't acceptable.
111
+		if updateAcceptor != nil {
112
+			glog.Infof("Scaling %s to 1 before validating first replica", deployutil.LabelForDeployment(to))
113
+			updatedTo, err := s.scaleAndWait(to, 1, retryParams, waitParams)
114
+			if err != nil {
115
+				return fmt.Errorf("couldn't scale %s to 1: %v", deployutil.LabelForDeployment(to), err)
116
+			}
117
+			glog.Infof("Validating first replica of %s", deployutil.LabelForDeployment(to))
118
+			if err := updateAcceptor.Accept(updatedTo); err != nil {
119
+				return fmt.Errorf("first replica rejected for %s: %v", to.Name, err)
120
+			}
121
+			to = updatedTo
115 122
 		}
116
-		glog.Infof("Validating first replica of %s", deployutil.LabelForDeployment(to))
117
-		if err := updateAcceptor.Accept(updatedTo); err != nil {
118
-			return fmt.Errorf("first replica rejected for %s: %v", to.Name, err)
123
+		// Complete the scale up.
124
+		if to.Spec.Replicas != desiredReplicas {
125
+			glog.Infof("Scaling %s to %d", deployutil.LabelForDeployment(to), desiredReplicas)
126
+			updatedTo, err := s.scaleAndWait(to, desiredReplicas, retryParams, waitParams)
127
+			if err != nil {
128
+				return fmt.Errorf("couldn't scale %s to %d: %v", deployutil.LabelForDeployment(to), desiredReplicas, err)
129
+			}
130
+			to = updatedTo
119 131
 		}
120
-		to = updatedTo
121
-	}
122
-
123
-	// Complete the scale up.
124
-	glog.Infof("Scaling %s to %d", deployutil.LabelForDeployment(to), desiredReplicas)
125
-	updatedTo, err := s.scaleAndWait(to, desiredReplicas, retryParams, waitParams)
126
-	if err != nil {
127
-		return fmt.Errorf("couldn't scale %s to %d: %v", deployutil.LabelForDeployment(to), desiredReplicas, err)
128 132
 	}
129
-	to = updatedTo
130 133
 
131 134
 	// Execute any post-hook. Errors are logged and ignored.
132 135
 	if params != nil && params.Post != nil {
... ...
@@ -182,8 +182,10 @@ func TestRecreate_acceptorSuccess(t *testing.T) {
182 182
 		scaler: scaler,
183 183
 	}
184 184
 
185
+	acceptorCalled := false
185 186
 	acceptor := &testAcceptor{
186 187
 		acceptFn: func(deployment *kapi.ReplicationController) error {
188
+			acceptorCalled = true
187 189
 			return nil
188 190
 		},
189 191
 	}
... ...
@@ -194,6 +196,10 @@ func TestRecreate_acceptorSuccess(t *testing.T) {
194 194
 		t.Fatalf("unexpected deploy error: %#v", err)
195 195
 	}
196 196
 
197
+	if !acceptorCalled {
198
+		t.Fatalf("expected acceptor to be called")
199
+	}
200
+
197 201
 	if e, a := 2, len(scaler.Events); e != a {
198 202
 		t.Fatalf("expected %s scale calls, got %d", e, a)
199 203
 	}