From a2ea2996f31da8e9644c83a7f307021bbef442ce Mon Sep 17 00:00:00 2001 From: Alexey Perevalov Date: Mon, 29 Apr 2019 17:22:55 +0300 Subject: [PATCH] move to libcni 0.7.0 Previous commit "Use ip address from CNI output" introduces ability to run pod which can havn't eth0. But also it add problem: after kubelet restart, if we have already started pod w/o eth0, kubelet can't find proper interface (it's normal for vhostuser type of cni plugin when eth0 doesn't exist) and kubelet restarts "broken" pod. Fix of this issue requeres new feature of libcni - caching results. Looks like new libcni requires cniVersion in CNI output. This patch specifies version both for CNI conf and CNI output. Signed-off-by: Alexey Perevalov --- pkg/kubelet/dockershim/network/cni/cni.go | 5 +- .../dockershim/network/cni/cni_test.go | 17 ++++-- .../network/cni/testing/mock_cni.go | 57 ++++++++++++++----- .../network/kubenet/kubenet_linux.go | 5 +- .../network/kubenet/kubenet_linux_test.go | 4 +- 5 files changed, 63 insertions(+), 25 deletions(-) diff --git a/pkg/kubelet/dockershim/network/cni/cni.go b/pkg/kubelet/dockershim/network/cni/cni.go index c2f5b257346..f391ae68181 100644 --- a/pkg/kubelet/dockershim/network/cni/cni.go +++ b/pkg/kubelet/dockershim/network/cni/cni.go @@ -17,6 +17,7 @@ limitations under the License. package cni import ( + "context" "encoding/json" "errors" "fmt" @@ -326,7 +327,7 @@ func (plugin *cniNetworkPlugin) addToNetwork(network *cniNetwork, podName string pdesc := podDesc(podNamespace, podName, podSandboxID) netConf, cniNet := network.NetworkConfig, network.CNIConfig klog.V(4).Infof("Adding %s to network %s/%s netns %q", pdesc, netConf.Plugins[0].Network.Type, netConf.Name, podNetnsPath) - res, err := cniNet.AddNetworkList(netConf, rt) + res, err := cniNet.AddNetworkList(context.TODO(), netConf, rt) if err != nil { klog.Errorf("Error adding %s to network %s/%s: %v", pdesc, netConf.Plugins[0].Network.Type, netConf.Name, err) return nil, err @@ -345,7 +346,7 @@ func (plugin *cniNetworkPlugin) deleteFromNetwork(network *cniNetwork, podName s pdesc := podDesc(podNamespace, podName, podSandboxID) netConf, cniNet := network.NetworkConfig, network.CNIConfig klog.V(4).Infof("Deleting %s from network %s/%s netns %q", pdesc, netConf.Plugins[0].Network.Type, netConf.Name, podNetnsPath) - err = cniNet.DelNetworkList(netConf, rt) + err = cniNet.DelNetworkList(context.TODO(), netConf, rt) // The pod may not get deleted successfully at the first time. // Ignore "no such file or directory" error in case the network has already been deleted in previous attempts. if err != nil && !strings.Contains(err.Error(), "no such file or directory") { diff --git a/pkg/kubelet/dockershim/network/cni/cni_test.go b/pkg/kubelet/dockershim/network/cni/cni_test.go index 997bf973b7f..18593a3abe7 100644 --- a/pkg/kubelet/dockershim/network/cni/cni_test.go +++ b/pkg/kubelet/dockershim/network/cni/cni_test.go @@ -20,6 +20,7 @@ package cni import ( "bytes" + "context" "encoding/json" "fmt" "io/ioutil" @@ -48,7 +49,7 @@ import ( ) // Returns .in file path, .out file path, and .env file path -func installPluginUnderTest(t *testing.T, testBinDir, testConfDir, testDataDir, binName string, confName string) (string, string, string) { +func installPluginUnderTest(t *testing.T, testBinDir, testConfDir, testDataDir, binName string, confName, podIP string) (string, string, string) { for _, dir := range []string{testBinDir, testConfDir, testDataDir} { err := os.MkdirAll(dir, 0777) if err != nil { @@ -56,12 +57,14 @@ func installPluginUnderTest(t *testing.T, testBinDir, testConfDir, testDataDir, } } + const cniVersion = "0.2.0" + confFile := path.Join(testConfDir, confName+".conf") f, err := os.Create(confFile) if err != nil { t.Fatalf("Failed to install plugin %s: %v", confFile, err) } - networkConfig := fmt.Sprintf(`{ "name": "%s", "type": "%s", "capabilities": {"portMappings": true, "bandwidth": true, "ipRanges": true} }`, confName, binName) + networkConfig := fmt.Sprintf(`{ "cniVersion": "%s", "name": "%s", "type": "%s", "capabilities": {"portMappings": true, "bandwidth": true, "ipRanges": true} }`, cniVersion, confName, binName) _, err = f.WriteString(networkConfig) if err != nil { t.Fatalf("Failed to write network config file (%v)", err) @@ -78,8 +81,8 @@ echo "%@" >> {{.OutputEnv}} export $(echo ${CNI_ARGS} | sed 's/;/ /g') &> /dev/null mkdir -p {{.OutputDir}} &> /dev/null echo -n "$CNI_COMMAND $CNI_NETNS $K8S_POD_NAMESPACE $K8S_POD_NAME $K8S_POD_INFRA_CONTAINER_ID" >& {{.OutputFile}} -echo -n "{ \"ip4\": { \"ip\": \"10.1.0.23/24\" } }" -` +echo -n "{ \"cniVersion\": \"{{.CNIVersion}}\", \"ip4\": { \"ip\": \"{{.PodIP}}/24\" } }"` + inputFile := path.Join(testDataDir, binName+".in") outputFile := path.Join(testDataDir, binName+".out") envFile := path.Join(testDataDir, binName+".env") @@ -88,6 +91,8 @@ echo -n "{ \"ip4\": { \"ip\": \"10.1.0.23/24\" } }" "OutputFile": outputFile, "OutputEnv": envFile, "OutputDir": testDataDir, + "CNIVersion": cniVersion, + "PodIP": podIP, } tObj := template.Must(template.New("test").Parse(execScriptTempl)) @@ -190,7 +195,7 @@ func TestCNIPlugin(t *testing.T) { testBinDir := path.Join(tmpDir, "opt", "cni", "bin") testDataDir := path.Join(tmpDir, "output") defer tearDownPlugin(tmpDir) - inputFile, outputFile, outputEnv := installPluginUnderTest(t, testBinDir, testConfDir, testDataDir, binName, netName) + inputFile, outputFile, outputEnv := installPluginUnderTest(t, testBinDir, testConfDir, testDataDir, binName, netName, podIP) containerID := kubecontainer.ContainerID{Type: "test", ID: "test_infra_container"} pods := []*containertest.FakePod{{ @@ -217,7 +222,7 @@ func TestCNIPlugin(t *testing.T) { cniPlugin.execer = fexec cniPlugin.loNetwork.CNIConfig = mockLoCNI - mockLoCNI.On("AddNetworkList", cniPlugin.loNetwork.NetworkConfig, mock.AnythingOfType("*libcni.RuntimeConf")).Return(&types020.Result{IP4: &types020.IPConfig{IP: net.IPNet{IP: []byte{127, 0, 0, 1}}}}, nil) + mockLoCNI.On("AddNetworkList", context.TODO(), cniPlugin.loNetwork.NetworkConfig, mock.AnythingOfType("*libcni.RuntimeConf")).Return(&types020.Result{IP4: &types020.IPConfig{IP: net.IPNet{IP: []byte{127, 0, 0, 1}}}}, nil) // Check that status returns an error if err := cniPlugin.Status(); err == nil { diff --git a/pkg/kubelet/dockershim/network/cni/testing/mock_cni.go b/pkg/kubelet/dockershim/network/cni/testing/mock_cni.go index 1e5c0d00ee2..afc3a510f09 100644 --- a/pkg/kubelet/dockershim/network/cni/testing/mock_cni.go +++ b/pkg/kubelet/dockershim/network/cni/testing/mock_cni.go @@ -19,6 +19,7 @@ limitations under the License. package mock_cni import ( + "context" "github.com/containernetworking/cni/libcni" "github.com/containernetworking/cni/pkg/types" "github.com/stretchr/testify/mock" @@ -28,22 +29,52 @@ type MockCNI struct { mock.Mock } -func (m *MockCNI) AddNetwork(net *libcni.NetworkConfig, rt *libcni.RuntimeConf) (types.Result, error) { +func (m *MockCNI) AddNetwork(ctx context.Context, net *libcni.NetworkConfig, rt *libcni.RuntimeConf) (types.Result, error) { + args := m.Called(ctx, net, rt) + return args.Get(0).(types.Result), args.Error(1) +} + +func (m *MockCNI) DelNetwork(ctx context.Context, net *libcni.NetworkConfig, rt *libcni.RuntimeConf) error { + args := m.Called(ctx, net, rt) + return args.Error(0) +} + +func (m *MockCNI) DelNetworkList(ctx context.Context, net *libcni.NetworkConfigList, rt *libcni.RuntimeConf) error { + args := m.Called(ctx, net, rt) + return args.Error(0) +} + +func (m *MockCNI) GetNetworkListCachedResult(net *libcni.NetworkConfigList, rt *libcni.RuntimeConf) (types.Result, error) { args := m.Called(net, rt) return args.Get(0).(types.Result), args.Error(1) } -func (m *MockCNI) DelNetwork(net *libcni.NetworkConfig, rt *libcni.RuntimeConf) error { - args := m.Called(net, rt) - return args.Error(0) -} - -func (m *MockCNI) DelNetworkList(net *libcni.NetworkConfigList, rt *libcni.RuntimeConf) error { - args := m.Called(net, rt) - return args.Error(0) -} - -func (m *MockCNI) AddNetworkList(net *libcni.NetworkConfigList, rt *libcni.RuntimeConf) (types.Result, error) { - args := m.Called(net, rt) +func (m *MockCNI) AddNetworkList(ctx context.Context, net *libcni.NetworkConfigList, rt *libcni.RuntimeConf) (types.Result, error) { + args := m.Called(ctx, net, rt) return args.Get(0).(types.Result), args.Error(1) } + +func (m *MockCNI) CheckNetworkList(ctx context.Context, net *libcni.NetworkConfigList, rt *libcni.RuntimeConf) error { + args := m.Called(ctx, net, rt) + return args.Error(0) +} + +func (m *MockCNI) CheckNetwork(ctx context.Context, net *libcni.NetworkConfig, rt *libcni.RuntimeConf) error { + args := m.Called(ctx, net, rt) + return args.Error(0) +} + +func (m *MockCNI) GetNetworkCachedResult(net *libcni.NetworkConfig, rt *libcni.RuntimeConf) (types.Result, error) { + args := m.Called(net, rt) + return args.Get(0).(types.Result), args.Error(0) +} + +func (m *MockCNI) ValidateNetworkList(ctx context.Context, net *libcni.NetworkConfigList) ([]string, error) { + args := m.Called(ctx, net) + return args.Get(0).([]string), args.Error(0) +} + +func (m *MockCNI) ValidateNetwork(ctx context.Context, net *libcni.NetworkConfig) ([]string, error) { + args := m.Called(ctx, net) + return args.Get(0).([]string), args.Error(0) +} diff --git a/pkg/kubelet/dockershim/network/kubenet/kubenet_linux.go b/pkg/kubelet/dockershim/network/kubenet/kubenet_linux.go index acd47ee2fbd..d1b7a871532 100644 --- a/pkg/kubelet/dockershim/network/kubenet/kubenet_linux.go +++ b/pkg/kubelet/dockershim/network/kubenet/kubenet_linux.go @@ -19,6 +19,7 @@ limitations under the License. package kubenet import ( + "context" "fmt" "io/ioutil" "net" @@ -570,7 +571,7 @@ func (plugin *kubenetNetworkPlugin) addContainerToNetwork(config *libcni.Network // The network plugin can take up to 3 seconds to execute, // so yield the lock while it runs. plugin.mu.Unlock() - res, err := plugin.cniConfig.AddNetwork(config, rt) + res, err := plugin.cniConfig.AddNetwork(context.TODO(), config, rt) plugin.mu.Lock() if err != nil { return nil, fmt.Errorf("Error adding container to network: %v", err) @@ -585,7 +586,7 @@ func (plugin *kubenetNetworkPlugin) delContainerFromNetwork(config *libcni.Netwo } klog.V(3).Infof("Removing %s/%s from '%s' with CNI '%s' plugin and runtime: %+v", namespace, name, config.Network.Name, config.Network.Type, rt) - err = plugin.cniConfig.DelNetwork(config, rt) + err = plugin.cniConfig.DelNetwork(context.TODO(), config, rt) // The pod may not get deleted successfully at the first time. // Ignore "no such file or directory" error in case the network has already been deleted in previous attempts. if err != nil && !strings.Contains(err.Error(), "no such file or directory") { diff --git a/pkg/kubelet/dockershim/network/kubenet/kubenet_linux_test.go b/pkg/kubelet/dockershim/network/kubenet/kubenet_linux_test.go index 0c50149a98c..cc91b74f624 100644 --- a/pkg/kubelet/dockershim/network/kubenet/kubenet_linux_test.go +++ b/pkg/kubelet/dockershim/network/kubenet/kubenet_linux_test.go @@ -143,7 +143,7 @@ func TestTeardownCallsShaper(t *testing.T) { kubenet.bandwidthShaper = fshaper kubenet.hostportSyncer = hostporttest.NewFakeHostportSyncer() - mockcni.On("DelNetwork", mock.AnythingOfType("*libcni.NetworkConfig"), mock.AnythingOfType("*libcni.RuntimeConf")).Return(nil) + mockcni.On("DelNetwork", mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("*libcni.NetworkConfig"), mock.AnythingOfType("*libcni.RuntimeConf")).Return(nil) details := make(map[string]interface{}) details[network.NET_PLUGIN_EVENT_POD_CIDR_CHANGE_DETAIL_CIDR] = "10.0.0.1/24" @@ -247,7 +247,7 @@ func TestTearDownWithoutRuntime(t *testing.T) { existingContainerID := kubecontainer.BuildContainerID("docker", "123") kubenet.podIPs[existingContainerID] = tc.ip - mockcni.On("DelNetwork", mock.AnythingOfType("*libcni.NetworkConfig"), mock.AnythingOfType("*libcni.RuntimeConf")).Return(nil) + mockcni.On("DelNetwork", mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("*libcni.NetworkConfig"), mock.AnythingOfType("*libcni.RuntimeConf")).Return(nil) if err := kubenet.TearDownPod("namespace", "name", existingContainerID); err != nil { t.Fatalf("Unexpected error in TearDownPod: %v", err)