Skip to content

Commit

Permalink
Fix multus to support CNI plugin which does not create interface
Browse files Browse the repository at this point in the history 
This change fixes multus to support CNI plugin which does not
create any interface and return empty result. Some CNI plugin
may do network configuration change only and does not create
any interface and return empty CNI result. Current multus assumes
that CNI config always creates some interfaces hence above CNI
plugin is out of assumption and multus may not work with such
plugins.
  • Loading branch information
@s1061123
s1061123 committed May 17, 2023
1 parent 0c37bb0 commit 2230480
Show file tree
Hide file tree
Showing 4 changed files with 258 additions and 59 deletions.
130 changes: 71 additions & 59 deletions pkg/multus/multus.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,14 @@ func DelegateAdd(exec invoke.Exec, kubeClient *k8s.ClientInfo, pod *v1.Pod, dele
}

if pod != nil {
// send kubernetes events
if delegate.Name != "" {
kubeClient.Eventf(pod, v1.EventTypeNormal, "AddedInterface", "Add %s %v from %s", rt.IfName, ips, delegate.Name)
} else {
kubeClient.Eventf(pod, v1.EventTypeNormal, "AddedInterface", "Add %s %v", rt.IfName, ips)
// check Interfaces and IPs because some CNI plugin just return empty result
if res.Interfaces != nil || res.IPs != nil {
// send kubernetes events
if delegate.Name != "" {
kubeClient.Eventf(pod, v1.EventTypeNormal, "AddedInterface", "Add %s %v from %s", rt.IfName, ips, delegate.Name)
} else {
kubeClient.Eventf(pod, v1.EventTypeNormal, "AddedInterface", "Add %s %v", rt.IfName, ips)
}
}
} else {
// for further debug https://github.com/k8snetworkplumbingwg/multus-cni/issues/481
Expand Down Expand Up @@ -636,69 +639,78 @@ func CmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) (c
return nil, cmdPluginErr(k8sArgs, netName, "error adding container to network %q: %v", netName, err)
}

// Remove gateway from routing table if the gateway is not used
deleteV4gateway := false
deleteV6gateway := false
adddefaultgateway := false
if delegate.IsFilterV4Gateway {
deleteV4gateway = true
logging.Debugf("Marked interface %v for v4 gateway deletion", ifName)
} else {
// Otherwise, determine if this interface now gets our default route.
// According to
// https://docs.google.com/document/d/1Ny03h6IDVy_e_vmElOqR7UdTPAG_RNydhVE1Kx54kFQ (4.1.2.1.9)
// the list can be empty; if it is, we'll assume the CNI's config for the default gateway holds,
// else we'll update the defaultgateway to the one specified.
if delegate.GatewayRequest != nil && len(*delegate.GatewayRequest) != 0 {
deleteV4gateway = true
adddefaultgateway = true
logging.Debugf("Detected gateway override on interface %v to %v", ifName, delegate.GatewayRequest)
}
// Master plugin result is always used if present
if delegate.MasterPlugin || result == nil {
result = tmpResult
}

if delegate.IsFilterV6Gateway {
deleteV6gateway = true
logging.Debugf("Marked interface %v for v6 gateway deletion", ifName)
} else {
// Otherwise, determine if this interface now gets our default route.
// According to
// https://docs.google.com/document/d/1Ny03h6IDVy_e_vmElOqR7UdTPAG_RNydhVE1Kx54kFQ (4.1.2.1.9)
// the list can be empty; if it is, we'll assume the CNI's config for the default gateway holds,
// else we'll update the defaultgateway to the one specified.
if delegate.GatewayRequest != nil && len(*delegate.GatewayRequest) != 0 {
deleteV6gateway = true
adddefaultgateway = true
logging.Debugf("Detected gateway override on interface %v to %v", ifName, delegate.GatewayRequest)
}
res, err := cni100.NewResultFromResult(tmpResult)
if err != nil {
logging.Errorf("CmdAdd: failed to read result: %v, but proceed", err)
}

// Remove gateway if `default-route` network selection is specified
if deleteV4gateway || deleteV6gateway {
err = netutils.DeleteDefaultGW(args.Netns, ifName)
if err != nil {
return nil, cmdErr(k8sArgs, "error deleting default gateway: %v", err)
}
err = netutils.DeleteDefaultGWCache(n.CNIDir, rt, netName, ifName, deleteV4gateway, deleteV6gateway)
if err != nil {
return nil, cmdErr(k8sArgs, "error deleting default gateway in cache: %v", err)
// check Interfaces and IPs because some CNI plugin does not create any interface
// and just returns empty result
if res != nil && (res.Interfaces != nil || res.IPs != nil) {
// Remove gateway from routing table if the gateway is not used
deleteV4gateway := false
deleteV6gateway := false
adddefaultgateway := false
if delegate.IsFilterV4Gateway {
deleteV4gateway = true
logging.Debugf("Marked interface %v for v4 gateway deletion", ifName)
} else {
// Otherwise, determine if this interface now gets our default route.
// According to
// https://docs.google.com/document/d/1Ny03h6IDVy_e_vmElOqR7UdTPAG_RNydhVE1Kx54kFQ (4.1.2.1.9)
// the list can be empty; if it is, we'll assume the CNI's config for the default gateway holds,
// else we'll update the defaultgateway to the one specified.
if delegate.GatewayRequest != nil && len(*delegate.GatewayRequest) != 0 {
deleteV4gateway = true
adddefaultgateway = true
logging.Debugf("Detected gateway override on interface %v to %v", ifName, delegate.GatewayRequest)
}
}
}

// Here we'll set the default gateway which specified in `default-route` network selection
if adddefaultgateway {
err = netutils.SetDefaultGW(args.Netns, ifName, *delegate.GatewayRequest)
if err != nil {
return nil, cmdErr(k8sArgs, "error setting default gateway: %v", err)
if delegate.IsFilterV6Gateway {
deleteV6gateway = true
logging.Debugf("Marked interface %v for v6 gateway deletion", ifName)
} else {
// Otherwise, determine if this interface now gets our default route.
// According to
// https://docs.google.com/document/d/1Ny03h6IDVy_e_vmElOqR7UdTPAG_RNydhVE1Kx54kFQ (4.1.2.1.9)
// the list can be empty; if it is, we'll assume the CNI's config for the default gateway holds,
// else we'll update the defaultgateway to the one specified.
if delegate.GatewayRequest != nil && len(*delegate.GatewayRequest) != 0 {
deleteV6gateway = true
adddefaultgateway = true
logging.Debugf("Detected gateway override on interface %v to %v", ifName, delegate.GatewayRequest)
}
}
err = netutils.AddDefaultGWCache(n.CNIDir, rt, netName, ifName, *delegate.GatewayRequest)
if err != nil {
return nil, cmdErr(k8sArgs, "error setting default gateway in cache: %v", err)

// Remove gateway if `default-route` network selection is specified
if deleteV4gateway || deleteV6gateway {
err = netutils.DeleteDefaultGW(args.Netns, ifName)
if err != nil {
return nil, cmdErr(k8sArgs, "error deleting default gateway: %v", err)
}
err = netutils.DeleteDefaultGWCache(n.CNIDir, rt, netName, ifName, deleteV4gateway, deleteV6gateway)
if err != nil {
return nil, cmdErr(k8sArgs, "error deleting default gateway in cache: %v", err)
}
}
}

// Master plugin result is always used if present
if delegate.MasterPlugin || result == nil {
result = tmpResult
// Here we'll set the default gateway which specified in `default-route` network selection
if adddefaultgateway {
err = netutils.SetDefaultGW(args.Netns, ifName, *delegate.GatewayRequest)
if err != nil {
return nil, cmdErr(k8sArgs, "error setting default gateway: %v", err)
}
err = netutils.AddDefaultGWCache(n.CNIDir, rt, netName, ifName, *delegate.GatewayRequest)
if err != nil {
return nil, cmdErr(k8sArgs, "error setting default gateway in cache: %v", err)
}
}
}

// Read devInfo from CNIDeviceInfoFile if it exists so
Expand Down
61 changes: 61 additions & 0 deletions pkg/multus/multus_cni020_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,67 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() {
Expect(fExec.delIndex).To(Equal(len(fExec.plugins)))
})

It("executes delegates (plugin without interface)", func() {
args := &skel.CmdArgs{
ContainerID: "123456789",
Netns: testNS.Path(),
IfName: "eth0",
StdinData: []byte(`{
"name": "node-cni-network",
"type": "multus",
"defaultnetworkfile": "/tmp/foo.multus.conf",
"defaultnetworkwaitseconds": 3,
"delegates": [{
"name": "weave1",
"cniVersion": "0.2.0",
"type": "weave-net"
},{
"name": "other1",
"cniVersion": "0.2.0",
"type": "other-plugin"
}]
}`),
}

logging.SetLogLevel("verbose")

fExec := newFakeExec()
expectedResult1 := &types020.Result{
CNIVersion: "0.2.0",
IP4: &types020.IPConfig{
IP: *testhelpers.EnsureCIDR("1.1.1.2/24"),
},
}
expectedConf1 := `{
"name": "weave1",
"cniVersion": "0.2.0",
"type": "weave-net"
}`
fExec.addPlugin020(nil, "eth0", expectedConf1, expectedResult1, nil)

// other1 just returns empty result
expectedResult2 := &types020.Result{
CNIVersion: "0.2.0",
}
expectedConf2 := `{
"name": "other1",
"cniVersion": "0.2.0",
"type": "other-plugin"
}`
fExec.addPlugin020(nil, "net1", expectedConf2, expectedResult2, nil)

result, err := CmdAdd(args, fExec, nil)
Expect(err).NotTo(HaveOccurred())
Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
r := result.(*types020.Result)
// plugin 1 is the masterplugin
Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())

err = CmdDel(args, fExec, nil)
Expect(err).NotTo(HaveOccurred())
Expect(fExec.delIndex).To(Equal(len(fExec.plugins)))
})

It("executes delegates given faulty namespace", func() {
args := &skel.CmdArgs{
ContainerID: "123456789",
Expand Down
61 changes: 61 additions & 0 deletions pkg/multus/multus_cni040_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,67 @@ var _ = Describe("multus operations cniVersion 0.3.1 config", func() {
Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())
})

It("executes delegates (plugin without interface)", func() {
args := &skel.CmdArgs{
ContainerID: "123456789",
Netns: testNS.Path(),
IfName: "eth0",
StdinData: []byte(`{
"name": "node-cni-network",
"type": "multus",
"defaultnetworkfile": "/tmp/foo.multus.conf",
"defaultnetworkwaitseconds": 3,
"delegates": [{
"name": "weave1",
"cniVersion": "0.3.1",
"type": "weave-net"
},{
"name": "other1",
"cniVersion": "0.3.1",
"type": "other-plugin"
}]
}`),
}

logging.SetLogLevel("verbose")

fExec := newFakeExec()
expectedResult1 := &cni040.Result{
CNIVersion: "0.3.1",
IPs: []*cni040.IPConfig{{
Address: *testhelpers.EnsureCIDR("1.1.1.2/24"),
}},
}
expectedConf1 := `{
"name": "weave1",
"cniVersion": "0.3.1",
"type": "weave-net"
}`
fExec.addPlugin040(nil, "eth0", expectedConf1, expectedResult1, nil)

// other1 just returns empty result
expectedResult2 := &cni040.Result{
CNIVersion: "0.3.1",
}
expectedConf2 := `{
"name": "other1",
"cniVersion": "0.3.1",
"type": "other-plugin"
}`
fExec.addPlugin040(nil, "net1", expectedConf2, expectedResult2, nil)

result, err := CmdAdd(args, fExec, nil)
Expect(err).NotTo(HaveOccurred())
Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
r := result.(*cni040.Result)
// plugin 1 is the masterplugin
Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue())

err = CmdDel(args, fExec, nil)
Expect(err).NotTo(HaveOccurred())
Expect(fExec.delIndex).To(Equal(len(fExec.plugins)))
})

It("fails when pod UID is provided and does not match Kube API pod UID", func() {
fakePod := testhelpers.NewFakePod("testpod", "net1", "")
net1 := `{
Expand Down
65 changes: 65 additions & 0 deletions pkg/multus/multus_cni100_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,71 @@ var _ = Describe("multus operations cniVersion 1.0.0 config", func() {
Expect(err).To(MatchError("[//:weave1]: error adding container to network \"weave1\": DelegateAdd: cannot set \"weave-net\" interface name to \"eth0\": validateIfName: no net namespace fsdadfad found: failed to Statfs \"fsdadfad\": no such file or directory"))
})

It("executes delegates (plugin without interface)", func() {
args := &skel.CmdArgs{
ContainerID: "123456789",
Netns: testNS.Path(),
IfName: "eth0",
StdinData: []byte(`{
"name": "node-cni-network",
"type": "multus",
"defaultnetworkfile": "/tmp/foo.multus.conf",
"defaultnetworkwaitseconds": 3,
"delegates": [{
"name": "weave1",
"cniVersion": "1.0.0",
"type": "weave-net"
},{
"name": "other1",
"cniVersion": "1.0.0",
"type": "other-plugin"
}]
}`),
}

logging.SetLogLevel("verbose")

fExec := newFakeExec()
expectedResult1 := &cni100.Result{
CNIVersion: "1.0.0",
IPs: []*cni100.IPConfig{{
Address: *testhelpers.EnsureCIDR("1.1.1.2/24"),
},
},
}
expectedConf1 := `{
"name": "weave1",
"cniVersion": "1.0.0",
"type": "weave-net"
}`
fExec.addPlugin100(nil, "eth0", expectedConf1, expectedResult1, nil)

// other1 just returns empty result
expectedResult2 := &cni100.Result{
CNIVersion: "0.4.0",
}
expectedConf2 := `{
"name": "other1",
"cniVersion": "1.0.0",
"type": "other-plugin"
}`
fExec.addPlugin100(nil, "net1", expectedConf2, expectedResult2, nil)

result, err := CmdAdd(args, fExec, nil)
Expect(err).NotTo(HaveOccurred())

Expect(fExec.addIndex).To(Equal(len(fExec.plugins)))
// plugin 1 is the masterplugin
Expect(reflect.DeepEqual(result, expectedResult1)).To(BeTrue())

err = CmdCheck(args, fExec, nil)
Expect(err).NotTo(HaveOccurred())

err = CmdDel(args, fExec, nil)
Expect(err).NotTo(HaveOccurred())
Expect(fExec.delIndex).To(Equal(len(fExec.plugins)))
})

It("returns the previous result using CmdCheck", func() {
args := &skel.CmdArgs{
ContainerID: "123456789",
Expand Down

0 comments on commit 2230480

Please sign in to comment.