Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

unittest: tiny fix for incorrect parameters #348

Merged
merged 1 commit into from
Jun 1, 2018

Conversation

keloyang
Copy link
Contributor

runCommand([]string{"docker", "image", "inspect", testDockerImage}) will always fail, and it lead testcase pull busybox every time whether it exists or not.

Fixes #347

Signed-off-by: y00316549 yangshukui@huawei.com

@katabuilder
Copy link

PSS Measurement:
Qemu: 144251 KB
Proxy: 6695 KB
Shim: 11102 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 2014588 KB

@jodh-intel
Copy link
Contributor

Hi @keloyang - I'm a little confused since the following two commands appear to be equivalent, atleast using docker 18.03.1-ce...?!

  • docker image inspect $image
  • docker inspect $image

@WeiZhang555
Copy link
Member

Guess you are trying to test it with an older version of docker? @keloyang

@sboeuf
Copy link

sboeuf commented May 30, 2018

@jodh-intel yeah I guess @keloyang is trying to use a command that works both for old Docker versions as well as more recent ones like 18.03

@jodh-intel
Copy link
Contributor

@keloyang - it would be useful for us to know which version of docker you're targetting here as we try to keep track of that:

Would it be docker-1.11.2 again by any chance (#334)? 😄

@amshinde
Copy link
Member

@keloyang Yes, curious about the docker version where this is failing.
If we decide to to go with docker inspect, a more specific check with docker inspect --type=image busybox should be used, rather than looking for any docker object.

@keloyang
Copy link
Contributor Author

@amshinde @jodh-intel @WeiZhang555 the version I used is 1.11.2, maybe it's better to use compatible command, WDYT ?

@WeiZhang555
Copy link
Member

I think it makes sense to use a more compatible command. As @amshinde said, docker inspect --type=image busybox can be a better improvement.
The change will be LGTM with @amshinde 's suggested modification.

@amshinde
Copy link
Member

@keloyang Agree on using a more compatible command.

@codecov
Copy link

codecov bot commented Jun 1, 2018

Codecov Report

Merging #348 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #348   +/-   ##
=======================================
  Coverage   63.74%   63.74%           
=======================================
  Files          87       87           
  Lines        8731     8731           
=======================================
  Hits         5566     5566           
  Misses       2569     2569           
  Partials      596      596

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f5f619...c10db01. Read the comment docs.

Fixes #347

Signed-off-by: y00316549 <yangshukui@huawei.com>
@keloyang
Copy link
Contributor Author

keloyang commented Jun 1, 2018

ping @amshinde @WeiZhang555 added --type=image, thanks.

@sboeuf
Copy link

sboeuf commented Jun 1, 2018

I have restarted the failing builds and opened kata-containers/tests#367 related to the CI failure.

@katabuilder
Copy link

PSS Measurement:
Qemu: 161983 KB
Proxy: 4636 KB
Shim: 8799 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 1997500 KB

@jodh-intel
Copy link
Contributor

jodh-intel commented Jun 1, 2018

lgtm

Approved with PullApprove

@jodh-intel jodh-intel self-requested a review June 1, 2018 07:51
@jodh-intel
Copy link
Contributor

pullapprove seems to be confused. We have the requisite number of acks and the CI is green, so merging...

@jodh-intel jodh-intel merged commit 42765bf into kata-containers:master Jun 1, 2018
zklei pushed a commit to zklei/runtime that referenced this pull request Jun 13, 2019
Adds support for running OCI hooks within the guest. A 'drop-in'
path (guest_hook_path) is specified in the cli configuration file
and if set, the agent will look for OCI hooks in this directory
and inject them into the container life cycle.

Fixes: kata-containers#348
Replaces: kata-containers#365

Co-authored-by: Edward Guzman <eguzman@nvidia.com>
Co-authored-by: Felix Abecassis <fabecassis@nvidia.com>
Signed-off-by: Edward Guzman <eguzman@nvidia.com>
Signed-off-by: Felix Abecassis <fabecassis@nvidia.com>
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.

6 participants