fix: zero-value abortScaleDownDelay was not honored with setCanaryScale #1375
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1375 +/- ##
==========================================
+ Coverage 81.29% 81.31% +0.02%
==========================================
Files 108 108
Lines 10033 10037 +4
==========================================
+ Hits 8156 8162 +6
+ Misses 1317 1315 -2
Partials 560 560
Continue to review full report at Codecov.
|
jessesuen
force-pushed
the
fix-e2e-breakage
branch
from
fbc1246 to
7cff453
Compare
jessesuen
changed the title
jessesuen
force-pushed
the
fix-e2e-breakage
branch
2 times, most recently
from
3f07cc9 to
7c66320
Compare
|
Still trying to fix tests. |
Signed-off-by: Jesse Suen <[email protected]>
jessesuen
force-pushed
the
fix-e2e-breakage
branch
from
7c66320 to
14f8ffa
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -53,7 +53,7 @@ func (c *rolloutContext) addScaleDownDelay(rs *appsv1.ReplicaSet, scaleDownDelay | |||
| } | |||
| return nil | |||
| } | |||
| deadline := metav1.Now().Add(scaleDownDelaySeconds * time.Second).UTC().Format(time.RFC3339) | |||
| deadline := metav1.Now().Add(scaleDownDelaySeconds).UTC().Format(time.RFC3339) | |||
There was a problem hiding this comment.
Fixed confusing method behavior where duration was expected to be supplied in nanoseconds.
There was a problem hiding this comment.
All callers of this method were supplying scaleDownDelaySeconds in nanosecond duration. I also changed the callers to simply supply proper durations.
| } else { | ||
| abortScaleDownDelaySeconds := time.Duration(defaults.GetAbortScaleDownDelaySecondsOrDefault(c.rollout)) | ||
| err = c.addScaleDownDelay(c.newRS, abortScaleDownDelaySeconds) | ||
| } else if abortScaleDownDelaySeconds != nil { |
There was a problem hiding this comment.
is this condition check required as it is verified in shouldDelayScaleDownOnAbort
There was a problem hiding this comment.
You're right that it's not required. But I was trying to be defensive here in case there was a bug or change in behavior to shouldDelayScaleDownOnAbort().
Recent PR did not rebase ontop of new scaledowndelay on abort behavior and the two caused a conflict in behavior
Signed-off-by: Jesse Suen [email protected]