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

Soak tests: Do not assume hypervisors run with the full path#1803

Closed
mcastelino wants to merge 1 commit intokata-containers:masterfrom
mcastelino:topic/fix_jailer_soak
Closed

Soak tests: Do not assume hypervisors run with the full path#1803
mcastelino wants to merge 1 commit intokata-containers:masterfrom
mcastelino:topic/fix_jailer_soak

Conversation

@mcastelino
Copy link
Copy Markdown
Contributor

@mcastelino mcastelino commented Jul 10, 2019

Soak tests assume hypervisors run with the full path.

However when running a hypervisor with jailer the path is relative to
the jail and hence the check should only look for the process.

For example when running firecracker with jailer the process will be

/firecracker
--id=5af26fcea8dbf413067e90026fd9845515d5a1f300d61f66427760a27ef114c2
--seccomp-level=2 --start-time-us=1562786230593253
--start-time-cpu-us=223 --api-sock=/api.socket�
where /firecracker is relative to the jailed location.

Fixes: #1802

Signed-off-by: Manohar Castelino manohar.r.castelino@intel.com

@GabyCT
Copy link
Copy Markdown
Contributor

GabyCT commented Jul 10, 2019

/test

@mcastelino mcastelino force-pushed the topic/fix_jailer_soak branch from 4221868 to 26d99df Compare July 10, 2019 20:45
@GabyCT
Copy link
Copy Markdown
Contributor

GabyCT commented Jul 10, 2019

it seems that is not detecting the hypervisor

15:21:16 Wrong number of hypervisors running (0 != 1) - stopping
15:21:16 Showing system state:
15:21:16  --Docker ps--
15:21:16 CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES
15:21:16  --kata-runtime list--
15:21:16 ID          PID         STATUS      BUNDLE      CREATED     OWNER
15:21:16  --pgrep kata-proxy--
15:21:16  --pgrep kata-shim--
15:21:16  --pgrep kata-runtime--
15:21:16  --pgrep qemu--
15:21:16 ERROR: Got 1 errors, quitting

@GabyCT
Copy link
Copy Markdown
Contributor

GabyCT commented Jul 10, 2019

@mcastelino to test locally your change just run a container in detach an see if pgrep -a -f firecracker is showing you error, as it seems that is failing in that

@GabyCT
Copy link
Copy Markdown
Contributor

GabyCT commented Jul 10, 2019

/test

@chavafg
Copy link
Copy Markdown
Contributor

chavafg commented Jul 10, 2019

Seems like we need to modify https://github.com/kata-containers/tests/blob/master/metrics/lib/common.bash#L192-L214 to get values from firecracker and nemu

@mcastelino
CI: Do not assume hypervisors run with the full path
cc8b905 
Soak tests assume hypervisors run with the full path.

However when running a hypervisor with jailer the path is relative to
the jail and hence the check should only look for the process.

For example when running firecracker with jailer the process will be

```
 /firecracker
--id=5af26fcea8dbf413067e90026fd9845515d5a1f300d61f66427760a27ef114c2
--seccomp-level=2 --start-time-us=1562786230593253
--start-time-cpu-us=223 --api-sock=/api.socket�
where /firecracker is relative to the jailed location.
```

Fixes: kata-containers#1802.

Signed-off-by: Manohar Castelino <manohar.r.castelino@intel.com>
@mcastelino mcastelino force-pushed the topic/fix_jailer_soak branch from 26d99df to cc8b905 Compare July 10, 2019 22:59
@mcastelino
Copy link
Copy Markdown
Contributor Author

mcastelino commented Jul 10, 2019

/test_fc

@mcastelino mcastelino mentioned this pull request Jul 11, 2019
Merged
@mcastelino
Copy link
Copy Markdown
Contributor Author

mcastelino commented Jul 11, 2019

@mcastelino to test locally your change just run a container in detach an see if pgrep -a -f firecracker is showing you error, as it seems that is failing in that

@GabyCT yes it does. And tests passes locally.

grahamwhaley
grahamwhaley approved these changes Jul 11, 2019
View reviewed changes
Copy link
Copy Markdown
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.

code change lgtm
there are some tricky rules with pkill and pgrep to do with paths and pattern matching iirc, but afaict this change looks right.

integration/stability/soak_parallel_rm.sh
how_many_qemus=$(pgrep -a -f ${HYPERVISOR_PATH} | wc -l)
if (( ${how_many_running} != ${how_many_qemus} )); then
echo "Wrong number of qemus running (${how_many_running} != ${how_many_qemus}) - stopping"
vmm_process=$(basename ${HYPERVISOR_PATH})
Copy link
Copy Markdown
Contributor

@grahamwhaley grahamwhaley Jul 11, 2019

Choose a reason for hiding this comment

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

note: you can use the bashism ${x##*/} or similar instead of invoking basename :-)

@grahamwhaley
Copy link
Copy Markdown
Contributor

grahamwhaley commented Jul 11, 2019

/test
not sure we have a /test-fc yet do we @GabyCT ?

@grahamwhaley
Copy link
Copy Markdown
Contributor

grahamwhaley commented Jul 11, 2019

Seems like we need to modify https://github.com/kata-containers/tests/blob/master/metrics/lib/common.bash#L192-L214 to get values from firecracker and nemu

I also see that has a pgrep -a, and not a pgrep -a -f, which in theory I think with a long path might mean the patterns may not match. Let's keep an eye out for that.

@GabyCT
Copy link
Copy Markdown
Contributor

GabyCT commented Jul 11, 2019

@mcastelino and @grahamwhaley , I just enable the test-fc so @mcastelino can trigger his PR to test without running all the CI (to do it just run /test-fc)

@mcastelino
Copy link
Copy Markdown
Contributor Author

mcastelino commented Jul 11, 2019

/test-fc

@GabyCT
Copy link
Copy Markdown
Contributor

GabyCT commented Jul 11, 2019

we can add an skip to the soak test, as I can not reproduce it locally on a VM this is the issue #1804 where I will debug the CI.

@GabyCT GabyCT mentioned this pull request Jul 11, 2019
Closed
GabyCT added a commit to GabyCT/tests-1 that referenced this pull request Jul 11, 2019
@GabyCT
tests: Skip soak test in firecracker
259fd09 
Currently we have issues while trying to run the soak test on
firecracker (see kata-containers#1803).
We need to remove it so the change kata-containers/runtime#1649
can be merged.

Fixes kata-containers#1805

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
@GabyCT GabyCT mentioned this pull request Jul 11, 2019
Merged
@GabyCT
Copy link
Copy Markdown
Contributor

GabyCT commented Jul 11, 2019

Closing this one as this is being addressed in #1807

@GabyCT GabyCT closed this Jul 11, 2019
GabyCT added a commit to GabyCT/tests-1 that referenced this pull request Jul 17, 2019
@GabyCT
tests: Skip soak test in firecracker
4f709a2 
Currently we have issues while trying to run the soak test on
firecracker (see kata-containers#1803).
We need to remove it so the change kata-containers/runtime#1649
can be merged.

Fixes kata-containers#1805

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
GabyCT added a commit to GabyCT/tests-1 that referenced this pull request Jul 17, 2019
@GabyCT
tests: Skip soak test in firecracker
6e6291e 
Currently we have issues while trying to run the soak test on
firecracker (see kata-containers#1803).
We need to remove it so the change kata-containers/runtime#1649
can be merged.

Fixes kata-containers#1805

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@grahamwhaley grahamwhaley grahamwhaley approved these changes

@GabyCT GabyCT GabyCT approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Soak tests: Soak tests assume hypervisors run with the full path

4 participants

@mcastelino @GabyCT @chavafg @grahamwhaley