updater: Fix rollback regression#2105
Conversation
When Monitor is not explicitly set in the spec, we're supposed to use the default. Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
|
This is needed for 17.05. |
Codecov Report
@@ Coverage Diff @@
## master #2105 +/- ##
==========================================
- Coverage 59.23% 57.11% -2.12%
==========================================
Files 117 117
Lines 19361 20128 +767
==========================================
+ Hits 11468 11497 +29
- Misses 6563 7308 +745
+ Partials 1330 1323 -7Continue to review full report at Codecov.
|
| assert.Equal(t, observedTask.Status.State, api.TaskStateNew) | ||
| assert.Equal(t, observedTask.Spec.GetContainer().Image, "image1") | ||
|
|
||
| if !setMonitor { |
There was a problem hiding this comment.
Rather than just return, is there some other way we can set a test for this regression? Possibly changing the default value, so that we can set a timeout and hence detect if this regresses again?
There was a problem hiding this comment.
The test checks that the rollback is triggered (see the check for UpdateStatus_ROLLBACK_STARTED above). The rest of the test is about what happens after the rollback starts, and I wouldn't have included this in a test designed to catch this regression, but wanted to reuse the code of this existing test, so I made it return here for this new testcase. Does that sound okay?
|
LGTM, sorry for missing this! |
|
LGTM |
3 participants
When Monitor is not explicitly set in the spec, we're supposed to use the default. This regressed in #2080.
cc @cyli @dongluochen