Merge pull request #239 from steveeJ/master

MAINTAINERS: remove @zachgersh, add @dcbw

CONTRIBUTING.md

@@ -1,11 +1,11 @@
 # How to Contribute
 
-cni is [Apache 2.0 licensed](LICENSE) and accepts contributions via GitHub
+CNI is [Apache 2.0 licensed](LICENSE) and accepts contributions via GitHub
 pull requests. This document outlines some of the conventions on development
 workflow, commit message formatting, contact points and other resources to make
 it easier to get your contribution accepted.
 
-We welcome improvements to documentation as well as to code.
+We gratefully welcome improvements to documentation as well as to code.
 
 # Certificate of Origin
 
@@ -41,7 +41,19 @@ This is a rough outline of how to prepare a contribution:
 - Make sure any new code files have a license header.
 - Submit a pull request to the original repository.
 
-Thanks for your contributions!
+# Acceptance policy
+
+These things will make a PR more likely to be accepted:
+
+ * a well-described requirement
+ * tests for new code
+ * tests for old code!
+ * new code follows the conventions in old code
+ * a good commit message (see below)
+
+In general, we will merge a PR once two maintainers have endorsed it.
+Trivial changes (e.g., corrections to spelling) may get waved through.
+For substantial changes, more people may become involved, and you might get asked to resubmit the PR or divide the changes into more than one PR.
 
 ### Format of the Commit Message
 

MAINTAINERS

@@ -1,5 +1,5 @@
+Dan Williams <dcbw@redhat.com> (@dcbw)
 Gabe Rosenhouse <grosenhouse@pivotal.io> (@rosenhouse)
 Michael Bridgen <michael@weave.works> (@squaremo)
 Stefan Junker <stefan.junker@coreos.com> (@steveeJ)
 Tom Denham <tom.denham@metaswitch.com> (@tomdee)
-Zach Gershman <zachgersh@gmail.com> (@zachgersh)

README.md

