Address comment.

Signed-off-by: Lantao Liu <lantaol@google.com>
This commit is contained in:
Lantao Liu 2019-09-19 14:05:19 -07:00
parent 35eb96d901
commit c1ece0c801
2 changed files with 24 additions and 15 deletions

View File

@ -137,14 +137,13 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
// In this case however caching the IP will add a subtle performance enhancement by avoiding // In this case however caching the IP will add a subtle performance enhancement by avoiding
// calls to network namespace of the pod to query the IP of the veth interface on every // calls to network namespace of the pod to query the IP of the veth interface on every
// SandboxStatus request. // SandboxStatus request.
sandbox.IP, sandbox.AdditionalIPs, sandbox.CNIResult, err = c.setupPodNetwork(ctx, id, sandbox.NetNSPath, config) if err := c.setupPodNetwork(ctx, &sandbox); err != nil {
if err != nil {
return nil, errors.Wrapf(err, "failed to setup network for sandbox %q", id) return nil, errors.Wrapf(err, "failed to setup network for sandbox %q", id)
} }
defer func() { defer func() {
if retErr != nil { if retErr != nil {
// Teardown network if an error is returned. // Teardown network if an error is returned.
if err := c.teardownPodNetwork(ctx, id, sandbox.NetNSPath, config); err != nil { if err := c.teardownPodNetwork(ctx, sandbox); err != nil {
log.G(ctx).WithError(err).Errorf("Failed to destroy network for sandbox %q", id) log.G(ctx).WithError(err).Errorf("Failed to destroy network for sandbox %q", id)
} }
} }
@ -303,31 +302,37 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
} }
// setupPodNetwork setups up the network for a pod // setupPodNetwork setups up the network for a pod
func (c *criService) setupPodNetwork(ctx context.Context, id string, path string, config *runtime.PodSandboxConfig) (string, []string, *cni.CNIResult, error) { func (c *criService) setupPodNetwork(ctx context.Context, sandbox *sandboxstore.Sandbox) error {
var (
id = sandbox.ID
config = sandbox.Config
path = sandbox.NetNSPath
)
if c.netPlugin == nil { if c.netPlugin == nil {
return "", nil, nil, errors.New("cni config not initialized") return errors.New("cni config not initialized")
} }
opts, err := cniNamespaceOpts(id, config) opts, err := cniNamespaceOpts(id, config)
if err != nil { if err != nil {
return "", nil, nil, errors.Wrap(err, "get cni namespace options") return errors.Wrap(err, "get cni namespace options")
} }
result, err := c.netPlugin.Setup(ctx, id, path, opts...) result, err := c.netPlugin.Setup(ctx, id, path, opts...)
if err != nil { if err != nil {
return "", nil, nil, err return err
} }
logDebugCNIResult(ctx, id, result) logDebugCNIResult(ctx, id, result)
// Check if the default interface has IP config // Check if the default interface has IP config
if configs, ok := result.Interfaces[defaultIfName]; ok && len(configs.IPConfigs) > 0 { if configs, ok := result.Interfaces[defaultIfName]; ok && len(configs.IPConfigs) > 0 {
ip, additionalIPs := selectPodIPs(configs.IPConfigs) sandbox.IP, sandbox.AdditionalIPs = selectPodIPs(configs.IPConfigs)
return ip, additionalIPs, result, nil sandbox.CNIResult = result
return nil
} }
// If it comes here then the result was invalid so destroy the pod network and return error // If it comes here then the result was invalid so destroy the pod network and return error
if err := c.teardownPodNetwork(ctx, id, path, config); err != nil { if err := c.teardownPodNetwork(ctx, *sandbox); err != nil {
log.G(ctx).WithError(err).Errorf("Failed to destroy network for sandbox %q", id) log.G(ctx).WithError(err).Errorf("Failed to destroy network for sandbox %q", id)
} }
return "", nil, result, errors.Errorf("failed to find network info for sandbox %q", id) return errors.Errorf("failed to find network info for sandbox %q", id)
} }
// cniNamespaceOpts get CNI namespace options from sandbox config. // cniNamespaceOpts get CNI namespace options from sandbox config.

View File

@ -71,15 +71,14 @@ func (c *criService) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb
// Teardown network for sandbox. // Teardown network for sandbox.
if sandbox.NetNS != nil { if sandbox.NetNS != nil {
netNSPath := sandbox.NetNSPath
// Use empty netns path if netns is not available. This is defined in: // Use empty netns path if netns is not available. This is defined in:
// https://github.com/containernetworking/cni/blob/v0.7.0-alpha1/SPEC.md // https://github.com/containernetworking/cni/blob/v0.7.0-alpha1/SPEC.md
if closed, err := sandbox.NetNS.Closed(); err != nil { if closed, err := sandbox.NetNS.Closed(); err != nil {
return nil, errors.Wrap(err, "failed to check network namespace closed") return nil, errors.Wrap(err, "failed to check network namespace closed")
} else if closed { } else if closed {
netNSPath = "" sandbox.NetNSPath = ""
} }
if err := c.teardownPodNetwork(ctx, id, netNSPath, sandbox.Config); err != nil { if err := c.teardownPodNetwork(ctx, sandbox); err != nil {
return nil, errors.Wrapf(err, "failed to destroy network for sandbox %q", id) return nil, errors.Wrapf(err, "failed to destroy network for sandbox %q", id)
} }
if err = sandbox.NetNS.Remove(); err != nil { if err = sandbox.NetNS.Remove(); err != nil {
@ -156,11 +155,16 @@ func (c *criService) waitSandboxStop(ctx context.Context, sandbox sandboxstore.S
} }
// teardownPodNetwork removes the network from the pod // teardownPodNetwork removes the network from the pod
func (c *criService) teardownPodNetwork(ctx context.Context, id string, path string, config *runtime.PodSandboxConfig) error { func (c *criService) teardownPodNetwork(ctx context.Context, sandbox sandboxstore.Sandbox) error {
if c.netPlugin == nil { if c.netPlugin == nil {
return errors.New("cni config not initialized") return errors.New("cni config not initialized")
} }
var (
id = sandbox.ID
path = sandbox.NetNSPath
config = sandbox.Config
)
opts, err := cniNamespaceOpts(id, config) opts, err := cniNamespaceOpts(id, config)
if err != nil { if err != nil {
return errors.Wrap(err, "get cni namespace options") return errors.Wrap(err, "get cni namespace options")