rootfs: Support agent tracing#200
Conversation
|
Blocked on kata-containers/agent#415. |
grahamwhaley
left a comment
There was a problem hiding this comment.
changes look good to me.
nice cleanup pickups as well.
lgtm
rootfs-builder/rootfs.sh
Outdated
|
|
||
| AGENT_TRACE When set to "yes", create a rootfs containing additional | ||
| elements to support tracing the agent using https://jaegertracing.io. | ||
| Incompatible with AGENT_INIT="yes". |
There was a problem hiding this comment.
single whitespace indent anomoly?
|
but you tripped over your own travis URL check :-) |
1c35f5c to
b91b0c3
Compare
|
@grahamwhaley - yep, that was expected - see blocked comment above ;) |
|
Added dnm to make it clearer there is a block on the agent PR. |
b91b0c3 to
909d373
Compare
|
This PR is still blocked on kata-containers/agent#415. |
909d373 to
bd23d58
Compare
|
Branch updated for latest agent tracing branch (kata-containers/agent#415). |
|
Please do NOT remove the If it does land, it will currently break agent output since |
|
@jodh-intel What's the relevance/status of this one? |
|
@egernst - it's sitting here "semi-blocked" on kata-containers/runtime#1352; we could land this PR now, but as we're about to rework the agent changes for opencensus, there may be no point as this PR won't work with that. I'll leave it up to @mcastelino to decide though. Related: kata-containers/runtime#1369. |
|
kata-containers/agent#415 has now landed so this can now land. /retest |
Add missing seccomp packages for Debian and openSuSE config files. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Use single equals in string test for consistency, replace the visually distracting `|| true` by a single call to `true(1)` at the end of the script. This guarantees the sourced script succeeds even if any of the tests it makes fail. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
bd23d58 to
d033f57
Compare
|
Regarding my comment here: #200 (comment), I've reworked the PR slightly so that by default the agent output will not be redirected to the journal. Now that kata-containers/shim#172 has landed, this is the behaviour we want I think. Note also: kata-containers/agent#547. |
|
/retest |
| ARCH_EXCLUDE_LIST=() | ||
|
|
||
| [ "$SECCOMP" = "yes" ] && PACKAGES+=" libseccomp" | ||
| [ "$AGENT_TRACE" = "yes" ] && PACKAGES+=" socat" |
There was a problem hiding this comment.
@jodh-intel Does this need to go to the Dockerfiles as well? I am not clear if the Dockerfiles are used when USE_DOCKER is true.
There was a problem hiding this comment.
Not really as the docker files are mostly static - we want socat to be added conditionally to avoid bloat at this stage.
Add `AGENT_TRACE=yes` option to build a rootfs with agent tracing support. Fixes kata-containers#199. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
d033f57 to
0059ca2
Compare
|
/retest |
|
@jodh-intel Can you take a look at the CI failures? Will be good to land this PR. |
|
/test |
egernst
left a comment
There was a problem hiding this comment.
Looks fine. AFAICT we don't have any tests in place which will exercise this, correct?
|
/retest |
|
CI seems miserable with this change. @jcvenegas , @chavafg - seems there may be some issues w/ the osbuilder CI? |
|
I should probably tal at this PR again tomorrow as it hasn't been touched in a while, so adding dnm for now... |
|
Let me close this one as it's a do-not-merge which won't make to the 1.13.0 release. |
Add
AGENT_TRACE=yesoption to build a rootfs with agent tracing support.Fixes #199.
Signed-off-by: James O. D. Hunt james.o.hunt@intel.com