Skip to content

Commit 36a54fb

Browse files
stefanprodanstefanprodan
authored
Merge pull request #31 from stefanprodan/reset
Restart analysis if revision changes during validation
2 parents 1b3c3b2 + 60f6b05 commit 36a54fb

File tree

4 files changed

+274
-92
lines changed

4 files changed

+274
-92
lines changed

‎pkg/controller/deployer.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,11 @@ func (c *CanaryDeployer) IsPrimaryReady(cd *flaggerv1.Canary) (bool, error) {
8282

8383
retriable, err := c.isDeploymentReady(primary, cd.GetProgressDeadlineSeconds())
8484
if err != nil {
85-
if retriable {
86-
return retriable, fmt.Errorf("Halt %s.%s advancement %s", cd.Name, cd.Namespace, err.Error())
87-
} else {
88-
return retriable, err
89-
}
85+
return retriable, fmt.Errorf("Halt advancement %s.%s %s", primaryName, cd.Namespace, err.Error())
9086
}
9187

9288
if primary.Spec.Replicas == int32p(0) {
93-
return true, fmt.Errorf("halt %s.%s advancement primary deployment is scaled to zero",
89+
return true, fmt.Errorf("Halt %s.%s advancement primary deployment is scaled to zero",
9490
cd.Name, cd.Namespace)
9591
}
9692
return true, nil
@@ -112,7 +108,7 @@ func (c *CanaryDeployer) IsCanaryReady(cd *flaggerv1.Canary) (bool, error) {
112108
retriable, err := c.isDeploymentReady(canary, cd.GetProgressDeadlineSeconds())
113109
if err != nil {
114110
if retriable {
115-
return retriable, fmt.Errorf("Halt %s.%s advancement %s", cd.Name, cd.Namespace, err.Error())
111+
return retriable, fmt.Errorf("Halt advancement %s.%s %s", targetName, cd.Namespace, err.Error())
116112
} else {
117113
return retriable, fmt.Errorf("deployment does not have minimum availability for more than %vs",
118114
cd.GetProgressDeadlineSeconds())

‎pkg/controller/job.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import "time"
66
type CanaryJob struct {
77
Name string
88
Namespace string
9-
function func(name string, namespace string)
9+
SkipTests bool
10+
function func(name string, namespace string, skipTests bool)
1011
done chan bool
1112
ticker *time.Ticker
1213
}
@@ -15,11 +16,11 @@ type CanaryJob struct {
1516
func (j CanaryJob) Start() {
1617
go func() {
1718
// run the infra bootstrap on job creation
18-
j.function(j.Name, j.Namespace)
19+
j.function(j.Name, j.Namespace, j.SkipTests)
1920
for {
2021
select {
2122
case <-j.ticker.C:
22-
j.function(j.Name, j.Namespace)
23+
j.function(j.Name, j.Namespace, j.SkipTests)
2324
case <-j.done:
2425
return
2526
}

‎pkg/controller/scheduler.go‎

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ func (c *Controller) scheduleCanaries() {
5858
for canaryName, targetName := range current {
5959
for name, target := range current {
6060
if name != canaryName && target == targetName {
61-
c.logger.Errorf("Bad things will happen! Found more than one canary with the same target %s",
62-
targetName)
61+
c.logger.With("canary", canaryName).Errorf("Bad things will happen! Found more than one canary with the same target %s", targetName)
6362
}
6463
}
6564
}
@@ -70,7 +69,7 @@ func (c *Controller) scheduleCanaries() {
7069
}
7170
}
7271

73-
func (c *Controller) advanceCanary(name string, namespace string) {
72+
func (c *Controller) advanceCanary(name string, namespace string, skipLivenessChecks bool) {
7473
begin := time.Now()
7574
// check if the canary exists
7675
cd, err := c.flaggerClient.FlaggerV1alpha3().Canaries(namespace).Get(name, v1.GetOptions{})
@@ -105,9 +104,11 @@ func (c *Controller) advanceCanary(name string, namespace string) {
105104
}
106105

107106
// check primary deployment status
108-
if _, err := c.deployer.IsPrimaryReady(cd); err != nil {
109-
c.recordEventWarningf(cd, "%v", err)
110-
return
107+
if !skipLivenessChecks {
108+
if _, err := c.deployer.IsPrimaryReady(cd); err != nil {
109+
c.recordEventWarningf(cd, "%v", err)
110+
return
111+
}
111112
}
112113

113114
// check if virtual service exists
@@ -121,7 +122,33 @@ func (c *Controller) advanceCanary(name string, namespace string) {
121122
c.recorder.SetWeight(cd, primaryRoute.Weight, canaryRoute.Weight)
122123

123124
// check if canary analysis should start (canary revision has changes) or continue
124-
if ok := c.checkCanaryStatus(cd, c.deployer); !ok {
125+
if ok := c.checkCanaryStatus(cd); !ok {
126+
return
127+
}
128+
129+
// check if canary revision changed during analysis
130+
if restart := c.hasCanaryRevisionChanged(cd); restart {
131+
c.recordEventInfof(cd, "New revision detected! Restarting analysis for %s.%s",
132+
cd.Spec.TargetRef.Name, cd.Namespace)
133+
134+
// route all traffic back to primary
135+
primaryRoute.Weight = 100
136+
canaryRoute.Weight = 0
137+
if err := c.router.SetRoutes(cd, primaryRoute, canaryRoute); err != nil {
138+
c.recordEventWarningf(cd, "%v", err)
139+
return
140+
}
141+
142+
// reset status
143+
status := flaggerv1.CanaryStatus{
144+
Phase: flaggerv1.CanaryProgressing,
145+
CanaryWeight: 0,
146+
FailedChecks: 0,
147+
}
148+
if err := c.deployer.SyncStatus(cd, status); err != nil {
149+
c.recordEventWarningf(cd, "%v", err)
150+
return
151+
}
125152
return
126153
}
127154

@@ -130,10 +157,13 @@ func (c *Controller) advanceCanary(name string, namespace string) {
130157
}()
131158

132159
// check canary deployment status
133-
retriable, err := c.deployer.IsCanaryReady(cd)
134-
if err != nil && retriable {
135-
c.recordEventWarningf(cd, "%v", err)
136-
return
160+
var retriable = true
161+
if !skipLivenessChecks {
162+
retriable, err = c.deployer.IsCanaryReady(cd)
163+
if err != nil && retriable {
164+
c.recordEventWarningf(cd, "%v", err)
165+
return
166+
}
137167
}
138168

139169
// check if the number of failed checks reached the threshold
@@ -185,7 +215,7 @@ func (c *Controller) advanceCanary(name string, namespace string) {
185215
// check if the canary success rate is above the threshold
186216
// skip check if no traffic is routed to canary
187217
if canaryRoute.Weight == 0 {
188-
c.recordEventInfof(cd, "Starting canary deployment for %s.%s", cd.Name, cd.Namespace)
218+
c.recordEventInfof(cd, "Starting canary analysis for %s.%s", cd.Spec.TargetRef.Name, cd.Namespace)
189219
} else {
190220
if ok := c.analyseCanary(cd); !ok {
191221
if err := c.deployer.SetStatusFailedChecks(cd, cd.Status.FailedChecks+1); err != nil {
@@ -260,14 +290,14 @@ func (c *Controller) advanceCanary(name string, namespace string) {
260290
}
261291
}
262292

263-
func (c *Controller) checkCanaryStatus(cd *flaggerv1.Canary, deployer CanaryDeployer) bool {
293+
func (c *Controller) checkCanaryStatus(cd *flaggerv1.Canary) bool {
264294
c.recorder.SetStatus(cd)
265295
if cd.Status.Phase == flaggerv1.CanaryProgressing {
266296
return true
267297
}
268298

269299
if cd.Status.Phase == "" {
270-
if err := deployer.SyncStatus(cd, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryInitialized}); err != nil {
300+
if err := c.deployer.SyncStatus(cd, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryInitialized}); err != nil {
271301
c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).Errorf("%v", err)
272302
return false
273303
}
@@ -278,15 +308,15 @@ func (c *Controller) checkCanaryStatus(cd *flaggerv1.Canary, deployer CanaryDepl
278308
return false
279309
}
280310

281-
if diff, err := deployer.IsNewSpec(cd); diff {
311+
if diff, err := c.deployer.IsNewSpec(cd); diff {
282312
c.recordEventInfof(cd, "New revision detected! Scaling up %s.%s", cd.Spec.TargetRef.Name, cd.Namespace)
283313
c.sendNotification(cd, "New revision detected, starting canary analysis.",
284314
true, false)
285-
if err = deployer.Scale(cd, 1); err != nil {
315+
if err = c.deployer.Scale(cd, 1); err != nil {
286316
c.recordEventErrorf(cd, "%v", err)
287317
return false
288318
}
289-
if err := deployer.SyncStatus(cd, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryProgressing}); err != nil {
319+
if err := c.deployer.SyncStatus(cd, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryProgressing}); err != nil {
290320
c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).Errorf("%v", err)
291321
return false
292322
}
@@ -296,6 +326,15 @@ func (c *Controller) checkCanaryStatus(cd *flaggerv1.Canary, deployer CanaryDepl
296326
return false
297327
}
298328

329+
func (c *Controller) hasCanaryRevisionChanged(cd *flaggerv1.Canary) bool {
330+
if cd.Status.Phase == flaggerv1.CanaryProgressing {
331+
if diff, _ := c.deployer.IsNewSpec(cd); diff {
332+
return true
333+
}
334+
}
335+
return false
336+
}
337+
299338
func (c *Controller) analyseCanary(r *flaggerv1.Canary) bool {
300339
// run metrics checks
301340
for _, metric := range r.Spec.CanaryAnalysis.Metrics {

0 commit comments

Comments
 (0)