Skip to content

OCPBUGS-18548: Stop all MicroShift components when signaled to exit#2319

Merged
openshift-merge-robot merged 3 commits intoopenshift:mainfrom
pmtk:OCPBUGS-18548-graceful-shutdown-of-microshift
Sep 8, 2023
Merged

OCPBUGS-18548: Stop all MicroShift components when signaled to exit#2319
openshift-merge-robot merged 3 commits intoopenshift:mainfrom
pmtk:OCPBUGS-18548-graceful-shutdown-of-microshift

Conversation

@pmtk
Copy link
Member

@pmtk pmtk commented Sep 7, 2023

make KCM use context we give to command

By using cmd.Context(), KCM will use a context MicroShift passes down through all the components.

When using context.Background() it won't ever stop, because it's never cancelled, and result in blocking ServiceManager which waits for all components to stop and make MicroShift use its whole internal gracefulShutdownTimer.


override KAS' shutdown-delay-duration to 5s (from 70s)

shutdown-delay-duration tells KAS how long to serve after receiving a stop signal.

Its value is provided to time.Sleep(), so it's used in full; there is not short-circuit mechanism that would let KAS exit earlier.

Default value of 70s is set in assets/controllers/kube-apiserver/defaultconfig.yaml

@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Sep 7, 2023
@openshift-ci-robot
Copy link

@pmtk: This pull request references Jira Issue OCPBUGS-18548, which is invalid:

  • expected the bug to target the "4.14.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

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 openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 7, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 7, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 7, 2023
@pmtk
Copy link
Member Author

pmtk commented Sep 7, 2023

/test all

pmtk added 3 commits September 8, 2023 09:35
By using cmd.Context(), KCM will use a context
MicroShift passes down through all the components.

When using context.Background() it won't ever stop,
because it's never cancelled, and result in blocking
ServiceManager which waits for all components to
stop and make MicroShift use its whole
internal gracefulShutdownTimer.
shutdown-delay-duration tells KAS how long
to serve after receiving a stop signal.

Its value is provided to time.Sleep(), so it's used in full;
there is not short-circuit mechanism that would let KAS
exit earlier.
@pmtk pmtk force-pushed the OCPBUGS-18548-graceful-shutdown-of-microshift branch from 9e28f91 to 0454f28 Compare September 8, 2023 08:10
@pmtk pmtk marked this pull request as ready for review September 8, 2023 08:11
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 8, 2023
@pmtk pmtk changed the title OCPBUGS-18548: Graceful shutdown OCPBUGS-18548: Stop all MicroShift components when signalled to exit Sep 8, 2023
@openshift-ci openshift-ci bot requested review from jerpeter1 and jogeo September 8, 2023 08:13
@pmtk
Copy link
Member Author

pmtk commented Sep 8, 2023

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Sep 8, 2023
@openshift-ci-robot
Copy link

@pmtk: This pull request references Jira Issue OCPBUGS-18548, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jogeo

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

/jira refresh

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
Copy link

@pmtk: This pull request references Jira Issue OCPBUGS-18548, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jogeo

Details

In response to this:

make KCM use context we give to command

By using cmd.Context(), KCM will use a context MicroShift passes down through all the components.

When using context.Background() it won't ever stop, because it's never cancelled, and result in blocking ServiceManager which waits for all components to stop and make MicroShift use its whole internal gracefulShutdownTimer.


override KAS' shutdown-delay-duration to 5s

shutdown-delay-duration tells KAS how long to serve after receiving a stop signal.

Its value is provided to time.Sleep(), so it's used in full; there is not short-circuit mechanism that would let KAS exit earlier.

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.

@pmtk pmtk changed the title OCPBUGS-18548: Stop all MicroShift components when signalled to exit OCPBUGS-18548: Stop all MicroShift components when signaled to exit Sep 8, 2023
@openshift-ci-robot
Copy link

@pmtk: This pull request references Jira Issue OCPBUGS-18548, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jogeo

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

make KCM use context we give to command

By using cmd.Context(), KCM will use a context MicroShift passes down through all the components.

When using context.Background() it won't ever stop, because it's never cancelled, and result in blocking ServiceManager which waits for all components to stop and make MicroShift use its whole internal gracefulShutdownTimer.


override KAS' shutdown-delay-duration to 5s (from 70s)

shutdown-delay-duration tells KAS how long to serve after receiving a stop signal.

Its value is provided to time.Sleep(), so it's used in full; there is not short-circuit mechanism that would let KAS exit earlier.

Default value of 70s is set in assets/controllers/kube-apiserver/defaultconfig.yaml

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.

@eggfoobar
Copy link
Contributor

/lgtm

nice find!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eggfoobar, pmtk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 8, 2023

@pmtk: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

@openshift-merge-robot openshift-merge-robot merged commit 5f4721d into openshift:main Sep 8, 2023
@openshift-ci-robot
Copy link

@pmtk: Jira Issue OCPBUGS-18548: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-18548 has been moved to the MODIFIED state.

Details

In response to this:

make KCM use context we give to command

By using cmd.Context(), KCM will use a context MicroShift passes down through all the components.

When using context.Background() it won't ever stop, because it's never cancelled, and result in blocking ServiceManager which waits for all components to stop and make MicroShift use its whole internal gracefulShutdownTimer.


override KAS' shutdown-delay-duration to 5s (from 70s)

shutdown-delay-duration tells KAS how long to serve after receiving a stop signal.

Its value is provided to time.Sleep(), so it's used in full; there is not short-circuit mechanism that would let KAS exit earlier.

Default value of 70s is set in assets/controllers/kube-apiserver/defaultconfig.yaml

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.

@pmtk
Copy link
Member Author

pmtk commented Sep 11, 2023

/cherrypick release-4.14

@openshift-cherrypick-robot

@pmtk: new pull request created: #2330

Details

In response to this:

/cherrypick release-4.14

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants