Don't drop traffic when upgrading a deployment fails #14795
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
knative-prow
bot
added
do-not-merge/work-in-progress
knative-prow
bot
added
approved
dprotaso
force-pushed
the
reachability-fix
branch
from
8e0535d
to
a18e776
Compare
knative-prow
bot
added
size/L
dprotaso
force-pushed
the
reachability-fix
branch
2 times, most recently
from
d2f5d97
to
1687537
Compare
dprotaso
force-pushed
the
reachability-fix
branch
from
1687537
to
6ac8cfa
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #14795 +/- ##
==========================================
- Coverage 86.02% 85.78% -0.24%
==========================================
Files 197 198 +1
Lines 14950 15126 +176
==========================================
+ Hits 12860 12976 +116
- Misses 1778 1827 +49
- Partials 312 323 +11 ☔ View full report in Codecov by Sentry. |
skonto
reviewed
Jan 22, 2024
When transforming the deployment status to the revision we want to bubble up the more severe condition to Ready. Since Replica failures will include a more actionable error message this condition is preferred
This isn't accurate when the Revision has failed to rollout an update to it's deployment
dprotaso
force-pushed
the
reachability-fix
branch
from
6ac8cfa
to
eaa966b
Compare
1. PA Reachability now depends on the status of the Deployment If we have available replicas we don't mark the revision as unreachable. This allows ongoing requests to be handled 2. Always propagate the K8s Deployment Status to the Revision. We don't need to gate this depending on whether the Revision required activation. Since the only two conditions we propagate from the Deployment is Progressing and ReplicaSetFailure=False 3. Mark Revision as Deploying if the PA's service name isn't set
dprotaso
force-pushed
the
reachability-fix
branch
from
eaa966b
to
23247c1
Compare
|
/test-all |
knative-prow
bot
added
the
area/test-and-release
dprotaso
force-pushed
the
reachability-fix
branch
2 times, most recently
from
cf53416
to
b8b019b
Compare
|
I'm testing the upgrade test to the 1.12 branch to ensure it fails when the fix isn't present. - #14840 |
dprotaso
force-pushed
the
reachability-fix
branch
from
b8b019b
to
87c1f10
Compare
dprotaso
force-pushed
the
reachability-fix
branch
from
3882852
to
42dc27d
Compare
knative-prow
bot
added
size/XL
dprotaso
force-pushed
the
reachability-fix
branch
from
591db65
to
42dc27d
Compare
knative-prow
bot
added
size/L
dprotaso
changed the title
knative-prow
bot
removed
the
do-not-merge/work-in-progress
|
/hold So the upgrade test is failing because the bug exists in 1.12 and 1.13 release branches. When we upgrade from those older release branches the older controller observes the config map changes and then marks reachability=false incorrectly. Thus to ensure this upgrade test works we should cherry pick to 1.12 - perform a release, then cherry pick to 1.13 then do a release. |
knative-prow
bot
added
the
do-not-merge/hold
|
/retest v1.13.1 is out that should unblock the upgrade test |
knative-prow
bot
removed
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprotaso, ReToCode The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #14660
Depends on 1.13 point release with the following fix #14846