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

metrics: Remove sleep while shutting down multiple containers #636

Merged
merged 1 commit into from
Aug 23, 2018

Conversation

GabyCT
Copy link
Contributor

@GabyCT GabyCT commented Aug 22, 2018

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

Fixes #635

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 while shutting down containers.

Fixes kata-containers#635

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

@devimc devimc left a comment

Choose a reason for hiding this comment

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

lgtm

@grahamwhaley
Copy link
Contributor

l g t m - and, whilst you are there, there is another instance over at https://github.com/kata-containers/tests/blob/master/metrics/density/footprint_data.sh#L160
Can you add that to this PR as well pls? :-)

I believe the use of rm -f is fine/correct - it will have the same effect. Just wanted to check we were not going to end up issuing a 'STOP' to the container, as that then introduces a 10s extra delay (whilst the STOP is ignored by most of the containers).

@grahamwhaley
Copy link
Contributor

Re-kicking 16.04 CI. afaict, it failed the log parser:

ERROR: kata-log-parser: failed to parse agent log entry {Count:0 TimeDelta:0 Filename:/tmp/jenkins/workspace/kata-containers-tests-ubuntu-16-04-PR/go/src/github.com/kata-containers/tests/kata-proxy.log Line:45878 Time:2018-08-22 20:48:42.641204298 +0000 UTC Pid:3089 Level:info Msg:time="2018-08-22T20:48:42.58584803Z" level=debug msg="new request" name=kata-agent pid=96 

@devimc devimc merged commit fe31510 into kata-containers:master Aug 23, 2018
@egernst egernst removed the review label Aug 23, 2018
@grahamwhaley
Copy link
Contributor

oh, I guess I should have marked that as 'request changes then' - as noted, there is another ocurrence of this that could have been fixed at the same time. @GabyCT - guess that will have to go on a new PR now.
I'll be a bit firmer with the github flow in future then...

@GabyCT GabyCT deleted the topic/removesleeps 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.

4 participants