Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

metrics: Remove sleep for the footprint script #645

Merged
merged 1 commit into from
Aug 24, 2018

Conversation

GabyCT
Copy link
Contributor

@GabyCT GabyCT commented Aug 23, 2018

Now that this issue has been solved kata-containers/agent#263,
it is necessary to remove the workaround of sleeping.

Fixes #644

Signed-off-by: Gabriela Cervantes [email protected]

Now that this issue has been solved kata-containers/agent#263,
it is necessary to remove the workaround of sleeping.

Fixes kata-containers#644

Signed-off-by: Gabriela Cervantes <[email protected]>
@egernst egernst added the review label Aug 23, 2018
@jodh-intel
Copy link
Contributor

jodh-intel commented Aug 23, 2018

\o/

lgtm

Approved with PullApprove

# request ends up doing a kill anyhow
docker kill $c
sleep 3
docker stop $c
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do a 'kill' or 'rm -f' instead please - issuing a 'stop' has a tendency to get the STOP ignored at the container end, and then that hits us with a default 10s timeout penalty, before docker issues a kill anyway.
See moby/moby#3766 and a number of other related Issues for details if interested.

So, this could probably just be a:

$ docker kill $(docker ps -qa)

which is a little harsh, as it will kill all containers even if we did not launch them - but, I think the pre-init scripts have probably ensured that is the case already when running metrics tests.

thx!

@GabyCT GabyCT force-pushed the topic/sleepfootprint branch from 33e86ee to 0694e29 Compare August 23, 2018 16:10
@GabyCT
Copy link
Contributor Author

GabyCT commented Aug 23, 2018

@grahamwhaley changes applied :)

@chavafg
Copy link
Contributor

chavafg commented Aug 24, 2018

lgtm

Approved with PullApprove

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

as the run has an --rm on it, that
lgtm

@grahamwhaley grahamwhaley merged commit 3db5109 into kata-containers:master Aug 24, 2018
@egernst egernst removed the review label Aug 24, 2018
@GabyCT GabyCT deleted the topic/sleepfootprint branch July 29, 2021 21:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants