Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Migrate OpenShift Cleanup Functions From util.sh #11838

Closed
wants to merge 3 commits into from
Closed

Migrate OpenShift Cleanup Functions From util.sh #11838

wants to merge 3 commits into from

Conversation

stevekuznetsov
Copy link
Contributor

@Miciah PTAL the last two commits

stevekuznetsov
stevekuznetsov commented Nov 8, 2016
View reviewed changes
hack/lib/cleanup.sh
@@ -35,18 +35,16 @@ function dump_container_logs() {
return
fi

mkdir -p ${LOG_DIR}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can assume it exists, we do everywhere else and this isn't special.

hack/lib/cleanup.sh
local sudo="${USE_SUDO:+sudo}"

pids=($(jobs -pr))
for i in "${pids[@]-}"; do
pgrep -P "${i}" | xargs ${sudo} kill &> /dev/null
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just assume a process kills it's children, or go down the whole tree recursively

@stevekuznetsov
Copy link
Contributor Author

Larger functional changes to come later when I integrate these methods into the EXIT trap.

@stevekuznetsov stevekuznetsov changed the title Skuznets/cleanup migration Migrate OpenShift Cleanup Functions From util.sh Nov 8, 2016
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 13, 2016
@stevekuznetsov stevekuznetsov force-pushed the skuznets/cleanup-migration branch 2 times, most recently from 9279517 to e821671 Compare November 22, 2016 21:48
@stevekuznetsov
Copy link
Contributor Author

@Miciah this is ready for review, second commit is simple Bash improvements, third commit is some functional refactors

@openshift-bot [test] this please

@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2016
@stevekuznetsov
Copy link
Contributor Author

@openshift-bot, the last build failed from the following flakes:

re[test]

@miminar miminar mentioned this pull request Nov 29, 2016
Closed
@stevekuznetsov
Copy link
Contributor Author

@openshift-bot, the last build failed from the following flakes:

re[test]

@stevekuznetsov
Copy link
Contributor Author

@Miciah ping

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 64bb815

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 6, 2016
@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12037/) (Base Commit: 6eefa2a)

@openshift-bot openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 20, 2017
@openshift-bot openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 19, 2017
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2017
@openshift-bot
Copy link
Contributor

Origin Action Required: Pull request cannot be automatically merged, please rebase your branch from latest HEAD and push again

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2017
@stevekuznetsov
Copy link
Contributor Author

Superseded by #14116 and #14435

1 similar comment
@stevekuznetsov
Copy link
Contributor Author

Superseded by #14116 and #14435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@stevekuznetsov @openshift-bot