-
Notifications
You must be signed in to change notification settings - Fork 895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: rollout status always in progressing if analysis fails #1099
Conversation
b531789
to
df0ead8
Compare
Codecov Report
@@ Coverage Diff @@
## master #1099 +/- ##
=======================================
Coverage 81.24% 81.24%
=======================================
Files 104 104
Lines 9420 9420
=======================================
Hits 7653 7653
Misses 1257 1257
Partials 510 510
Continue to review full report at Codecov.
|
- when analysis fails but rollout completes, the status should be degraded Signed-off-by: Hui Kang <[email protected]>
b161324
to
2d087c9
Compare
Signed-off-by: Hui Kang <[email protected]>
2d087c9
to
800145a
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -585,7 +585,7 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt | |||
// In such a case, we should simply not estimate any progress for this rollout. | |||
currentCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutProgressing) | |||
|
|||
isCompleteRollout := newStatus.Replicas == newStatus.AvailableReplicas && currentCond != nil && currentCond.Reason == conditions.NewRSAvailableReason | |||
isCompleteRollout := newStatus.Replicas == newStatus.AvailableReplicas && currentCond != nil && currentCond.Reason == conditions.NewRSAvailableReason && currentCond.Type != v1alpha1.RolloutProgressing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just reviewed the calculateRolloutConditions
to learn what's going on inside and found one interesting place where I'm a little bit confused:
The next expression currentCond.Type != v1alpha1.RolloutProgressing
will always be false
as the currentCond
is a condition filtered by the v1alpha1.RolloutProgressing type.
That means isCompleteRollout
is always false
. Maybe we will remove it?
@jessesuen @huikang Please correct me here if I'm wrong.
be degraded
address #942