OSD-27228: Ensure containers are cleaned up when SIGINT is called #586
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@joshbranham: This pull request references OSD-27228 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@joshbranham: This pull request references OSD-27228 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@joshbranham: This pull request references OSD-27228 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: joshbranham The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
joshbranham
force-pushed
the
console-container-cleanup
branch
from
6731810 to
1ddf826
Compare
|
@joshbranham: all tests passed! 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-sigs/prow repository. I understand the commands that are listed here. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #586 +/- ##
==========================================
+ Coverage 45.67% 45.72% +0.04%
==========================================
Files 87 87
Lines 6782 6779 -3
==========================================
+ Hits 3098 3100 +2
+ Misses 3299 3294 -5
Partials 385 385
|
What type of PR is this?
bug
What this PR does / Why we need it?
When running
ocm-backplane console $IDon macOS using Docker (in my casecolimawith the docker CLI binary), issuing a ctrl+c, akaSIGINT, the container(s) created would get left behind. Then a subsequentocm-backplane console $IDfor the same cluster would fail.The primary change here is to use
exec.CommandContextand thecancel()channel for thedockerRunConsoleContainer()function. This allows the context to cancel command execution, and then normal container cleanup runs.Additionally, made some other changes to cleanup the code. The biggest takeaway here is not using a global variable for the context and cancel func.
Which Jira/Github issue(s) does this PR fix?
Resolves https://issues.redhat.com/browse/OSD-27228
Special notes for your reviewer
Please run through a test of this locally on your machine. I have tested this on macOS with Docker, but ideally we want to have the following tested:
My test result:
Unit Test Coverage
Guidelines
Test coverage checks
Pre-checks (if applicable)