Skip to content

Commit

Permalink
Merge pull request #1095 from s1061123/fix/plugin-without-interface
Browse files Browse the repository at this point in the history 
Fix multus to support CNI plugin which does not create interface
  • Loading branch information
@dougbtv
dougbtv committed May 18, 2023
2 parents 0c37bb0 + 2230480 commit f037656
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 f037656

Please sign in to comment.