agent: set sandbox id log field in CreateSandbox#309
agent: set sandbox id log field in CreateSandbox#309bergwolf merged 1 commit intokata-containers:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #309 +/- ##
==========================================
+ Coverage 43.4% 43.64% +0.24%
==========================================
Files 14 14
Lines 2336 2362 +26
==========================================
+ Hits 1014 1031 +17
- Misses 1187 1195 +8
- Partials 135 136 +1
Continue to review full report at Codecov.
|
| sync.RWMutex | ||
|
|
||
| id string | ||
| hostname string |
There was a problem hiding this comment.
These hostname changes seem unrelated? Maybe put them on a different commit with an explanation?
There was a problem hiding this comment.
It's related... The original code saves hostname to sandbox.id. I moved it to sandbox.hostname.
| repeated string dns = 2; | ||
| repeated Storage storages = 3; | ||
|
|
There was a problem hiding this comment.
I deleted the extra whitespaces because it was unintentionally added by someone. I don't think we need white space there.
| // a shared pid namespace. All containers created will join this shared | ||
| // pid namespace. | ||
| bool sandbox_pidns = 4; | ||
| string sandbox_id = 5; |
There was a problem hiding this comment.
It might be worth adding a comment here explaining that we don't need this member in the other sandbox and network calls (as listed in kata-containers/tests#453 (comment)) since we implicitly require CreateSandbox() to be called before any other sandbox/network calls :)
There was a problem hiding this comment.
Sure, I'll add some text about it.
This effectively reverts 04457e3("logging: Add sandbox field"). CreateSandbox() is the point we know which sandbox id will use this guest. If we rely on the kernel command line for it, we have to put sandbox id in qemu args and it breaks vm factory. Fixes: kata-containers#308 Signed-off-by: Peng Tao <[email protected]>
|
lgtm!
|
|
@sboeuf , the problem is not Swarm, the test is detecting that some processes are running before running Swarm |
|
@GabyCT I see, but this |
|
@sboeuf , using the following configuration I ran |
|
As agreed we do not trust |
5 participants
This effectively reverts 04457e3("logging: Add sandbox field").
CreateSandbox() is the point we know which sandbox id will use
this guest. If we rely on the kernel command line for it, we have
to put sandbox id in qemu args and it breaks vm factory.
Fixes: #308
cc @jodh-intel