From 9afb6a46b22b509476f6d58d3dfbd7ffff3134d6 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 17 Mar 2022 13:53:46 +0100 Subject: [PATCH] libnetwork/cni: add support for arbitrary ipam plugins When we parse CNI config files to convert them into a native Network struct we should not error unsless there is something broken. The goal is to allow all cni configs to still function but podman inspect will not be able to show all informations such as subnets about this network. Because this is a valid use case we only log this at info level. Fixes containers/podman#12990 Fixes containers/podman#13124 Signed-off-by: Paul Holzinger --- libnetwork/cni/cni_conversion.go | 8 ++- libnetwork/cni/config_test.go | 40 +++++++++++++-- .../cni/testfiles/valid/ipam-empty.conflist | 34 +++++++++++++ .../cni/testfiles/valid/ipam-none.conflist | 33 +++++++++++++ .../cni/testfiles/valid/ipam-static.conflist | 49 +++++++++++++++++++ 5 files changed, 158 insertions(+), 6 deletions(-) create mode 100644 libnetwork/cni/testfiles/valid/ipam-empty.conflist create mode 100644 libnetwork/cni/testfiles/valid/ipam-none.conflist create mode 100644 libnetwork/cni/testfiles/valid/ipam-static.conflist diff --git a/libnetwork/cni/cni_conversion.go b/libnetwork/cni/cni_conversion.go index 5a87ebb33..127a5ddc1 100644 --- a/libnetwork/cni/cni_conversion.go +++ b/libnetwork/cni/cni_conversion.go @@ -133,7 +133,13 @@ func convertIPAMConfToNetwork(network *types.Network, ipam *ipamConfig, confPath } if ipam.PluginType != types.HostLocalIPAMDriver { - return errors.Errorf("unsupported ipam plugin %s in %s", ipam.PluginType, confPath) + // This is not an error. While we only support certain ipam drivers, we + // cannot make it fail for unsupported ones. CNI is still able to use them, + // just our translation logic cannot convert this into a Network. + // For the same reason this is not warning, it would just be annoying for + // everyone using a unknown ipam driver. + logrus.Infof("unsupported ipam plugin %q in %s", ipam.PluginType, confPath) + return nil } network.IPAMOptions[types.Driver] = types.HostLocalIPAMDriver diff --git a/libnetwork/cni/config_test.go b/libnetwork/cni/config_test.go index 2c90480ec..36fdbb0d1 100644 --- a/libnetwork/cni/config_test.go +++ b/libnetwork/cni/config_test.go @@ -34,6 +34,7 @@ var _ = Describe("Config", func() { } logBuffer = bytes.Buffer{} logrus.SetOutput(&logBuffer) + logrus.SetLevel(logrus.InfoLevel) }) JustBeforeEach(func() { @@ -45,6 +46,7 @@ var _ = Describe("Config", func() { }) AfterEach(func() { + logrus.SetLevel(logrus.InfoLevel) os.RemoveAll(cniConfDir) }) @@ -1071,6 +1073,8 @@ var _ = Describe("Config", func() { Context("network load valid existing ones", func() { + numberOfConfiFiles := 0 + BeforeEach(func() { dir := "testfiles/valid" files, err := ioutil.ReadDir(dir) @@ -1088,26 +1092,41 @@ var _ = Describe("Config", func() { Fail("Failed to copy test files") } } + numberOfConfiFiles = len(files) }) It("load networks from disk", func() { + logrus.SetLevel(logrus.WarnLevel) nets, err := libpodNet.NetworkList() Expect(err).To(BeNil()) - Expect(nets).To(HaveLen(9)) + Expect(nets).To(HaveLen(numberOfConfiFiles)) // test the we do not show logrus warnings/errors logString := logBuffer.String() Expect(logString).To(BeEmpty()) }) + It("load networks from disk with log level debug", func() { + logrus.SetLevel(logrus.DebugLevel) + nets, err := libpodNet.NetworkList() + Expect(err).To(BeNil()) + Expect(nets).To(HaveLen(numberOfConfiFiles)) + // check for the unsupported ipam plugin message + logString := logBuffer.String() + Expect(logString).ToNot(BeEmpty()) + Expect(logString).To(ContainSubstring("unsupported ipam plugin \\\"\\\" in %s", cniConfDir+"/ipam-none.conflist")) + Expect(logString).To(ContainSubstring("unsupported ipam plugin \\\"\\\" in %s", cniConfDir+"/ipam-empty.conflist")) + Expect(logString).To(ContainSubstring("unsupported ipam plugin \\\"static\\\" in %s", cniConfDir+"/ipam-static.conflist")) + }) + It("change network struct fields should not affect network struct in the backend", func() { nets, err := libpodNet.NetworkList() Expect(err).To(BeNil()) - Expect(nets).To(HaveLen(9)) + Expect(nets).To(HaveLen(numberOfConfiFiles)) nets[0].Name = "myname" nets, err = libpodNet.NetworkList() Expect(err).To(BeNil()) - Expect(nets).To(HaveLen(9)) + Expect(nets).To(HaveLen(numberOfConfiFiles)) Expect(nets).ToNot(ContainElement(HaveNetworkName("myname"))) network, err := libpodNet.NetworkInspect("bridge") @@ -1230,6 +1249,15 @@ var _ = Describe("Config", func() { )) }) + It("ipam static network", func() { + network, err := libpodNet.NetworkInspect("ipam-static") + Expect(err).To(BeNil()) + Expect(network.Name).To(Equal("ipam-static")) + Expect(network.ID).To(HaveLen(64)) + Expect(network.Driver).To(Equal("bridge")) + Expect(network.Subnets).To(HaveLen(0)) + }) + It("network list with filters (name)", func() { filters := map[string][]string{ "name": {"internal", "bridge"}, @@ -1304,10 +1332,12 @@ var _ = Describe("Config", func() { networks, err := libpodNet.NetworkList(filterFuncs...) Expect(err).To(BeNil()) - Expect(networks).To(HaveLen(9)) + Expect(networks).To(HaveLen(numberOfConfiFiles)) Expect(networks).To(ConsistOf(HaveNetworkName("internal"), HaveNetworkName("bridge"), HaveNetworkName("mtu"), HaveNetworkName("vlan"), HaveNetworkName("podman"), - HaveNetworkName("label"), HaveNetworkName("macvlan"), HaveNetworkName("macvlan_mtu"), HaveNetworkName("dualstack"))) + HaveNetworkName("label"), HaveNetworkName("macvlan"), HaveNetworkName("macvlan_mtu"), + HaveNetworkName("dualstack"), HaveNetworkName("ipam-none"), HaveNetworkName("ipam-empty"), + HaveNetworkName("ipam-static"))) }) It("network list with filters (label)", func() { diff --git a/libnetwork/cni/testfiles/valid/ipam-empty.conflist b/libnetwork/cni/testfiles/valid/ipam-empty.conflist new file mode 100644 index 000000000..4383cc0ec --- /dev/null +++ b/libnetwork/cni/testfiles/valid/ipam-empty.conflist @@ -0,0 +1,34 @@ +{ + "cniVersion": "0.4.0", + "name": "ipam-empty", + "plugins": [ + { + "type": "bridge", + "bridge": "cni-podman124", + "isGateway": true, + "ipMasq": true, + "hairpinMode": true, + "ipam": {} + }, + { + "type": "portmap", + "capabilities": { + "portMappings": true + } + }, + { + "type": "firewall", + "backend": "" + }, + { + "type": "tuning" + }, + { + "type": "dnsname", + "domainName": "dns.podman", + "capabilities": { + "aliases": true + } + } + ] +} diff --git a/libnetwork/cni/testfiles/valid/ipam-none.conflist b/libnetwork/cni/testfiles/valid/ipam-none.conflist new file mode 100644 index 000000000..90cecbca9 --- /dev/null +++ b/libnetwork/cni/testfiles/valid/ipam-none.conflist @@ -0,0 +1,33 @@ +{ + "cniVersion": "0.4.0", + "name": "ipam-none", + "plugins": [ + { + "type": "bridge", + "bridge": "cni-podman123", + "isGateway": true, + "ipMasq": true, + "hairpinMode": true + }, + { + "type": "portmap", + "capabilities": { + "portMappings": true + } + }, + { + "type": "firewall", + "backend": "" + }, + { + "type": "tuning" + }, + { + "type": "dnsname", + "domainName": "dns.podman", + "capabilities": { + "aliases": true + } + } + ] +} diff --git a/libnetwork/cni/testfiles/valid/ipam-static.conflist b/libnetwork/cni/testfiles/valid/ipam-static.conflist new file mode 100644 index 000000000..b9e70f4c7 --- /dev/null +++ b/libnetwork/cni/testfiles/valid/ipam-static.conflist @@ -0,0 +1,49 @@ +{ + "cniVersion": "0.4.0", + "name": "ipam-static", + "plugins": [ + { + "type": "bridge", + "bridge": "cni-podman125", + "isGateway": true, + "ipMasq": true, + "hairpinMode": true, + "ipam": { + "type": "static", + "routes": [ + { + "dst": "0.0.0.0/0" + } + ], + "addresses": [ + [ + { + "subnet": "10.89.0.89/16", + "gateway": "10.89.0.1" + } + ] + ] + } + }, + { + "type": "portmap", + "capabilities": { + "portMappings": true + } + }, + { + "type": "firewall", + "backend": "" + }, + { + "type": "tuning" + }, + { + "type": "dnsname", + "domainName": "dns.podman", + "capabilities": { + "aliases": true + } + } + ] +}