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.

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

ci: only kill containers that are based on the solanalabs/rust* images #32292

Merged
merged 4 commits into from
Jun 29, 2023

Conversation

yihau
Copy link
Member

@yihau yihau commented Jun 27, 2023

Problem

I'm trying to migrate our ci environment into a single docker image. Kill all containers will kill the buildkite process as well. Also it seems too aggressive to kill all containers in the machine.

Summary of Changes

only kill containers that are based on the solanalabs/rust* images.

@yihau yihau requested review from t-nelson and mvines June 27, 2023 08:43
@yihau yihau added v1.14 v1.16 PRs that should be backported to v1.16 labels Jun 27, 2023
@yihau yihau marked this pull request as ready for review June 27, 2023 08:43
t-nelson
t-nelson previously approved these changes Jun 27, 2023
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

lgtm, just nits

on another note, seems like everything in this hook is hacks around behavior that we should try to correct?

.buildkite/hooks/post-checkout Outdated Show resolved Hide resolved
.buildkite/hooks/post-checkout Outdated Show resolved Hide resolved
@yihau
Copy link
Member Author

yihau commented Jun 27, 2023

yeah! I separate this script into 1) process killing and 2) cargo uninstall

for 1) I would like to just remove them and see do we encounter issues. these codes are added like 4y ago. maybe buildkite has already those issues 🤔

for 2) I guess I can remove them when the migration finish!

@t-nelson
Copy link
Contributor

yeah! I separate this script into 1) process killing and 2) cargo uninstall

🙌

for 1) I would like to just remove them and see do we encounter issues. these codes are added like 4y ago. maybe buildkite has already those issues 🤔

if i had to guess this is more likely to be hung tests not responding to the kill signal than some bk problem

for 2) I guess I can remove them when the migration finish!

probably. the problem isn't exactly clear from the comment. i get how cargo uses CARGO_HOME, but not what the conflict is that makes us want to uninstall these bins by force. worst case seem like the solution would be to install them in a custom dir and control that dir's priority in PATH 🤷

@yihau yihau merged commit 3e5ee8d into solana-labs:master Jun 29, 2023
@yihau yihau deleted the docker-kill-rule branch June 29, 2023 04:18
mergify bot pushed a commit that referenced this pull request Jun 29, 2023
#32292)

* ci: only kill containers that are based on the solanalabs/rust* images

* Update .buildkite/hooks/post-checkout

Co-authored-by: Trent Nelson <[email protected]>

* Update .buildkite/hooks/post-checkout

Co-authored-by: Trent Nelson <[email protected]>

* fix lint

---------

Co-authored-by: Trent Nelson <[email protected]>
(cherry picked from commit 3e5ee8d)

# Conflicts:
#	.buildkite/hooks/post-checkout
mergify bot pushed a commit that referenced this pull request Jun 29, 2023
#32292)

* ci: only kill containers that are based on the solanalabs/rust* images

* Update .buildkite/hooks/post-checkout

Co-authored-by: Trent Nelson <[email protected]>

* Update .buildkite/hooks/post-checkout

Co-authored-by: Trent Nelson <[email protected]>

* fix lint

---------

Co-authored-by: Trent Nelson <[email protected]>
(cherry picked from commit 3e5ee8d)
mergify bot added a commit that referenced this pull request Jun 29, 2023
…* images (backport of #32292) (#32325)

* ci: only kill containers that are based on the solanalabs/rust* images (#32292)

* ci: only kill containers that are based on the solanalabs/rust* images

* Update .buildkite/hooks/post-checkout

Co-authored-by: Trent Nelson <[email protected]>

* Update .buildkite/hooks/post-checkout

Co-authored-by: Trent Nelson <[email protected]>

* fix lint

---------

Co-authored-by: Trent Nelson <[email protected]>
(cherry picked from commit 3e5ee8d)

# Conflicts:
#	.buildkite/hooks/post-checkout

* fix conflict

---------

Co-authored-by: Yihau Chen <[email protected]>
Co-authored-by: yihau <[email protected]>
mergify bot added a commit that referenced this pull request Jun 29, 2023
…* images (backport of #32292) (#32326)

ci: only kill containers that are based on the solanalabs/rust* images (#32292)

* ci: only kill containers that are based on the solanalabs/rust* images

* Update .buildkite/hooks/post-checkout

Co-authored-by: Trent Nelson <[email protected]>

* Update .buildkite/hooks/post-checkout

Co-authored-by: Trent Nelson <[email protected]>

* fix lint

---------

Co-authored-by: Trent Nelson <[email protected]>
(cherry picked from commit 3e5ee8d)

Co-authored-by: Yihau Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.16 PRs that should be backported to v1.16
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants