Bug 1943804: increases termination timeouts for AWS #1079
Conversation
|
/hold for testing |
openshift-ci-robot
added
the
do-not-merge/hold
p0lyn0mial
force-pushed
the
increase-shutdown-delay-duration
branch
from
0a43a9b
to
77ab7c4
Compare
p0lyn0mial
changed the title
p0lyn0mial
force-pushed
the
increase-shutdown-delay-duration
branch
from
77ab7c4
to
7be44f0
Compare
p0lyn0mial
changed the title
|
i suspect this will trigger alerts and apiserver degraded due to the extended duration (but maybe not. |
p0lyn0mial
force-pushed
the
increase-shutdown-delay-duration
branch
from
7be44f0
to
23d451a
Compare
|
/retest |
p0lyn0mial
force-pushed
the
increase-shutdown-delay-duration
branch
from
382d2e5
to
fdbeffc
Compare
openshift-ci
bot
added
the
needs-rebase
p0lyn0mial
force-pushed
the
increase-shutdown-delay-duration
branch
from
fdbeffc
to
1b58143
Compare
openshift-ci
bot
removed
the
needs-rebase
p0lyn0mial
changed the title
p0lyn0mial
changed the title
openshift-ci-robot
added
the
bugzilla/severity-urgent
|
@p0lyn0mial: This pull request references Bugzilla bug 1943804, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
bugzilla/invalid-bug
|
/assign @sttts |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/bugzilla refresh |
openshift-ci-robot
added
bugzilla/valid-bug
|
@p0lyn0mial: This pull request references Bugzilla bug 1943804, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
p0lyn0mial
force-pushed
the
increase-shutdown-delay-duration
branch
from
b54bd3a
to
702a485
Compare
p0lyn0mial
force-pushed
the
increase-shutdown-delay-duration
branch
from
702a485
to
e9a818d
Compare
| observedShutdownDelayDuration = "210s" | ||
| default: | ||
| // just return the existing configuration since we don't have an opinion anyway | ||
| return existingConfig, errs |
There was a problem hiding this comment.
existingConfig is not no opinion. You have to return an empty map.
There was a problem hiding this comment.
why an empty map? I haven't changed existingConfig so I am simply returning what I have got.
There was a problem hiding this comment.
You return the input for another merge. This is not about changing, it's about outputting desired state.
| observedGracefulTerminationDuration = "275" | ||
| default: | ||
| // just return the existing configuration since we don't have an opinion anyway | ||
| return existingConfig, errs |
There was a problem hiding this comment.
same here, I haven't changed existingConfig so I am simply returning what I have got.
| // the initial 70s is reserved fo the minimal termination period | ||
| // additional 60s for finishing all in-flight requests | ||
| // an extra 5s to make sure the potential SIGTERM will be sent after the server terminates itself | ||
| gracefulTerminationDuration = 135 |
There was a problem hiding this comment.
do we need this? Shouldn't this come from defaultconfig.yaml?
There was a problem hiding this comment.
yes, we need this.
defaultconfig.yaml defines shutdown-delay-duration
p0lyn0mial
force-pushed
the
increase-shutdown-delay-duration
branch
from
4422e85
to
729b5fe
Compare
p0lyn0mial
force-pushed
the
increase-shutdown-delay-duration
branch
from
729b5fe
to
78122f8
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: p0lyn0mial, sttts 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 |
openshift-ci
bot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@p0lyn0mial: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@p0lyn0mial: All pull requests linked via external trackers have merged:
Bugzilla bug 1943804 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Due to a known AWS issue: https://bugzilla.redhat.com/show_bug.cgi?id=1943804
the time needed for an LB to notice and remove unhealthy instances must be extended.
shutdown-delay-durationflag has been changed from70sto210sandterminationGracePeriodSecondsfield from135sto275s.For
terminationGracePeriodSecondsthe initial210sis reserved for the minimal termination period.Additional
60sfor finishing all in-flight requests and an extra5sto make sure a potential SIGTERM will be sent after a server terminates itself.