Skip to content
This repository was archived by the owner on May 6, 2020. It is now read-only.

Conversation

@jodh-intel
Copy link

Re-vendor virtcontainers and update the runtime to move to using one
proxy instance for each VM (pod). Also incorporates requires runtime
changes to take account of virtcontainers API changes.

virtcontainers changes since last re-vendor:

01b012b vendor: Move the ciao vendoring to ciao-project
148b06a proxy: Launch one proxy instance per pod
619300a proxy: Use CC3 proxy path
d8f2a22 shim: Only show shim-count if there are any
c993e45 shim: Quote urls in error

Fixes #795.

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com

@jodh-intel
Copy link
Author

Here is the "proper" PR that replaces #833. This will unbreak the virtcontainers tests (which pull in the runtime code), but this PR is going to fail at the CI stage until we get a new clear-containers-image package on the CI systems that includes clearcontainers/agent#160.

@erick0z, @gorozco1 - we might need to create a new clear-containers-image package early including the latest agent code.

/cc @sboeuf, @chavafg, @sameo.

Adding dnm tag to flag that this will break the CI tests currently.

@jodh-intel jodh-intel mentioned this pull request Nov 28, 2017
7 tasks
@clearcontainersbot
Copy link

kubernetes qa-passed 👍

@chavafg
Copy link
Contributor

chavafg commented Nov 28, 2017

Took a look at the logs and see these failures from unit tests:

16:40:22 INFO: Running 'go test' as current user on packages '.' with flags '-v -race -timeout 10s'
16:40:28 # github.com/clearcontainers/runtime
16:40:28 ./cc-env_test.go:137: unknown field 'URL' in struct literal of type ProxyInfo
16:40:28 ./cc-env_test.go:746: unknown field 'URL' in struct literal of type ProxyInfo
16:40:28 ./config_test.go:143: unknown field 'URL' in struct literal of type virtcontainers.CCProxyConfig
16:40:28 ./config_test.go:510: unknown field 'URL' in struct literal of type virtcontainers.CCProxyConfig
16:40:28 ./config_test.go:789: p.url undefined (type proxy has no field or method url)
16:40:28 ./config_test.go:789: undefined: defaultProxyURL
16:40:28 ./config_test.go:792: p.URL undefined (type proxy has no field or method URL)
16:40:28 ./config_test.go:793: p.url undefined (type proxy has no field or method url)
16:40:28 FAIL	github.com/clearcontainers/runtime [build failed]
16:40:28 make: *** [Makefile:307: check-go-test] Error 2
16:40:28 Build step 'Execute shell' marked build as failure

@jodh-intel
Copy link
Author

Ouch - fixing...

@jodh-intel
Copy link
Author

Branch updated.

@gorozco1
Copy link
Contributor

cc @jcvenegas

@clearcontainersbot
Copy link

kubernetes qa-passed 👍

jcvenegas added a commit to jcvenegas/cc-tests that referenced this pull request Nov 29, 2017
Allows test for clearcontainers/runtime#835

Fixes: clearcontainers#760

Changes:

version: 19170
Changes in package clear-containers-agent (from 243e2aefa4f9ff5a1bd32967a213e8533dab54df-15 to 04fa18641b046dbcf10d13dcd0715e288bff41ce-16):
     Jose Carlos Venegas Munoz - version bump from 04fa18641b046dbcf10d13dcd0715e288bff41ce-15 to 04fa18641b046dbcf10d13dcd0715e288bff41ce-16
     Jose Carlos Venegas Munoz - agent: New agent version 04fa18
https://download.clearlinux.org/releases/19170/clear/RELEASENOTES
version: 19180
Changes in package iptables (from 1.6.1-21 to 1.6.1-22):
     Arjan van de Ven - version bump from 1.6.1-21 to 1.6.1-22
https://download.clearlinux.org/releases/19180/clear/RELEASENOTES
version: 19200
Changes in package systemd (from 234-157 to 234-158):
     Arjan van de Ven - ship btrfs rule
https://download.clearlinux.org/releases/19200/clear/RELEASENOTES
version: 19340
Changes in package clear-containers-agent (from 04fa18641b046dbcf10d13dcd0715e288bff41ce-16 to b1d92e2aa14d915680e8fe770745a511857ac7ff-17):
     Jose Carlos Venegas Munoz - version bump from b1d92e2aa14d915680e8fe770745a511857ac7ff-16 to b1d92e2aa14d915680e8fe770745a511857ac7ff-17
     Jose Carlos Venegas Munoz - New agent version: b1d92e2
