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

Networking: Ensure that network namespace is propagated#1665

Merged
jodh-intel merged 1 commit intokata-containers:masterfrom
mcastelino:topic/bug_fix_1664
May 15, 2019
Merged

Networking: Ensure that network namespace is propagated#1665
jodh-intel merged 1 commit intokata-containers:masterfrom
mcastelino:topic/bug_fix_1664

Conversation

@mcastelino
Copy link
Contributor

Network namespace needs to be propagated if available at
createSandbox()

Fixes: #1664

Signed-off-by: Manohar Castelino manohar.r.castelino@intel.com

Network namespace needs to be propagated if available at
createSandbox()

Fixes: kata-containers#1664

Signed-off-by: Manohar Castelino <manohar.r.castelino@intel.com>
@mcastelino mcastelino requested review from bergwolf and sboeuf May 11, 2019 01:03
shmSize: sandboxConfig.ShmSize,
sharePidNs: sandboxConfig.SharePidNs,
stateful: sandboxConfig.Stateful,
networkNS: NetworkNamespace{NetNsPath: sandboxConfig.NetworkConfig.NetNSPath},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcastelino Looking at your description of the issue #1664 there are actually 2 ways to fix this. The network config is stored inside sandbox.config.NetworkConfig, so you could find the same information from the config.
But this patch looks good to me too :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two are the same, -- sandbox.config is sandboxConfig as assigned above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sboeuf yes the information is available within the store, and that is what I ended up doing. However it is wrong for us to pass in an empty value when there is a value present. So either we need to remove that parameter and always retrieve if from config or set it up properly.

@sboeuf
Copy link

sboeuf commented May 12, 2019

/test

shmSize: sandboxConfig.ShmSize,
sharePidNs: sandboxConfig.SharePidNs,
stateful: sandboxConfig.Stateful,
networkNS: NetworkNamespace{NetNsPath: sandboxConfig.NetworkConfig.NetNSPath},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two are the same, -- sandbox.config is sandboxConfig as assigned above.

@jodh-intel
Copy link

/retest

@mcastelino
Copy link
Contributor Author

Also when debugging this I find create sandbox being called multiple times during the course of the container creation. This is a bit unexpected. Hopefully the traces will indicate why.

@jcvenegas
Copy link
Member

/test

@jcvenegas
Copy link
Member

Re-test - now that kernel issue is fixed

@jodh-intel
Copy link

Merging as the sles CI is dead...

@jodh-intel jodh-intel merged commit 86d51f5 into kata-containers:master May 15, 2019
This was referenced Jun 3, 2019
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.

Network namespace not available at hypervisor.createSandbox time

7 participants