@@ -30,6 +30,7 @@ To avoid duplication, we think it is prudent to define a common interface betwee
 - [Weave - a multi-host Docker network](https://github.com/weaveworks/weave)
 - [Project Calico - a layer 3 virtual network](https://github.com/projectcalico/calico-cni)
 - [Contiv Networking - policy networking for various use cases](https://github.com/contiv/netplugin)
+- [Mesos - a distributed systems kernel](https://github.com/apache/mesos/blob/master/docs/cni.md)
 
 ## Contributing to CNI
 

ROADMAP.md

@@ -0,0 +1,33 @@
+# CNI Roadmap
+
+This document defines a high level roadmap for CNI development.
+The list below is not complete, and we advise to get the current project state from the [milestones defined in GitHub](https://github.com/containernetworking/cni/milestones).
+
+## CNI Milestones
+
+### [v0.2.0](https://github.com/containernetworking/cni/milestones/v0.2.0)
+
+* Signed release binaries
+* Introduction of a testing strategy/framework
+
+### [v0.3.0](https://github.com/containernetworking/cni/milestones/v0.3.0)
+
+* Further increase test coverage
+* Simpler default route handling in bridge plugin
+* Clarify project description, documentation and contribution guidelines
+
+### [v0.4.0](https://github.com/containernetworking/cni/milestones/v0.4.0)
+
+* Further increase test coverage
+* Simpler bridging of host interface
+* Improve IPAM allocator predictability
+* Allow in- and output of arbitrary K/V pairs for plugins
+
+### [v1.0.0](https://github.com/containernetworking/cni/milestones/v1.0.0)
+
+- Plugin composition functionality
+- IPv6 support
+- Stable SPEC
+- Strategy and tooling for backwards compatibility
+- Complete test coverage
+- Integrate build artefact generation with CI

pkg/ns/ns.go

@@ -20,6 +20,7 @@ import (
 	"os"
 	"path"
 	"runtime"
+	"strings"
 	"sync"
 	"syscall"
 
@@ -74,38 +75,66 @@ func GetCurrentNS() (NetNS, error) {
 	return GetNS(getCurrentThreadNetNSPath())
 }
 
-// Returns an object representing the namespace referred to by @path
-func GetNS(nspath string) (NetNS, error) {
-	fd, err := os.Open(nspath)
-	if err != nil {
-		return nil, fmt.Errorf("Failed to open %v: %v", nspath, err)
+const (
+	// https://github.com/torvalds/linux/blob/master/include/uapi/linux/magic.h
+	NSFS_MAGIC   = 0x6e736673
+	PROCFS_MAGIC = 0x9fa0
+)
+
+type NSPathNotExistErr struct{ msg string }
+
+func (e NSPathNotExistErr) Error() string { return e.msg }
+
+type NSPathNotNSErr struct{ msg string }
+
+func (e NSPathNotNSErr) Error() string { return e.msg }
+
+func IsNSorErr(nspath string) error {
+	stat := syscall.Statfs_t{}
+	if err := syscall.Statfs(nspath, &stat); err != nil {
+		if os.IsNotExist(err) {
+			err = NSPathNotExistErr{msg: fmt.Sprintf("failed to Statfs %q: %v", nspath, err)}
+		} else {
+			err = fmt.Errorf("failed to Statfs %q: %v", nspath, err)
+		}
+		return err
 	}
 
-	isNSFS, err := IsNSFS(nspath)
+	switch stat.Type {
+	case PROCFS_MAGIC:
+		// Kernel < 3.19
+
+		validPathContent := "ns/"
+		validName := strings.Contains(nspath, validPathContent)
+		if !validName {
+			return NSPathNotNSErr{msg: fmt.Sprintf("path %q doesn't contain %q", nspath, validPathContent)}
+		}
+
+		return nil
+	case NSFS_MAGIC:
+		// Kernel >= 3.19
+
+		return nil
+	default:
+		return NSPathNotNSErr{msg: fmt.Sprintf("unknown FS magic on %q: %x", nspath, stat.Type)}
+	}
+}
+
+// Returns an object representing the namespace referred to by @path
+func GetNS(nspath string) (NetNS, error) {
+	err := IsNSorErr(nspath)
 	if err != nil {
-		fd.Close()
 		return nil, err
 	}
-	if !isNSFS {
-		fd.Close()
-		return nil, fmt.Errorf("%v is not of type NSFS", nspath)
+
+	fd, err := os.Open(nspath)
+	if err != nil {
+		return nil, err
 	}
 
 	return &netNS{file: fd}, nil
 }
 
-// Returns whether or not the nspath argument points to a network namespace
-func IsNSFS(nspath string) (bool, error) {
-	const NSFS_MAGIC = 0x6e736673
-
-	stat := syscall.Statfs_t{}
-	if err := syscall.Statfs(nspath, &stat); err != nil {
-		return false, fmt.Errorf("failed to Statfs %q: %v", nspath, err)
-	}
-
-	return stat.Type == NSFS_MAGIC, nil
-}
-
 // Creates a new persistent network namespace and returns an object
 // representing that namespace, without switching to it
 func NewNS() (NetNS, error) {

pkg/ns/ns_test.go

@@ -180,7 +180,9 @@ var _ = Describe("Linux namespace operations", func() {
 				defer os.Remove(nspath)
 
 				_, err = ns.GetNS(nspath)
-				Expect(err).To(MatchError(fmt.Sprintf("%v is not of type NSFS", nspath)))
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(BeAssignableToTypeOf(ns.NSPathNotNSErr{}))
+				Expect(err).NotTo(BeAssignableToTypeOf(ns.NSPathNotExistErr{}))
 			})
 		})
 
@@ -212,12 +214,11 @@ var _ = Describe("Linux namespace operations", func() {
 		})
 	})
 
-	Describe("IsNSFS", func() {
+	Describe("IsNSorErr", func() {
 		It("should detect a namespace", func() {
 			createdNetNS, err := ns.NewNS()
-			isNSFS, err := ns.IsNSFS(createdNetNS.Path())
+			err = ns.IsNSorErr(createdNetNS.Path())
 			Expect(err).NotTo(HaveOccurred())
-			Expect(isNSFS).To(Equal(true))
 		})
 
 		It("should refuse other paths", func() {
@@ -228,9 +229,17 @@ var _ = Describe("Linux namespace operations", func() {
 			nspath := tempFile.Name()
 			defer os.Remove(nspath)
 
-			isNSFS, err := ns.IsNSFS(nspath)
-			Expect(err).NotTo(HaveOccurred())
-			Expect(isNSFS).To(Equal(false))
+			err = ns.IsNSorErr(nspath)
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(BeAssignableToTypeOf(ns.NSPathNotNSErr{}))
+			Expect(err).NotTo(BeAssignableToTypeOf(ns.NSPathNotExistErr{}))
+		})
+
+		It("should error on non-existing paths", func() {
+			err := ns.IsNSorErr("/tmp/IDoNotExist")
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(BeAssignableToTypeOf(ns.NSPathNotExistErr{}))
+			Expect(err).NotTo(BeAssignableToTypeOf(ns.NSPathNotNSErr{}))
 		})
 	})
 })

pkg/skel/skel.go

@@ -36,28 +36,72 @@ type CmdArgs struct {
 	StdinData   []byte
 }
 
+type reqForCmdEntry map[string]bool
+
 // PluginMain is the "main" for a plugin. It accepts
 // two callback functions for add and del commands.
 func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) {
 	var cmd, contID, netns, ifName, args, path string
 
 	vars := []struct {
-		name string
-		val  *string
-		req  bool
+		name      string
+		val       *string
+		reqForCmd reqForCmdEntry
 	}{
-		{"CNI_COMMAND", &cmd, true},
-		{"CNI_CONTAINERID", &contID, false},
-		{"CNI_NETNS", &netns, true},
-		{"CNI_IFNAME", &ifName, true},
-		{"CNI_ARGS", &args, false},
-		{"CNI_PATH", &path, true},
+		{
+			"CNI_COMMAND",
+			&cmd,
+			reqForCmdEntry{
+				"ADD": true,
+				"DEL": true,
+			},
+		},
+		{
+			"CNI_CONTAINERID",
+			&contID,
+			reqForCmdEntry{
+				"ADD": false,
+				"DEL": false,
+			},
+		},
+		{
+			"CNI_NETNS",
+			&netns,
+			reqForCmdEntry{
+				"ADD": true,
+				"DEL": false,
+			},
+		},
+		{
+			"CNI_IFNAME",
+			&ifName,
+			reqForCmdEntry{
+				"ADD": true,
+				"DEL": true,
+			},
+		},
+		{
+			"CNI_ARGS",
+			&args,
+			reqForCmdEntry{
+				"ADD": false,
+				"DEL": false,
+			},
+		},
+		{
+			"CNI_PATH",
+			&path,
+			reqForCmdEntry{
+				"ADD": true,
+				"DEL": true,
+			},
+		},
 	}
 
 	argsMissing := false
 	for _, v := range vars {
 		*v.val = os.Getenv(v.name)
-		if v.req && *v.val == "" {
+		if v.reqForCmd[cmd] && *v.val == "" {
 			log.Printf("%v env variable missing", v.name)
 			argsMissing = true
 		}

pkg/skel/skel_test.go

@@ -71,5 +71,14 @@ var _ = Describe("Skel", func() {
 		// 	Expect(err).NotTo(HaveOccurred())
 		// 	PluginMain(fErr, nil)
 		// })
+
+		It("should not fail with DEL and no NETNS and noop callback", func() {
+			err := os.Setenv("CNI_COMMAND", "DEL")
+			Expect(err).NotTo(HaveOccurred())
+			err = os.Unsetenv("CNI_NETNS")
+			Expect(err).NotTo(HaveOccurred())
+			PluginMain(nil, fNoop)
+		})
+
 	})
 })

plugins/main/bridge/bridge.go

@@ -289,6 +289,10 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if args.Netns == "" {
+		return nil
+	}
+
 	var ipn *net.IPNet
 	err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		var err error

plugins/main/ipvlan/ipvlan.go

@@ -152,6 +152,10 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if args.Netns == "" {
+		return nil
+	}
+
 	return ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		return ip.DelLinkByName(args.IfName)
 	})

plugins/main/macvlan/macvlan.go

@@ -170,6 +170,10 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if args.Netns == "" {
+		return nil
+	}
+
 	return ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		return ip.DelLinkByName(args.IfName)
 	})

plugins/main/ptp/ptp.go

@@ -199,6 +199,10 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if args.Netns == "" {
+		return nil
+	}
+
 	var ipn *net.IPNet
 	err := ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		var err error