https://download.clearlinux.org/releases/19340/clear/RELEASENOTES
version: 19350
Changes in package clear-containers-agent (from 04fa18641b046dbcf10d13dcd0715e288bff41ce-16 to b1d92e2aa14d915680e8fe770745a511857ac7ff-17):
     Jose Carlos Venegas Munoz - version bump from b1d92e2aa14d915680e8fe770745a511857ac7ff-16 to b1d92e2aa14d915680e8fe770745a511857ac7ff-17
     Jose Carlos Venegas Munoz - New agent version: b1d92e2
https://download.clearlinux.org/releases/19350/clear/RELEASENOTES

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
@jcvenegas
Copy link
Contributor

jcvenegas commented Nov 29, 2017

@jodh-intel the image with last agent changes is out, can review clearcontainers/tests#761 ?

Re-vendor virtcontainers and update the runtime to move to using one
proxy instance for each VM (pod). Also incorporates requires runtime
changes to take account of virtcontainers API changes.

virtcontainers changes since last re-vendor:

    01b012b vendor: Move the ciao vendoring to ciao-project
    148b06a proxy: Launch one proxy instance per pod
    619300a proxy: Use CC3 proxy path
    d8f2a22 shim: Only show shim-count if there are any
    c993e45 shim: Quote urls in error

Fixes clearcontainers#795.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel
Copy link
Author

Hi @jcvenegas - thanks - I've re-pushed to trigger a re-test.

However, I was actually expecting the tests to fail before the agent was updated on clearcontainers/tests#761, so I don't think this should be part of todays release - we need to understand the results before it's merged.

@clearcontainersbot
Copy link

kubernetes qa-passed 👍

@jodh-intel
Copy link
Author

I've re-tested this branch with 3.0.9 and it's all working fine.

Also confirmed that this PR fails when testing with image version 19030, but passes with 19070 and 19350 (which contain the required agent changes).

@jodh-intel
Copy link
Author

@grahamwhaley, @sameo - could you review so we can unbreak the virtcontainers builds?

@sameo
Copy link

sameo commented Nov 30, 2017

LGTM

Approved with PullApprove Approved with PullApprove

@grahamwhaley grahamwhaley merged commit bfb4891 into clearcontainers:master Nov 30, 2017
chavafg added a commit to chavafg/tests-cc that referenced this pull request Dec 6, 2017
Now that the cc-proxy will not be a service, we need to use
option -t in journalctl to get the logs from the proxy.
Relates to:
clearcontainers/runtime#835
clearcontainers/proxy#177

Fixes: clearcontainers#775

Signed-off-by: Salvador Fuentes <salvador.fuentes@intel.com>
chavafg added a commit to chavafg/tests-cc that referenced this pull request Dec 6, 2017
Now that the cc-proxy will not be a service, we need to use
option -t in journalctl to get the logs from the proxy.
Relates to:
clearcontainers/runtime#835
clearcontainers/proxy#177

Fixes: clearcontainers#775

Signed-off-by: Salvador Fuentes <salvador.fuentes@intel.com>
jodh-intel pushed a commit to jodh-intel/runtime that referenced this pull request Jan 9, 2018
Update the architecture document to reflect the new proxy behaviour
where there is a proxy instance per virtual machine rather than a
single host-level proxy instance.

This should strictly have been handled on clearcontainers#835.

Fixes clearcontainers#908.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
jodh-intel pushed a commit to jodh-intel/runtime that referenced this pull request Jan 9, 2018
Update the architecture document to reflect the new proxy behaviour
where there is a proxy instance per virtual machine rather than a
single host-level proxy instance.

This should strictly have been handled on clearcontainers#835.

Fixes clearcontainers#908.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
jodh-intel pushed a commit to jodh-intel/runtime that referenced this pull request Jan 9, 2018
Update the architecture document to reflect the new proxy behaviour
where there is a proxy instance per virtual machine rather than a
single host-level proxy instance.

This should strictly have been handled on clearcontainers#835.

Fixes clearcontainers#908.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
jodh-intel pushed a commit to jodh-intel/runtime that referenced this pull request Jan 10, 2018
Update the architecture document to reflect the new proxy behaviour
where there is a proxy instance per virtual machine rather than a
single host-level proxy instance.

This should strictly have been handled on clearcontainers#835.

Fixes clearcontainers#908.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.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.

7 participants