Skip to content

Commit

Permalink
Force to invoke all CNI plugin's delete at pods' tearing down
Browse files Browse the repository at this point in the history
In case of multiple interfaces in pod, when the pod is deleted,
forEachnetwork() is called with multiple network attachments.
If forEachnetwork() causes the error at the middle of processing networks,
then forEachnetwork() just returns and following network is not processed.
From CNI runtime point of view, all CNI plugin should be invoked to delete
interfaces.

This change introduce 'force' option in forEachnetwork() and try to
continue to process (i.e. delete network) even though forEachnetwork()
causes the error.
  • Loading branch information
s1061123 committed Mar 26, 2021
1 parent 87390b5 commit c20b9df
Showing 1 changed file with 20 additions and 5 deletions.
25 changes: 20 additions & 5 deletions pkg/ocicni/ocicni.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ func (plugin *cniNetworkPlugin) loadNetworkFromCache(name string, rt *libcni.Run

type forEachNetworkFn func(*cniNetwork, *PodNetwork, *libcni.RuntimeConf) error

func (plugin *cniNetworkPlugin) forEachNetwork(podNetwork *PodNetwork, fromCache bool, actionFn forEachNetworkFn) error {
func (plugin *cniNetworkPlugin) forEachNetwork(podNetwork *PodNetwork, fromCache bool, actionFn forEachNetworkFn, force bool) error {
networks := podNetwork.Networks
if len(networks) == 0 {
networks = append(networks, NetAttachment{
Expand All @@ -457,6 +457,9 @@ func (plugin *cniNetworkPlugin) forEachNetwork(podNetwork *PodNetwork, fromCache
if req.Ifname != "" {
// Make sure the requested name isn't already assigned
if allIfNames[req.Ifname] {
if force {
continue
}
return fmt.Errorf("network %q requested interface name %q already assigned", req.Name, req.Ifname)
}
allIfNames[req.Ifname] = true
Expand All @@ -482,6 +485,9 @@ func (plugin *cniNetworkPlugin) forEachNetwork(podNetwork *PodNetwork, fromCache
rt, err := buildCNIRuntimeConf(podNetwork, ifName, podNetwork.RuntimeConfig[network.Name])
if err != nil {
logrus.Errorf("error building CNI runtime config: %v", err)
if force {
continue
}
return err
}

Expand All @@ -503,16 +509,25 @@ func (plugin *cniNetworkPlugin) forEachNetwork(podNetwork *PodNetwork, fromCache
// try to load the networks again
if err2 := plugin.syncNetworkConfig(); err2 != nil {
logrus.Error(err2)
if force {
continue
}
return err
}
cniNet, err = plugin.getNetwork(network.Name)
if err != nil {
if force {
continue
}
return err
}
}
}

if err := actionFn(cniNet, podNetwork, rt); err != nil {
if force {
continue
}
return err
}
}
Expand Down Expand Up @@ -552,7 +567,7 @@ func (plugin *cniNetworkPlugin) SetUpPodWithContext(ctx context.Context, podNetw
},
})
return nil
}); err != nil {
}, false); err != nil {
return nil, err
}

Expand Down Expand Up @@ -658,7 +673,7 @@ func (plugin *cniNetworkPlugin) TearDownPodWithContext(ctx context.Context, podN
return fmt.Errorf("Error while removing pod from CNI network %q: %s", network.name, err)
}
return nil
})
}, true)
}

// GetPodNetworkStatus returns IP addressing and interface details for all
Expand Down Expand Up @@ -695,7 +710,7 @@ func (plugin *cniNetworkPlugin) GetPodNetworkStatusWithContext(ctx context.Conte
})
}
return nil
}); err != nil {
}, false); err != nil {
return nil, err
}

Expand Down Expand Up @@ -810,7 +825,7 @@ func buildCNIRuntimeConf(podNetwork *PodNetwork, ifName string, runtimeConfig Ru
}

// Propagate existing CNI_ARGS to non-k8s consumers
for _, kvpairs := range strings.Split(os.Getenv("CNI_ARGS"), ";"){
for _, kvpairs := range strings.Split(os.Getenv("CNI_ARGS"), ";") {
if keyval := strings.SplitN(kvpairs, "=", 2); len(keyval) == 2 {
rt.Args = append(rt.Args, [2]string{keyval[0], keyval[1]})
}
Expand Down

0 comments on commit c20b9df

Please sign in to comment.