pkg/oci: honour CPU period and quota

[devimc]

the number of vCPUs must be calculated using CPU period and quota doesn't matter if a memory limit was specified or not

fixes #540

Signed-off-by: Julio Montes julio.montes@intel.com

[sboeuf]

Thanks for fixing the semantic here ! LGTM

[sboeuf]

@jodh-intel @sameo please merge if you're fine with that.

[jodh-intel]

Good catch. The code looks good, but we clearly need some tests for this.

Could you add some to TestVmConfig()? We might also (separately) want a .bats test for asserting the Docker args over in https://github.com/clearcontainers/tests too.

[devimc]

@jodh-intel yes, I'm going to add a test in https://github.com/clearcontainers/tests

[jodh-intel]

@devimc - thanks. I think we do need a unit test too though as TestVmConfig() was updated for quota/period but clearly is missing some cases as we would have expected that test to fail without your fix on this PR.

[devimc]

@jodh-intel sure, working on that

[devimc]

@jodh-intel done, thanks

[jodh-intel]

Thanks @devimc - I've tested this and with your new test and the old code, TestVmConfig() fails as we'd expect.

lgtm

[sameo]

LGTM

[sameo]

coveralls shows a 3.5% drop in coverage with this PR. @devimc Could you please look at it?

[devimc]

Hi @sameo , I can't see coveralls results

coverage/coveralls — Waiting for status to be reported 

[jodh-intel]

Hi @devimc - something odd is up with coveralls today. If you look at the 16.04 log file, you can see https://coveralls.io/jobs/32558455, but like #539 I think it's probably lying about the drop in coverage (you could test that locally I guess).

[devimc]

@jodh-intel yep, I think the same (coverall lied) @sameo I don't see any drop in the coverage

cd $GOPATH/src/github.com/containers/virtcontainers/pkg/oci
go test -run TestVmConfig -cover -coverprofile cover.txt
go tool cover -html=cover.txt -o coverage.html

[sameo]

LGTM then