Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: use netNSPath dynamically #2443

Merged
merged 1 commit into from
Nov 8, 2018
Merged

bugfix: use netNSPath dynamically #2443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions cri/ocicni/cni_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

cnicurrent "github.com/containernetworking/cni/pkg/types/current"
"github.com/cri-o/ocicni/pkg/ocicni"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -75,11 +76,18 @@ func (c *CniManager) SetUpPodNetwork(podNetwork *ocicni.PodNetwork) error {

// TearDownPodNetwork is the method called before a pod's sandbox container will be deleted.
func (c *CniManager) TearDownPodNetwork(podNetwork *ocicni.PodNetwork) error {
// perform the teardown network operation whatever to
// give CNI Plugin a chance to perform some operations
err := c.plugin.TearDownPod(*podNetwork)
Copy link
Contributor

Choose a reason for hiding this comment

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

how about:

if err :=  c.plugin.TearDownPod(*podNetwork); err == nil {
    return nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

err will be used later. So no need to change. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

make sense

if err != nil {
return fmt.Errorf("failed to destroy network for sandbox %q: %v", podNetwork.ID, err)
if err == nil {
return nil
}
return nil

// if netNSPath is not found, should return the error of IsNotExist.
if _, err = os.Stat(podNetwork.NetNS); err != nil {
return err
}
return errors.Wrapf(err, "failed to destroy network for sandbox %q", podNetwork.ID)
}

// GetPodNetworkStatus is the method called to obtain the ipv4 or ipv6 addresses of the pod sandbox.
Expand Down
82 changes: 19 additions & 63 deletions cri/v1alpha2/cri.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,35 +318,13 @@ func (c *CriManager) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
}

// Step 4: Setup networking for the sandbox.
var netnsPath string
networkNamespaceMode := config.GetLinux().GetSecurityContext().GetNamespaceOptions().GetNetwork()
// If it is in host network, no need to configure the network of sandbox.
if networkNamespaceMode != runtime.NamespaceMode_NODE {
netnsPath, err = c.setupPodNetwork(ctx, id, config)
err = c.setupPodNetwork(ctx, id, config)
if err != nil {
return nil, err
}
defer func() {
// Teardown network if an error is returned.
if retErr != nil {
teardownNetErr := c.CniMgr.TearDownPodNetwork(&ocicni.PodNetwork{
Name: config.GetMetadata().GetName(),
Namespace: config.GetMetadata().GetNamespace(),
ID: id,
NetNS: netnsPath,
PortMappings: toCNIPortMappings(config.GetPortMappings()),
})
if teardownNetErr != nil {
logrus.Errorf("failed to destroy network for sandbox %q: %v", id, teardownNetErr)
}
}
}()

// update the metadata of sandbox container after network had been set up successfully.
sandboxMeta.NetNSPath = netnsPath
if err := c.SandboxStore.Put(sandboxMeta); err != nil {
return nil, err
}
}

metrics.PodSuccessActionsCounter.WithLabelValues(label).Inc()
Expand Down Expand Up @@ -390,36 +368,13 @@ func (c *CriManager) StartPodSandbox(ctx context.Context, r *runtime.StartPodSan
sandboxMeta := res.(*SandboxMeta)

// setup networking for the sandbox.
var netnsPath string
networkNamespaceMode := sandboxMeta.Config.GetLinux().GetSecurityContext().GetNamespaceOptions().GetNetwork()
// If it is in host network, no need to configure the network of sandbox.
if networkNamespaceMode != runtime.NamespaceMode_NODE {
netnsPath, err = c.setupPodNetwork(ctx, podSandboxID, sandboxMeta.Config)
err = c.setupPodNetwork(ctx, podSandboxID, sandboxMeta.Config)
if err != nil {
return nil, err
}
defer func() {
// Teardown network if an error is returned.
if err != nil {
teardownNetErr := c.CniMgr.TearDownPodNetwork(&ocicni.PodNetwork{
Name: sandboxMeta.Config.GetMetadata().GetName(),
Namespace: sandboxMeta.Config.GetMetadata().GetNamespace(),
ID: podSandboxID,
NetNS: netnsPath,
PortMappings: toCNIPortMappings(sandboxMeta.Config.GetPortMappings()),
})
if teardownNetErr != nil {
logrus.Errorf("failed to destroy network for sandbox %q: %v", podSandboxID, teardownNetErr)
}
}
}()
}

// update sandboxMeta
sandboxMeta.NetNSPath = netnsPath
err = c.SandboxStore.Put(sandboxMeta)
if err != nil {
return nil, err
}

metrics.PodSuccessActionsCounter.WithLabelValues(label).Inc()
Expand Down Expand Up @@ -477,23 +432,24 @@ func (c *CriManager) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb

// Teardown network of the pod, if it is not in host network mode.
if !hostNet {
_, err = os.Stat(sandboxMeta.NetNSPath)
// If the sandbox has been stopped, the corresponding network namespace will not exist.
if err == nil {
err = c.CniMgr.TearDownPodNetwork(&ocicni.PodNetwork{
Name: metadata.GetName(),
Namespace: metadata.GetNamespace(),
ID: podSandboxID,
NetNS: sandboxMeta.NetNSPath,
PortMappings: toCNIPortMappings(sandboxMeta.Config.GetPortMappings()),
})
if err != nil {
sandbox, err := c.ContainerMgr.Get(ctx, podSandboxID)
if err != nil {
return nil, fmt.Errorf("failed to get sandbox %q: %v", podSandboxID, err)
}

netNSPath := containerNetns(sandbox)
err = c.CniMgr.TearDownPodNetwork(&ocicni.PodNetwork{
Name: metadata.GetName(),
Namespace: metadata.GetNamespace(),
ID: podSandboxID,
NetNS: netNSPath,
PortMappings: toCNIPortMappings(sandboxMeta.Config.GetPortMappings()),
})
if err != nil {
if !os.IsNotExist(err) {
return nil, err
}
} else if !os.IsNotExist(err) {
return nil, fmt.Errorf("failed to stat network namespace file %s of sandbox %s: %v", sandboxMeta.NetNSPath, podSandboxID, err)
} else {
logrus.Warnf("failed to find network namespace file %s of sandbox %s which may have been already stopped", sandboxMeta.NetNSPath, podSandboxID)
logrus.Warnf("failed to find network namespace file %s of sandbox %s which may have been already stopped", netNSPath, podSandboxID)
}
}

Expand Down Expand Up @@ -607,7 +563,7 @@ func (c *CriManager) PodSandboxStatus(ctx context.Context, r *runtime.PodSandbox
var ip string
// No need to get ip for host network mode.
if !hostNet {
ip, err = c.CniMgr.GetPodNetworkStatus(sandboxMeta.NetNSPath)
ip, err = c.CniMgr.GetPodNetworkStatus(containerNetns(sandbox))
if err != nil {
// Maybe the pod has been stopped.
logrus.Warnf("failed to get ip of sandbox %q: %v", podSandboxID, err)
Expand Down
5 changes: 1 addition & 4 deletions cri/v1alpha2/cri_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@ type SandboxMeta struct {
// Config is CRI sandbox config.
Config *runtime.PodSandboxConfig

// NetNSPath is the network namespace used by the sandbox.
NetNSPath string

// Runtime is the runtime of sandbox
Runtime string

// Runtime whether to enable lxcfs for a container
LxcfsEnabled bool

// Netns is the sandbox's network namespace
// NetNS is the sandbox's network namespace
NetNS string
}

Expand Down
13 changes: 4 additions & 9 deletions cri/v1alpha2/cri_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,28 +455,23 @@ func setupSandboxFiles(sandboxRootDir string, config *runtime.PodSandboxConfig)

// setupPodNetwork sets up the network of PodSandbox and return the netnsPath of PodSandbox
// and do nothing when networkNamespaceMode equals runtime.NamespaceMode_NODE.
func (c *CriManager) setupPodNetwork(ctx context.Context, id string, config *runtime.PodSandboxConfig) (string, error) {
func (c *CriManager) setupPodNetwork(ctx context.Context, id string, config *runtime.PodSandboxConfig) error {
container, err := c.ContainerMgr.Get(ctx, id)
if err != nil {
return "", err
return err
}
netnsPath := containerNetns(container)
if netnsPath == "" {
return "", fmt.Errorf("failed to find network namespace path for sandbox %q", id)
return fmt.Errorf("failed to find network namespace path for sandbox %q", id)
}

err = c.CniMgr.SetUpPodNetwork(&ocicni.PodNetwork{
return c.CniMgr.SetUpPodNetwork(&ocicni.PodNetwork{
Name: config.GetMetadata().GetName(),
Namespace: config.GetMetadata().GetNamespace(),
ID: id,
NetNS: netnsPath,
PortMappings: toCNIPortMappings(config.GetPortMappings()),
})
if err != nil {
return "", err
}

return netnsPath, nil
}

// Container related tool functions.
Expand Down