From 263b0b99d0e6b76ad597f7c192bdb02775d65662 Mon Sep 17 00:00:00 2001 From: Abhinandan Prativadi Date: Tue, 19 Jun 2018 18:20:14 +0000 Subject: [PATCH 1/2] Change to keep in sync with latest cni config This commit contains change to pick the latest cni config from the configured CNIConfDir. With this change any changes made to the cni config file will be picked up on the kubelet's runtime status check call. Ofcourse this would lead to undefined behavior when the cni config change is made in parallel during pod creation. However its reasonable to assume that the operator is aware of the need to drain the nodes of pods before making cni configuration change. The behavior is currently not defined in kubernetes. However I see that similar approach being adopted in the upstream kubernetes with dockershim. Keeping the behavior consistent for now. Signed-off-by: Abhinandan Prativadi --- pkg/server/service.go | 2 +- pkg/server/status.go | 15 +++++++++------ pkg/server/testing/fake_cni_plugin.go | 2 +- pkg/server/update_runtime_config.go | 2 +- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/pkg/server/service.go b/pkg/server/service.go index 86ec5bb73..89074705a 100644 --- a/pkg/server/service.go +++ b/pkg/server/service.go @@ -144,7 +144,7 @@ func NewCRIService(config criconfig.Config, client *containerd.Client) (CRIServi // Try to load the config if it exists. Just log the error if load fails // This is not disruptive for containerd to panic - if err := c.netPlugin.Load(cni.WithLoNetwork(), cni.WithDefaultConf()); err != nil { + if err := c.netPlugin.Load(cni.WithLoNetwork, cni.WithDefaultConf); err != nil { logrus.WithError(err).Error("Failed to load cni during init, please check CRI plugin status before setting up network for pods") } // prepare streaming server diff --git a/pkg/server/status.go b/pkg/server/status.go index 059036239..f71b78196 100644 --- a/pkg/server/status.go +++ b/pkg/server/status.go @@ -22,6 +22,7 @@ import ( goruntime "runtime" cni "github.com/containerd/go-cni" + "github.com/sirupsen/logrus" "golang.org/x/net/context" runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" ) @@ -41,14 +42,16 @@ func (c *criService) Status(ctx context.Context, r *runtime.StatusRequest) (*run Type: runtime.NetworkReady, Status: true, } + + // Load the latest cni configuration to be in sync with the latest network configuration + if err := c.netPlugin.Load(cni.WithLoNetwork, cni.WithDefaultConf); err != nil { + logrus.WithError(err).Errorf("Failed to load cni configuration") + } // Check the status of the cni initialization if err := c.netPlugin.Status(); err != nil { - // If it is not initialized, then load the config and retry - if err = c.netPlugin.Load(cni.WithLoNetwork(), cni.WithDefaultConf()); err != nil { - networkCondition.Status = false - networkCondition.Reason = networkNotReadyReason - networkCondition.Message = fmt.Sprintf("Network plugin returns error: %v", err) - } + networkCondition.Status = false + networkCondition.Reason = networkNotReadyReason + networkCondition.Message = fmt.Sprintf("Network plugin returns error: %v", err) } resp := &runtime.StatusResponse{ diff --git a/pkg/server/testing/fake_cni_plugin.go b/pkg/server/testing/fake_cni_plugin.go index b88896fc9..4a7e8d602 100644 --- a/pkg/server/testing/fake_cni_plugin.go +++ b/pkg/server/testing/fake_cni_plugin.go @@ -47,6 +47,6 @@ func (f *FakeCNIPlugin) Status() error { } // Load loads the network config. -func (f *FakeCNIPlugin) Load(opts ...cni.LoadOption) error { +func (f *FakeCNIPlugin) Load(opts ...cni.CNIOpt) error { return f.LoadErr } diff --git a/pkg/server/update_runtime_config.go b/pkg/server/update_runtime_config.go index 8682da66d..0f55aedf8 100644 --- a/pkg/server/update_runtime_config.go +++ b/pkg/server/update_runtime_config.go @@ -52,7 +52,7 @@ func (c *criService) UpdateRuntimeConfig(ctx context.Context, r *runtime.UpdateR if err := c.netPlugin.Status(); err == nil { logrus.Infof("Network plugin is ready, skip generating cni config from template %q", confTemplate) return &runtime.UpdateRuntimeConfigResponse{}, nil - } else if err := c.netPlugin.Load(cni.WithLoNetwork(), cni.WithDefaultConf()); err == nil { + } else if err := c.netPlugin.Load(cni.WithLoNetwork, cni.WithDefaultConf); err == nil { logrus.Infof("CNI config is successfully loaded, skip generating cni config from template %q", confTemplate) return &runtime.UpdateRuntimeConfigResponse{}, nil } From 860971025f1ad628f773b405446776f0e074cf68 Mon Sep 17 00:00:00 2001 From: Abhinandan Prativadi Date: Thu, 21 Jun 2018 18:59:00 +0000 Subject: [PATCH 2/2] vendoring latest go-cni with fixes Signed-off-by: Abhinandan Prativadi --- vendor.conf | 2 +- vendor/github.com/containerd/go-cni/README.md | 25 +-- vendor/github.com/containerd/go-cni/cni.go | 42 +++-- vendor/github.com/containerd/go-cni/opts.go | 163 ++++++++---------- 4 files changed, 110 insertions(+), 122 deletions(-) diff --git a/vendor.conf b/vendor.conf index df672d7b7..5a27037fc 100644 --- a/vendor.conf +++ b/vendor.conf @@ -7,7 +7,7 @@ github.com/containerd/console cb7008ab3d8359b78c5f464cb7cf160107ad5925 github.com/containerd/containerd 84bebdd91d347c99069d1705b7d4e6d6f746160c github.com/containerd/continuity d3c23511c1bf5851696cba83143d9cbcd666869b github.com/containerd/fifo 3d5202aec260678c48179c56f40e6f38a095738c -github.com/containerd/go-cni f2d7272f12d045b16ed924f50e91f9f9cecc55a7 +github.com/containerd/go-cni 5882530828ecf62032409b298a3e8b19e08b6534 github.com/containerd/go-runc f271fa2021de855d4d918dbef83c5fe19db1bdd5 github.com/containerd/typeurl f6943554a7e7e88b3c14aad190bf05932da84788 github.com/containernetworking/cni v0.6.0 diff --git a/vendor/github.com/containerd/go-cni/README.md b/vendor/github.com/containerd/go-cni/README.md index 900b42440..c0856aebc 100644 --- a/vendor/github.com/containerd/go-cni/README.md +++ b/vendor/github.com/containerd/go-cni/README.md @@ -1,7 +1,10 @@ +[![Build Status](https://travis-ci.org/containerd/go-cni.svg?branch=master)](https://travis-ci.org/containerd/go-cni) + # go-cni A generic CNI library to provide APIs for CNI plugin interactions. The library provides APIs to: +- Load CNI network config from different sources - Setup networks for container namespace - Remove networks from container namespace - Query status of CNI network plugin initialization @@ -16,11 +19,17 @@ func main() { defaultIfName := "eth0" // Initialize library l = gocni.New(gocni.WithMinNetworkCount(2), - gocni.WithLoNetwork(), gocni.WithPluginConfDir("/etc/mycni/net.d"), gocni.WithPluginDir([]string{"/opt/mycni/bin", "/opt/cni/bin"}), gocni.WithDefaultIfName(defaultIfName)) - + + // Load the cni configuration + err:= l.Load(gocni.WithLoNetwork,gocni.WithDefaultConf) + if err != nil{ + log.Errorf("failed to load cni configuration: %v", err) + return + } + // Setup network for namespace. labels := map[string]string{ "K8S_POD_NAMESPACE": "namespace1", @@ -29,16 +38,10 @@ func main() { } result, err := l.Setup(id, netns, gocni.WithLabels(labels)) if err != nil { - return nil, fmt.Errorf("failed to setup network for namespace %q: %v", id, err) + log.Errorf("failed to setup network for namespace %q: %v",id, err) + return } - defer func() { - if retErr != nil { - // Teardown network if an error is returned. - if err := l.Remove(id, netns, gocni.WithLabels(labels)); err != nil { - fmt.Errorf("Failed to destroy network for namespace %q", id) - } - } - }() + // Get IP of the default interface IP := result.Interfaces[defaultIfName].IPConfigs[0].IP.String() fmt.Printf("IP of the default interface %s:%s", defaultIfName, IP) diff --git a/vendor/github.com/containerd/go-cni/cni.go b/vendor/github.com/containerd/go-cni/cni.go index 89f0dbc04..3993395fc 100644 --- a/vendor/github.com/containerd/go-cni/cni.go +++ b/vendor/github.com/containerd/go-cni/cni.go @@ -31,7 +31,7 @@ type CNI interface { // Remove tears down the network of the namespace. Remove(id string, path string, opts ...NamespaceOpts) error // Load loads the cni network config - Load(opts ...LoadOption) error + Load(opts ...CNIOpt) error // Status checks the status of the cni initialization Status() error } @@ -59,7 +59,7 @@ func defaultCNIConfig() *libcni { } } -func New(config ...ConfigOption) (CNI, error) { +func New(config ...CNIOpt) (CNI, error) { cni := defaultCNIConfig() var err error for _, c := range config { @@ -70,8 +70,10 @@ func New(config ...ConfigOption) (CNI, error) { return cni, nil } -func (c *libcni) Load(opts ...LoadOption) error { +func (c *libcni) Load(opts ...CNIOpt) error { var err error + c.Lock() + defer c.Unlock() // Reset the networks on a load operation to ensure // config happens on a clean slate c.reset() @@ -81,30 +83,27 @@ func (c *libcni) Load(opts ...LoadOption) error { return errors.Wrapf(ErrLoad, fmt.Sprintf("cni config load failed: %v", err)) } } - return c.Status() + return nil } func (c *libcni) Status() error { c.RLock() defer c.RUnlock() - if len(c.networks) < c.networkCount { - return ErrCNINotInitialized - } - return nil + return c.status() } // Setup setups the network in the namespace func (c *libcni) Setup(id string, path string, opts ...NamespaceOpts) (*CNIResult, error) { - if err:=c.Status();err!=nil{ - return nil,err + c.RLock() + defer c.RUnlock() + if err := c.status(); err != nil { + return nil, err } ns, err := newNamespace(id, path, opts...) if err != nil { return nil, err } var results []*current.Result - c.RLock() - defer c.RUnlock() for _, network := range c.networks { r, err := network.Attach(ns) if err != nil { @@ -117,15 +116,15 @@ func (c *libcni) Setup(id string, path string, opts ...NamespaceOpts) (*CNIResul // Remove removes the network config from the namespace func (c *libcni) Remove(id string, path string, opts ...NamespaceOpts) error { - if err:=c.Status();err!=nil{ - return err - } + c.RLock() + defer c.RUnlock() + if err := c.status(); err != nil { + return err + } ns, err := newNamespace(id, path, opts...) if err != nil { return err } - c.RLock() - defer c.RUnlock() for _, network := range c.networks { if err := network.Remove(ns); err != nil { return err @@ -135,7 +134,12 @@ func (c *libcni) Remove(id string, path string, opts ...NamespaceOpts) error { } func (c *libcni) reset() { - c.Lock() - defer c.Unlock() c.networks = nil } + +func (c *libcni) status() error { + if len(c.networks) < c.networkCount { + return ErrCNINotInitialized + } + return nil +} diff --git a/vendor/github.com/containerd/go-cni/opts.go b/vendor/github.com/containerd/go-cni/opts.go index ada1daa0c..5ad33209f 100644 --- a/vendor/github.com/containerd/go-cni/opts.go +++ b/vendor/github.com/containerd/go-cni/opts.go @@ -24,11 +24,11 @@ import ( "github.com/pkg/errors" ) -type ConfigOption func(c *libcni) error +type CNIOpt func(c *libcni) error // WithInterfacePrefix sets the prefix for network interfaces // e.g. eth or wlan -func WithInterfacePrefix(prefix string) ConfigOption { +func WithInterfacePrefix(prefix string) CNIOpt { return func(c *libcni) error { c.prefix = prefix return nil @@ -37,7 +37,7 @@ func WithInterfacePrefix(prefix string) ConfigOption { // WithPluginDir can be used to set the locations of // the cni plugin binaries -func WithPluginDir(dirs []string) ConfigOption { +func WithPluginDir(dirs []string) CNIOpt { return func(c *libcni) error { c.pluginDirs = dirs c.cniConfig = &cnilibrary.CNIConfig{Path: dirs} @@ -47,7 +47,7 @@ func WithPluginDir(dirs []string) ConfigOption { // WithPluginConfDir can be used to configure the // cni configuration directory. -func WithPluginConfDir(dir string) ConfigOption { +func WithPluginConfDir(dir string) CNIOpt { return func(c *libcni) error { c.pluginConfDir = dir return nil @@ -57,23 +57,17 @@ func WithPluginConfDir(dir string) ConfigOption { // WithMinNetworkCount can be used to configure the // minimum networks to be configured and initalized // for the status to report success. By default its 1. -func WithMinNetworkCount(count int) ConfigOption { +func WithMinNetworkCount(count int) CNIOpt { return func(c *libcni) error { c.networkCount = count return nil } } -// LoadOption can be used with Load API -// to load network configuration from different -// sources. -type LoadOption func(c *libcni) error - // WithLoNetwork can be used to load the loopback // network config. -func WithLoNetwork() LoadOption { - return func(c *libcni) error { - loConfig, _ := cnilibrary.ConfListFromBytes([]byte(`{ +func WithLoNetwork(c *libcni) error { + loConfig, _ := cnilibrary.ConfListFromBytes([]byte(`{ "cniVersion": "0.3.1", "name": "cni-loopback", "plugins": [{ @@ -81,20 +75,17 @@ func WithLoNetwork() LoadOption { }] }`)) - c.Lock() - defer c.Unlock() - c.networks = append(c.networks,&Network{ - cni: c.cniConfig, - config: loConfig, - ifName: "lo", - }) - return nil - } + c.networks = append(c.networks, &Network{ + cni: c.cniConfig, + config: loConfig, + ifName: "lo", + }) + return nil } // WithConf can be used to load config directly // from byte. -func WithConf(bytes []byte) LoadOption { +func WithConf(bytes []byte) CNIOpt { return func(c *libcni) error { conf, err := cnilibrary.ConfFromBytes(bytes) if err != nil { @@ -104,8 +95,6 @@ func WithConf(bytes []byte) LoadOption { if err != nil { return err } - c.Lock() - defer c.Unlock() c.networks = append(c.networks, &Network{ cni: c.cniConfig, config: confList, @@ -118,7 +107,7 @@ func WithConf(bytes []byte) LoadOption { // WithConfFile can be used to load network config // from an .conf file. Supported with absolute fileName // with path only. -func WithConfFile(fileName string) LoadOption { +func WithConfFile(fileName string) CNIOpt { return func(c *libcni) error { conf, err := cnilibrary.ConfFromFile(fileName) if err != nil { @@ -129,8 +118,6 @@ func WithConfFile(fileName string) LoadOption { if err != nil { return err } - c.Lock() - defer c.Unlock() c.networks = append(c.networks, &Network{ cni: c.cniConfig, config: confList, @@ -143,15 +130,13 @@ func WithConfFile(fileName string) LoadOption { // WithConfListFile can be used to load network config // from an .conflist file. Supported with absolute fileName // with path only. -func WithConfListFile(fileName string) LoadOption { +func WithConfListFile(fileName string) CNIOpt { return func(c *libcni) error { confList, err := cnilibrary.ConfListFromFile(fileName) if err != nil { return err } - c.Lock() - defer c.Unlock() - c.networks = append(c.networks,&Network{ + c.networks = append(c.networks, &Network{ cni: c.cniConfig, config: confList, ifName: getIfName(c.prefix, 0), @@ -163,64 +148,60 @@ func WithConfListFile(fileName string) LoadOption { // WithDefaultConf can be used to detect network config // files from the configured cni config directory and load // them. -func WithDefaultConf() LoadOption { - return func(c *libcni) error { - files, err := cnilibrary.ConfFiles(c.pluginConfDir, []string{".conf", ".conflist", ".json"}) - switch { - case err != nil: - return errors.Wrapf(ErrRead, "failed to read config file: %v", err) - case len(files) == 0: - return errors.Wrapf(ErrCNINotInitialized, "no network config found in %s", c.pluginConfDir) - } - - // files contains the network config files associated with cni network. - // Use lexicographical way as a defined order for network config files. - sort.Strings(files) - // Since the CNI spec does not specify a way to detect default networks, - // the convention chosen is - the first network configuration in the sorted - // list of network conf files as the default network and choose the default - // interface provided during init as the network interface for this default - // network. For every other network use a generated interface id. - i := 0 - c.Lock() - defer c.Unlock() - for _, confFile := range files { - var confList *cnilibrary.NetworkConfigList - if strings.HasSuffix(confFile, ".conflist") { - confList, err = cnilibrary.ConfListFromFile(confFile) - if err != nil { - return errors.Wrapf(ErrInvalidConfig, "failed to load CNI config list file %s: %v", confFile, err) - } - } else { - conf, err := cnilibrary.ConfFromFile(confFile) - if err != nil { - return errors.Wrapf(ErrInvalidConfig, "failed to load CNI config file %s: %v", confFile, err) - } - // Ensure the config has a "type" so we know what plugin to run. - // Also catches the case where somebody put a conflist into a conf file. - if conf.Network.Type == "" { - return errors.Wrapf(ErrInvalidConfig, "network type not found in %s", confFile) - } - - confList, err = cnilibrary.ConfListFromConf(conf) - if err != nil { - return errors.Wrapf(ErrInvalidConfig, "failed to convert CNI config file %s to list: %v", confFile, err) - } - } - if len(confList.Plugins) == 0 { - return errors.Wrapf(ErrInvalidConfig, "CNI config list %s has no networks, skipping", confFile) - - } - c.networks = append(c.networks, &Network{ - cni: c.cniConfig, - config: confList, - ifName: getIfName(c.prefix, i), - }) - i++ - } - if len(c.networks) == 0 { - return errors.Wrapf(ErrCNINotInitialized, "no valid networks found in %s", c.pluginDirs) - } - return nil +func WithDefaultConf(c *libcni) error { + files, err := cnilibrary.ConfFiles(c.pluginConfDir, []string{".conf", ".conflist", ".json"}) + switch { + case err != nil: + return errors.Wrapf(ErrRead, "failed to read config file: %v", err) + case len(files) == 0: + return errors.Wrapf(ErrCNINotInitialized, "no network config found in %s", c.pluginConfDir) } + + // files contains the network config files associated with cni network. + // Use lexicographical way as a defined order for network config files. + sort.Strings(files) + // Since the CNI spec does not specify a way to detect default networks, + // the convention chosen is - the first network configuration in the sorted + // list of network conf files as the default network and choose the default + // interface provided during init as the network interface for this default + // network. For every other network use a generated interface id. + i := 0 + for _, confFile := range files { + var confList *cnilibrary.NetworkConfigList + if strings.HasSuffix(confFile, ".conflist") { + confList, err = cnilibrary.ConfListFromFile(confFile) + if err != nil { + return errors.Wrapf(ErrInvalidConfig, "failed to load CNI config list file %s: %v", confFile, err) + } + } else { + conf, err := cnilibrary.ConfFromFile(confFile) + if err != nil { + return errors.Wrapf(ErrInvalidConfig, "failed to load CNI config file %s: %v", confFile, err) + } + // Ensure the config has a "type" so we know what plugin to run. + // Also catches the case where somebody put a conflist into a conf file. + if conf.Network.Type == "" { + return errors.Wrapf(ErrInvalidConfig, "network type not found in %s", confFile) + } + + confList, err = cnilibrary.ConfListFromConf(conf) + if err != nil { + return errors.Wrapf(ErrInvalidConfig, "failed to convert CNI config file %s to list: %v", confFile, err) + } + } + if len(confList.Plugins) == 0 { + return errors.Wrapf(ErrInvalidConfig, "CNI config list %s has no networks, skipping", confFile) + + } + c.networks = append(c.networks, &Network{ + cni: c.cniConfig, + config: confList, + ifName: getIfName(c.prefix, i), + }) + i++ + } + if len(c.networks) == 0 { + return errors.Wrapf(ErrCNINotInitialized, "no valid networks found in %s", c.pluginDirs) + } + return nil }