From 449700f7ea1952e630bbbb7a05f9e4ae7e3864c4 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 7 Nov 2017 16:07:04 -0600 Subject: [PATCH 01/14] pkg/ip: don't return error from DelLinkByNameAddr() if no addresses exist For some reason no addresses on the interface returned an error, despite having a testcase that explicitly tested for success. --- pkg/ip/link.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ip/link.go b/pkg/ip/link.go index 3cb84861..843ce8aa 100644 --- a/pkg/ip/link.go +++ b/pkg/ip/link.go @@ -182,7 +182,7 @@ func DelLinkByNameAddr(ifName string) ([]*net.IPNet, error) { } addrs, err := netlink.AddrList(iface, netlink.FAMILY_ALL) - if err != nil || len(addrs) == 0 { + if err != nil { return nil, fmt.Errorf("failed to get IP addresses for %q: %v", ifName, err) } From 5576f3120e992d3d4610ebe2f54511a7f8762f44 Mon Sep 17 00:00:00 2001 From: Casey Callendrello Date: Fri, 3 Nov 2017 16:53:12 +0000 Subject: [PATCH 02/14] portmap: support hairpin, improve performance This change improves the performance of the portmap plugin and fixes hairpin, when a container is mapped back to itself. Performance is improved by using a multiport test to reduce rule traversal, and by using a masquerade mark. Hairpin is fixed by enabling masquerading for hairpin traffic. --- plugins/meta/portmap/README.md | 88 ++++--- plugins/meta/portmap/chain.go | 22 +- plugins/meta/portmap/chain_test.go | 28 +- plugins/meta/portmap/main.go | 27 +- plugins/meta/portmap/portmap.go | 286 ++++++++++++--------- plugins/meta/portmap/portmap_integ_test.go | 70 ++--- plugins/meta/portmap/portmap_test.go | 238 +++++++++++------ plugins/meta/portmap/utils.go | 50 ++++ 8 files changed, 513 insertions(+), 296 deletions(-) diff --git a/plugins/meta/portmap/README.md b/plugins/meta/portmap/README.md index fc6b86c9..748c5080 100644 --- a/plugins/meta/portmap/README.md +++ b/plugins/meta/portmap/README.md @@ -8,6 +8,8 @@ You should use this plugin as part of a network configuration list. It accepts the following configuration options: * `snat` - boolean, default true. If true or omitted, set up the SNAT chains +* `markMasqBit` - int, (0-31), default 13. The mark bit to use for masquerading (see section SNAT). Cannot be set when `externalSetMarkChain` is used. +* `externalSetMarkChain` - string, default nil. If you already have a Masquerade mark chain (e.g. Kubernetes), specify it here. This will use that instead of creating a separate chain. When this is set, `markMasqBit` must be unspecified. * `conditionsV4`, `conditionsV6` - array of strings. A list of arbitrary `iptables` matches to add to the per-container rule. This may be useful if you wish to exclude specific IPs from port-mapping @@ -15,7 +17,7 @@ exclude specific IPs from port-mapping The plugin expects to receive the actual list of port mappings via the `portMappings` [capability argument](https://github.com/containernetworking/cni/blob/master/CONVENTIONS.md) -So a sample standalone config list (with the file extension .conflist) might +A sample standalone config list for Kubernetes (with the file extension .conflist) might look like: ```json @@ -39,21 +41,31 @@ look like: { "type": "portmap", "capabilities": {"portMappings": true}, - "snat": false, - "conditionsV4": ["!", "-d", "192.0.2.0/24"], - "conditionsV6": ["!", "-d", "fc00::/7"] + "externalSetMarkChain": "KUBE-MARK-MASQ" } ] } ``` +A configuration file with all options set: +```json +{ + "type": "portmap", + "capabilities": {"portMappings": true}, + "snat": true, + "markMasqBit": 13, + "externalSetMarkChain": "CNI-HOSTPORT-SETMARK", + "conditionsV4": ["!", "-d", "192.0.2.0/24"], + "conditionsV6": ["!", "-d", "fc00::/7"] +} +``` + ## Rule structure The plugin sets up two sequences of chains and rules - one "primary" DNAT sequence to rewrite the destination, and one additional SNAT sequence that -rewrites the source address for packets from localhost. The sequence is somewhat -complex to minimize the number of rules non-forwarded packets must traverse. +will masquerade traffic as needed. ### DNAT @@ -68,50 +80,54 @@ rules look like this: - `--dst-type LOCAL -j CNI-HOSTPORT-DNAT` `CNI-HOSTPORT-DNAT` chain: -- `${ConditionsV4/6} -j CNI-DN-xxxxxx` (where xxxxxx is a function of the ContainerID and network name) +- `${ConditionsV4/6} -p tcp --destination-ports 8080,8043 -j CNI-DN-xxxxxx` (where xxxxxx is a function of the ContainerID and network name) -`CNI-DN-xxxxxx` chain: -- `-p tcp --dport 8080 -j DNAT --to-destination 172.16.30.2:80` +`CNI-HOSTPORT-SETMARK` chain: +- `-j MARK --set-xmark 0x2000/0x2000` + +`CNI-DN-xxxxxx` chain: +- `-p tcp -s 172.16.30.2 --dport 8080 -j CNI-HOSTPORT-SETMARK` (masquerade hairpin traffic) +- `-p tcp -s 127.0.0.1 --dport 8080 -j CNI-HOSTPORT-SETMARK` (masquerade localhost traffic) +- `-p tcp --dport 8080 -j DNAT --to-destination 172.16.30.2:80` (rewrite destination) +- `-p tcp -s 172.16.30.2 --dport 8043 -j CNI-HOSTPORT-SETMARK` +- `-p tcp -s 127.0.0.1 --dport 8043 -j CNI-HOSTPORT-SETMARK` - `-p tcp --dport 8043 -j DNAT --to-destination 172.16.30.2:443` New connections to the host will have to traverse every rule, so large numbers of port forwards may have a performance impact. This won't affect established connections, just the first packet. -### SNAT -The SNAT rule enables port-forwarding from the localhost IP on the host. -This rule rewrites (masquerades) the source address for connections from -localhost. If this rule did not exist, a connection to `localhost:80` would -still have a source IP of 127.0.0.1 when received by the container, so no -packets would respond. Again, it is a sequence of 3 chains. Because SNAT has to -occur in the `POSTROUTING` chain, the packet has already been through the DNAT -chain. +### SNAT (Masquerade) +Some packets also need to have the source address rewritten: +* connections from localhost +* Hairpin traffic back to the container. + +In the DNAT chain, a bit is set on the mark for packets that need snat. This +chain performs that masquerading. By default, bit 13 is set, but this is +configurable. If you are using other tools that also use the iptables mark, +you should make sure this doesn't conflict. + +Some container runtimes, most notably Kubernetes, already have a set of rules +for masquerading when a specific mark bit is set. If so enabled, the plugin +will use that chain instead. `POSTROUTING`: -- `-s 127.0.0.1 ! -d 127.0.0.1 -j CNI-HOSTPORT-SNAT` +- `-j CNI-HOSTPORT-MASQ` -`CNI-HOSTPORT-SNAT`: -- `-j CNI-SN-xxxxx` - -`CNI-SN-xxxxx`: -- `-p tcp -s 127.0.0.1 -d 172.16.30.2 --dport 80 -j MASQUERADE` -- `-p tcp -s 127.0.0.1 -d 172.16.30.2 --dport 443 -j MASQUERADE` - -Only new connections from the host, where the source address is 127.0.0.1 but -not the destination will traverse this chain. It is unlikely that any packets -will reach these rules without being SNATted, so the cost should be minimal. +`CNI-HOSTPORT-MASQ`: +- `--mark 0x2000 -j MASQUERADE` Because MASQUERADE happens in POSTROUTING, it means that packets with source ip -127.0.0.1 need to pass a routing boundary. By default, that is not allowed -in Linux. So, need to enable the sysctl `net.ipv4.conf.IFNAME.route_localnet`, -where IFNAME is the name of the host-side interface that routes traffic to the -container. +127.0.0.1 need to first pass a routing boundary before being masqueraded. By +default, that is not allowed in Linux. So, the plugin needs to enable the sysctl +`net.ipv4.conf.IFNAME.route_localnet`, where IFNAME is the name of the host-side +interface that routes traffic to the container. -There is no equivalent to `route_localnet` for ipv6, so SNAT does not work -for ipv6. If you need port forwarding from localhost, your container must have -an ipv4 address. +There is no equivalent to `route_localnet` for ipv6, so connections to ::1 +will not be portmapped for ipv6. If you need port forwarding from localhost, +your container must have an ipv4 address. ## Known issues - ipsets could improve efficiency -- SNAT does not work with ipv6. +- forwarding from localhost does not work with ipv6. diff --git a/plugins/meta/portmap/chain.go b/plugins/meta/portmap/chain.go index f8a53a45..5ebfe6a3 100644 --- a/plugins/meta/portmap/chain.go +++ b/plugins/meta/portmap/chain.go @@ -25,12 +25,14 @@ import ( type chain struct { table string name string - entryRule []string // the rule that enters this chain entryChains []string // the chains to add the entry rule + + entryRules [][]string // the rules that "point" to this chain + rules [][]string // the rules this chain contains } // setup idempotently creates the chain. It will not error if the chain exists. -func (c *chain) setup(ipt *iptables.IPTables, rules [][]string) error { +func (c *chain) setup(ipt *iptables.IPTables) error { // create the chain exists, err := chainExists(ipt, c.table, c.name) if err != nil { @@ -43,17 +45,21 @@ func (c *chain) setup(ipt *iptables.IPTables, rules [][]string) error { } // Add the rules to the chain - for i := len(rules) - 1; i >= 0; i-- { - if err := prependUnique(ipt, c.table, c.name, rules[i]); err != nil { + for i := len(c.rules) - 1; i >= 0; i-- { + if err := prependUnique(ipt, c.table, c.name, c.rules[i]); err != nil { return err } } - // Add the entry rules - entryRule := append(c.entryRule, "-j", c.name) + // Add the entry rules to the entry chains for _, entryChain := range c.entryChains { - if err := prependUnique(ipt, c.table, entryChain, entryRule); err != nil { - return err + for i := len(c.entryRules) - 1; i >= 0; i-- { + r := []string{} + r = append(r, c.entryRules[i]...) + r = append(r, "-j", c.name) + if err := prependUnique(ipt, c.table, entryChain, r); err != nil { + return err + } } } diff --git a/plugins/meta/portmap/chain_test.go b/plugins/meta/portmap/chain_test.go index 5cc4cf64..bff85c12 100644 --- a/plugins/meta/portmap/chain_test.go +++ b/plugins/meta/portmap/chain_test.go @@ -49,8 +49,12 @@ var _ = Describe("chain tests", func() { testChain = chain{ table: TABLE, name: chainName, - entryRule: []string{"-d", "203.0.113.1"}, entryChains: []string{tlChainName}, + entryRules: [][]string{{"-d", "203.0.113.1"}}, + rules: [][]string{ + {"-m", "comment", "--comment", "test 1", "-j", "RETURN"}, + {"-m", "comment", "--comment", "test 2", "-j", "RETURN"}, + }, } ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4) @@ -90,11 +94,7 @@ var _ = Describe("chain tests", func() { Expect(err).NotTo(HaveOccurred()) // Create the chain - chainRules := [][]string{ - {"-m", "comment", "--comment", "test 1", "-j", "RETURN"}, - {"-m", "comment", "--comment", "test 2", "-j", "RETURN"}, - } - err = testChain.setup(ipt, chainRules) + err = testChain.setup(ipt) Expect(err).NotTo(HaveOccurred()) // Verify the chain exists @@ -151,15 +151,11 @@ var _ = Describe("chain tests", func() { It("creates chains idempotently", func() { defer cleanup() - // Create the chain - chainRules := [][]string{ - {"-m", "comment", "--comment", "test", "-j", "RETURN"}, - } - err := testChain.setup(ipt, chainRules) + err := testChain.setup(ipt) Expect(err).NotTo(HaveOccurred()) // Create it again! - err = testChain.setup(ipt, chainRules) + err = testChain.setup(ipt) Expect(err).NotTo(HaveOccurred()) // Make sure there are only two rules @@ -167,18 +163,14 @@ var _ = Describe("chain tests", func() { rules, err := ipt.List(TABLE, testChain.name) Expect(err).NotTo(HaveOccurred()) - Expect(len(rules)).To(Equal(2)) + Expect(len(rules)).To(Equal(3)) }) It("deletes chains idempotently", func() { defer cleanup() - // Create the chain - chainRules := [][]string{ - {"-m", "comment", "--comment", "test", "-j", "RETURN"}, - } - err := testChain.setup(ipt, chainRules) + err := testChain.setup(ipt) Expect(err).NotTo(HaveOccurred()) err = testChain.teardown(ipt) diff --git a/plugins/meta/portmap/main.go b/plugins/meta/portmap/main.go index 6424a777..dfc52994 100644 --- a/plugins/meta/portmap/main.go +++ b/plugins/meta/portmap/main.go @@ -47,10 +47,12 @@ type PortMapEntry struct { type PortMapConf struct { types.NetConf - SNAT *bool `json:"snat,omitempty"` - ConditionsV4 *[]string `json:"conditionsV4"` - ConditionsV6 *[]string `json:"conditionsV6"` - RuntimeConfig struct { + SNAT *bool `json:"snat,omitempty"` + ConditionsV4 *[]string `json:"conditionsV4"` + ConditionsV6 *[]string `json:"conditionsV6"` + MarkMasqBit *int `json:"markMasqBit"` + ExternalSetMarkChain *string `json:"externalSetMarkChain"` + RuntimeConfig struct { PortMaps []PortMapEntry `json:"portMappings,omitempty"` } `json:"runtimeConfig,omitempty"` RawPrevResult map[string]interface{} `json:"prevResult,omitempty"` @@ -63,6 +65,10 @@ type PortMapConf struct { ContIPv6 net.IP `json:"-"` } +// The default mark bit to signal that masquerading is required +// Kubernetes uses 14 and 15, Calico uses 20-31. +const DefaultMarkBit = 13 + func cmdAdd(args *skel.CmdArgs) error { netConf, err := parseConfig(args.StdinData, args.IfName) if err != nil { @@ -145,6 +151,19 @@ func parseConfig(stdin []byte, ifName string) (*PortMapConf, error) { conf.SNAT = &tvar } + if conf.MarkMasqBit != nil && conf.ExternalSetMarkChain != nil { + return nil, fmt.Errorf("Cannot specify externalSetMarkChain and markMasqBit") + } + + if conf.MarkMasqBit == nil { + bvar := DefaultMarkBit // go constants are "special" + conf.MarkMasqBit = &bvar + } + + if *conf.MarkMasqBit < 0 || *conf.MarkMasqBit > 31 { + return nil, fmt.Errorf("MasqMarkBit must be between 0 and 31") + } + // Reject invalid port numbers for _, pm := range conf.RuntimeConfig.PortMaps { if pm.ContainerPort <= 0 { diff --git a/plugins/meta/portmap/portmap.go b/plugins/meta/portmap/portmap.go index 133dfef2..870552f6 100644 --- a/plugins/meta/portmap/portmap.go +++ b/plugins/meta/portmap/portmap.go @@ -17,6 +17,7 @@ package main import ( "fmt" "net" + "sort" "strconv" "github.com/containernetworking/plugins/pkg/utils/sysctl" @@ -24,33 +25,26 @@ import ( ) // This creates the chains to be added to iptables. The basic structure is -// a bit complex for efficiencies sake. We create 2 chains: a summary chain +// a bit complex for efficiency's sake. We create 2 chains: a summary chain // that is shared between invocations, and an invocation (container)-specific // chain. This minimizes the number of operations on the top level, but allows // for easy cleanup. // -// We also create DNAT chains to rewrite destinations, and SNAT chains so that -// connections to localhost work. -// // The basic setup (all operations are on the nat table) is: // // DNAT case (rewrite destination IP and port): -// PREROUTING, OUTPUT: --dst-type local -j CNI-HOSTPORT_DNAT -// CNI-HOSTPORT-DNAT: -j CNI-DN-abcd123 +// PREROUTING, OUTPUT: --dst-type local -j CNI-HOSTPORT-DNAT +// CNI-HOSTPORT-DNAT: --destination-ports 8080,8081 -j CNI-DN-abcd123 // CNI-DN-abcd123: -p tcp --dport 8080 -j DNAT --to-destination 192.0.2.33:80 // CNI-DN-abcd123: -p tcp --dport 8081 -j DNAT ... -// -// SNAT case (rewrite source IP from localhost after dnat): -// POSTROUTING: -s 127.0.0.1 ! -d 127.0.0.1 -j CNI-HOSTPORT-SNAT -// CNI-HOSTPORT-SNAT: -j CNI-SN-abcd123 -// CNI-SN-abcd123: -p tcp -s 127.0.0.1 -d 192.0.2.33 --dport 80 -j MASQUERADE -// CNI-SN-abcd123: -p tcp -s 127.0.0.1 -d 192.0.2.33 --dport 90 -j MASQUERADE // The names of the top-level summary chains. // These should never be changed, or else upgrading will require manual // intervention. const TopLevelDNATChainName = "CNI-HOSTPORT-DNAT" -const TopLevelSNATChainName = "CNI-HOSTPORT-SNAT" +const SetMarkChainName = "CNI-HOSTPORT-SETMARK" +const MarkMasqChainName = "CNI-HOSTPORT-MASQ" +const OldTopLevelSNATChainName = "CNI-HOSTPORT-SNAT" // forwardPorts establishes port forwarding to a given container IP. // containerIP can be either v4 or v6. @@ -59,48 +53,35 @@ func forwardPorts(config *PortMapConf, containerIP net.IP) error { var ipt *iptables.IPTables var err error - var conditions *[]string if isV6 { ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6) - conditions = config.ConditionsV6 } else { ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4) - conditions = config.ConditionsV4 } if err != nil { return fmt.Errorf("failed to open iptables: %v", err) } - toplevelDnatChain := genToplevelDnatChain() - if err := toplevelDnatChain.setup(ipt, nil); err != nil { - return fmt.Errorf("failed to create top-level DNAT chain: %v", err) - } + // Enable masquerading for traffic as necessary. + // The DNAT chain sets a mark bit for traffic that needs masq: + // - connections from localhost + // - hairpin traffic back to the container + // Idempotently create the rule that masquerades traffic with this mark. + // Need to do this first; the DNAT rules reference these chains + if *config.SNAT { + if config.ExternalSetMarkChain == nil { + setMarkChain := genSetMarkChain(*config.MarkMasqBit) + if err := setMarkChain.setup(ipt); err != nil { + return fmt.Errorf("unable to create chain %s: %v", setMarkChain.name, err) + } - dnatChain := genDnatChain(config.Name, config.ContainerID, conditions) - _ = dnatChain.teardown(ipt) // If we somehow collide on this container ID + network, cleanup - - dnatRules := dnatRules(config.RuntimeConfig.PortMaps, containerIP) - if err := dnatChain.setup(ipt, dnatRules); err != nil { - return fmt.Errorf("unable to setup DNAT: %v", err) - } - - // Enable SNAT for connections to localhost. - // This won't work for ipv6, since the kernel doesn't have the equvalent - // route_localnet sysctl. - if *config.SNAT && !isV6 { - toplevelSnatChain := genToplevelSnatChain(isV6) - if err := toplevelSnatChain.setup(ipt, nil); err != nil { - return fmt.Errorf("failed to create top-level SNAT chain: %v", err) + masqChain := genMarkMasqChain(*config.MarkMasqBit) + if err := masqChain.setup(ipt); err != nil { + return fmt.Errorf("unable to create chain %s: %v", setMarkChain.name, err) + } } - snatChain := genSnatChain(config.Name, config.ContainerID) - _ = snatChain.teardown(ipt) - - snatRules := snatRules(config.RuntimeConfig.PortMaps, containerIP) - if err := snatChain.setup(ipt, snatRules); err != nil { - return fmt.Errorf("unable to setup SNAT: %v", err) - } if !isV6 { // Set the route_localnet bit on the host interface, so that // 127/8 can cross a routing boundary. @@ -113,6 +94,20 @@ func forwardPorts(config *PortMapConf, containerIP net.IP) error { } } + // Generate the DNAT (actual port forwarding) rules + toplevelDnatChain := genToplevelDnatChain() + if err := toplevelDnatChain.setup(ipt); err != nil { + return fmt.Errorf("failed to create top-level DNAT chain: %v", err) + } + + dnatChain := genDnatChain(config.Name, config.ContainerID) + // First, idempotently tear down this chain in case there was some + // sort of collision or bad state. + fillDnatRules(&dnatChain, config, containerIP) + if err := dnatChain.setup(ipt); err != nil { + return fmt.Errorf("unable to setup DNAT: %v", err) + } + return nil } @@ -124,106 +119,153 @@ func genToplevelDnatChain() chain { return chain{ table: "nat", name: TopLevelDNATChainName, - entryRule: []string{ + entryRules: [][]string{{ "-m", "addrtype", "--dst-type", "LOCAL", - }, + }}, entryChains: []string{"PREROUTING", "OUTPUT"}, } } // genDnatChain creates the per-container chain. // Conditions are any static entry conditions for the chain. -func genDnatChain(netName, containerID string, conditions *[]string) chain { - name := formatChainName("DN-", netName, containerID) - comment := fmt.Sprintf(`dnat name: "%s" id: "%s"`, netName, containerID) - - ch := chain{ - table: "nat", - name: name, - entryRule: []string{ - "-m", "comment", - "--comment", comment, - }, +func genDnatChain(netName, containerID string) chain { + return chain{ + table: "nat", + name: formatChainName("DN-", netName, containerID), entryChains: []string{TopLevelDNATChainName}, } - if conditions != nil && len(*conditions) != 0 { - ch.entryRule = append(ch.entryRule, *conditions...) - } - - return ch } // dnatRules generates the destination NAT rules, one per port, to direct // traffic from hostip:hostport to podip:podport -func dnatRules(entries []PortMapEntry, containerIP net.IP) [][]string { - out := make([][]string, 0, len(entries)) +func fillDnatRules(c *chain, config *PortMapConf, containerIP net.IP) { + isV6 := (containerIP.To4() == nil) + comment := trimComment(fmt.Sprintf(`dnat name: "%s" id: "%s"`, config.Name, config.ContainerID)) + entries := config.RuntimeConfig.PortMaps + setMarkChainName := SetMarkChainName + if config.ExternalSetMarkChain != nil { + setMarkChainName = *config.ExternalSetMarkChain + } + + //Generate the dnat entry rules. We'll use multiport, but it ony accepts + // up to 15 rules, so partition the list if needed. + // Do it in a stable order for testing + protoPorts := groupByProto(entries) + protos := []string{} + for proto := range protoPorts { + protos = append(protos, proto) + } + sort.Strings(protos) + for _, proto := range protos { + for _, portSpec := range splitPortList(protoPorts[proto]) { + r := []string{ + "-m", "comment", + "--comment", comment, + "-m", "multiport", + "-p", proto, + "--destination-ports", portSpec, + } + + if isV6 && config.ConditionsV6 != nil && len(*config.ConditionsV6) > 0 { + r = append(r, *config.ConditionsV6...) + } else if !isV6 && config.ConditionsV4 != nil && len(*config.ConditionsV4) > 0 { + r = append(r, *config.ConditionsV4...) + } + c.entryRules = append(c.entryRules, r) + } + } + + // For every entry, generate 3 rules: + // - mark hairpin for masq + // - mark localhost for masq (for v4) + // - do dnat + // the ordering is important here; the mark rules must be first. + c.rules = make([][]string, 0, 3*len(entries)) for _, entry := range entries { - rule := []string{ + ruleBase := []string{ "-p", entry.Protocol, "--dport", strconv.Itoa(entry.HostPort)} - if entry.HostIP != "" { - rule = append(rule, + ruleBase = append(ruleBase, "-d", entry.HostIP) } - rule = append(rule, + // Add mark-to-masquerade rules for hairpin and localhost + if *config.SNAT { + // hairpin + hpRule := make([]string, len(ruleBase), len(ruleBase)+4) + copy(hpRule, ruleBase) + + hpRule = append(hpRule, + "-s", containerIP.String(), + "-j", setMarkChainName, + ) + c.rules = append(c.rules, hpRule) + + if !isV6 { + // localhost + localRule := make([]string, len(ruleBase), len(ruleBase)+4) + copy(localRule, ruleBase) + + localRule = append(localRule, + "-s", "127.0.0.1", + "-j", setMarkChainName, + ) + c.rules = append(c.rules, localRule) + } + } + + // The actual dnat rule + dnatRule := make([]string, len(ruleBase), len(ruleBase)+4) + copy(dnatRule, ruleBase) + dnatRule = append(dnatRule, "-j", "DNAT", - "--to-destination", fmtIpPort(containerIP, entry.ContainerPort)) - - out = append(out, rule) - } - return out -} - -// genToplevelSnatChain creates the top-level summary snat chain. -// IMPORTANT: do not change this, or else upgrading plugins will require -// manual intervention -func genToplevelSnatChain(isV6 bool) chain { - return chain{ - table: "nat", - name: TopLevelSNATChainName, - entryRule: []string{ - "-s", localhostIP(isV6), - "!", "-d", localhostIP(isV6), - }, - entryChains: []string{"POSTROUTING"}, + "--to-destination", fmtIpPort(containerIP, entry.ContainerPort), + ) + c.rules = append(c.rules, dnatRule) } } -// genSnatChain creates the snat (localhost) chain for this container. -func genSnatChain(netName, containerID string) chain { - name := formatChainName("SN-", netName, containerID) - comment := fmt.Sprintf(`snat name: "%s" id: "%s"`, netName, containerID) - - return chain{ +// genSetMarkChain creates the SETMARK chain - the chain that sets the +// "to-be-masqueraded" mark and returns. +// Chains are idempotent, so we'll always create this. +func genSetMarkChain(markBit int) chain { + markValue := 1 << uint(markBit) + markDef := fmt.Sprintf("%#x/%#x", markValue, markValue) + ch := chain{ table: "nat", - name: name, - entryRule: []string{ + name: SetMarkChainName, + rules: [][]string{{ "-m", "comment", - "--comment", comment, - }, - entryChains: []string{TopLevelSNATChainName}, + "--comment", "CNI portfwd masquerade mark", + "-j", "MARK", + "--set-xmark", markDef, + }}, } + return ch } -// snatRules sets up masquerading for connections to localhost:hostport, -// rewriting the source so that returning packets are correct. -func snatRules(entries []PortMapEntry, containerIP net.IP) [][]string { - isV6 := (containerIP.To4() == nil) - - out := make([][]string, 0, len(entries)) - for _, entry := range entries { - out = append(out, []string{ - "-p", entry.Protocol, - "-s", localhostIP(isV6), - "-d", containerIP.String(), - "--dport", strconv.Itoa(entry.ContainerPort), +// genMarkMasqChain creates the chain that masquerades all packets marked +// in the SETMARK chain +func genMarkMasqChain(markBit int) chain { + markValue := 1 << uint(markBit) + markDef := fmt.Sprintf("%#x/%#x", markValue, markValue) + ch := chain{ + table: "nat", + name: MarkMasqChainName, + entryChains: []string{"POSTROUTING"}, + entryRules: [][]string{{ + "-m", "comment", + "--comment", "CNI portfwd requiring masquerade", + }}, + rules: [][]string{{ + "-m", "mark", + "--mark", markDef, "-j", "MASQUERADE", - }) + }}, } - return out + return ch } // enableLocalnetRouting tells the kernel not to treat 127/8 as a martian, @@ -234,6 +276,18 @@ func enableLocalnetRouting(ifName string) error { return err } +// genOldSnatChain is no longer used, but used to be created. We'll try and +// tear it down in case the plugin version changed between ADD and DEL +func genOldSnatChain(netName, containerID string) chain { + name := formatChainName("SN-", netName, containerID) + + return chain{ + table: "nat", + name: name, + entryChains: []string{OldTopLevelSNATChainName}, + } +} + // unforwardPorts deletes any iptables rules created by this plugin. // It should be idempotent - it will not error if the chain does not exist. // @@ -245,8 +299,10 @@ func enableLocalnetRouting(ifName string) error { // So, we first check that iptables is "generally OK" by doing a check. If // not, we ignore the error, unless neither v4 nor v6 are OK. func unforwardPorts(config *PortMapConf) error { - dnatChain := genDnatChain(config.Name, config.ContainerID, nil) - snatChain := genSnatChain(config.Name, config.ContainerID) + dnatChain := genDnatChain(config.Name, config.ContainerID) + + // Might be lying around from old versions + oldSnatChain := genOldSnatChain(config.Name, config.ContainerID) ip4t := maybeGetIptables(false) ip6t := maybeGetIptables(true) @@ -258,16 +314,14 @@ func unforwardPorts(config *PortMapConf) error { if err := dnatChain.teardown(ip4t); err != nil { return fmt.Errorf("could not teardown ipv4 dnat: %v", err) } - if err := snatChain.teardown(ip4t); err != nil { - return fmt.Errorf("could not teardown ipv4 snat: %v", err) - } + oldSnatChain.teardown(ip4t) } if ip6t != nil { if err := dnatChain.teardown(ip6t); err != nil { return fmt.Errorf("could not teardown ipv6 dnat: %v", err) } - // no SNAT teardown because it doesn't work for v6 + oldSnatChain.teardown(ip6t) } return nil } diff --git a/plugins/meta/portmap/portmap_integ_test.go b/plugins/meta/portmap/portmap_integ_test.go index 91a7518f..7bd9e0cf 100644 --- a/plugins/meta/portmap/portmap_integ_test.go +++ b/plugins/meta/portmap/portmap_integ_test.go @@ -15,12 +15,14 @@ package main import ( + "bytes" "fmt" "math/rand" "net" "os" + "os/exec" "path/filepath" - "time" + "strconv" "github.com/containernetworking/cni/libcni" "github.com/containernetworking/cni/pkg/types/current" @@ -124,13 +126,20 @@ var _ = Describe("portmap integration tests", func() { // we'll also manually check the iptables chains ipt, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) Expect(err).NotTo(HaveOccurred()) - dnatChainName := genDnatChain("cni-portmap-unit-test", runtimeConfig.ContainerID, nil).name + dnatChainName := genDnatChain("cni-portmap-unit-test", runtimeConfig.ContainerID).name // Create the network resI, err := cniConf.AddNetworkList(configList, &runtimeConfig) Expect(err).NotTo(HaveOccurred()) defer deleteNetwork() + // Undo Docker's forwarding policy + cmd := exec.Command("iptables", "-t", "filter", + "-P", "FORWARD", "ACCEPT") + cmd.Stderr = GinkgoWriter + err = cmd.Run() + Expect(err).NotTo(HaveOccurred()) + // Check the chain exists _, err = ipt.List("nat", dnatChainName) Expect(err).NotTo(HaveOccurred()) @@ -155,13 +164,16 @@ var _ = Describe("portmap integration tests", func() { hostIP, hostPort, contIP, containerPort) // Sanity check: verify that the container is reachable directly - contOK := testEchoServer(fmt.Sprintf("%s:%d", contIP.String(), containerPort)) + contOK := testEchoServer(contIP.String(), containerPort, "") // Verify that a connection to the forwarded port works - dnatOK := testEchoServer(fmt.Sprintf("%s:%d", hostIP, hostPort)) + dnatOK := testEchoServer(hostIP, hostPort, "") // Verify that a connection to localhost works - snatOK := testEchoServer(fmt.Sprintf("%s:%d", "127.0.0.1", hostPort)) + snatOK := testEchoServer("127.0.0.1", hostPort, "") + + // verify that hairpin works + hairpinOK := testEchoServer(hostIP, hostPort, targetNS.Path()) // Cleanup session.Terminate() @@ -182,6 +194,9 @@ var _ = Describe("portmap integration tests", func() { if !snatOK { Fail("connection to 127.0.0.1 was not forwarded") } + if !hairpinOK { + Fail("Hairpin connection failed") + } close(done) @@ -189,40 +204,33 @@ var _ = Describe("portmap integration tests", func() { }) // testEchoServer returns true if we found an echo server on the port -func testEchoServer(address string) bool { - fmt.Fprintln(GinkgoWriter, "dialing", address) - conn, err := net.Dial("tcp", address) - if err != nil { - fmt.Fprintln(GinkgoWriter, "connection to", address, "failed:", err) - return false - } - defer conn.Close() - - conn.SetDeadline(time.Now().Add(TIMEOUT * time.Second)) - fmt.Fprintln(GinkgoWriter, "connected to", address) - +func testEchoServer(address string, port int, netns string) bool { message := "Aliquid melius quam pessimum optimum non est." - _, err = fmt.Fprint(conn, message) + + bin, err := exec.LookPath("nc") + Expect(err).NotTo(HaveOccurred()) + var cmd *exec.Cmd + if netns != "" { + netns = filepath.Base(netns) + cmd = exec.Command("ip", "netns", "exec", netns, bin, "-v", address, strconv.Itoa(port)) + } else { + cmd = exec.Command("nc", address, strconv.Itoa(port)) + } + cmd.Stdin = bytes.NewBufferString(message) + cmd.Stderr = GinkgoWriter + out, err := cmd.Output() if err != nil { - fmt.Fprintln(GinkgoWriter, "sending message to", address, " failed:", err) + fmt.Fprintln(GinkgoWriter, "got non-zero exit from ", cmd.Args) return false } - conn.SetDeadline(time.Now().Add(TIMEOUT * time.Second)) - fmt.Fprintln(GinkgoWriter, "reading...") - response := make([]byte, len(message)) - _, err = conn.Read(response) - if err != nil { - fmt.Fprintln(GinkgoWriter, "receiving message from", address, " failed:", err) + if string(out) != message { + fmt.Fprintln(GinkgoWriter, "returned message didn't match?") + fmt.Fprintln(GinkgoWriter, string(out)) return false } - fmt.Fprintln(GinkgoWriter, "read...") - if string(response) == message { - return true - } - fmt.Fprintln(GinkgoWriter, "returned message didn't match?") - return false + return true } func getLocalIP() string { diff --git a/plugins/meta/portmap/portmap_test.go b/plugins/meta/portmap/portmap_test.go index cf5ed5fb..d47b483c 100644 --- a/plugins/meta/portmap/portmap_test.go +++ b/plugins/meta/portmap/portmap_test.go @@ -15,6 +15,7 @@ package main import ( + "fmt" "net" . "github.com/onsi/ginkgo" @@ -25,13 +26,6 @@ var _ = Describe("portmapping configuration", func() { netName := "testNetName" containerID := "icee6giejonei6sohng6ahngee7laquohquee9shiGo7fohferakah3Feiyoolu2pei7ciPhoh7shaoX6vai3vuf0ahfaeng8yohb9ceu0daez5hashee8ooYai5wa3y" - mappings := []PortMapEntry{ - {80, 90, "tcp", ""}, - {1000, 2000, "udp", ""}, - } - ipv4addr := net.ParseIP("192.2.0.1") - ipv6addr := net.ParseIP("2001:db8::1") - Context("config parsing", func() { It("Correctly parses an ADD config", func() { configBytes := []byte(`{ @@ -156,101 +150,179 @@ var _ = Describe("portmapping configuration", func() { Describe("Generating chains", func() { Context("for DNAT", func() { - It("generates a correct container chain", func() { - ch := genDnatChain(netName, containerID, &[]string{"-m", "hello"}) + It("generates a correct standard container chain", func() { + ch := genDnatChain(netName, containerID) Expect(ch).To(Equal(chain{ - table: "nat", - name: "CNI-DN-bfd599665540dd91d5d28", - entryRule: []string{ - "-m", "comment", - "--comment", `dnat name: "testNetName" id: "` + containerID + `"`, - "-m", "hello", - }, + table: "nat", + name: "CNI-DN-bfd599665540dd91d5d28", entryChains: []string{TopLevelDNATChainName}, })) + configBytes := []byte(`{ + "name": "test", + "type": "portmap", + "cniVersion": "0.3.1", + "runtimeConfig": { + "portMappings": [ + { "hostPort": 8080, "containerPort": 80, "protocol": "tcp"}, + { "hostPort": 8081, "containerPort": 80, "protocol": "tcp"}, + { "hostPort": 8080, "containerPort": 81, "protocol": "udp"}, + { "hostPort": 8082, "containerPort": 82, "protocol": "udp"} + ] + }, + "snat": true, + "conditionsV4": ["a", "b"], + "conditionsV6": ["c", "d"] +}`) + + conf, err := parseConfig(configBytes, "foo") + Expect(err).NotTo(HaveOccurred()) + conf.ContainerID = containerID + + ch = genDnatChain(conf.Name, containerID) + Expect(ch).To(Equal(chain{ + table: "nat", + name: "CNI-DN-67e92b96e692a494b6b85", + entryChains: []string{"CNI-HOSTPORT-DNAT"}, + })) + + fillDnatRules(&ch, conf, net.ParseIP("10.0.0.2")) + + Expect(ch.entryRules).To(Equal([][]string{ + {"-m", "comment", "--comment", + fmt.Sprintf("dnat name: \"test\" id: \"%s\"", containerID), + "-m", "multiport", + "-p", "tcp", + "--destination-ports", "8080,8081", + "a", "b"}, + {"-m", "comment", "--comment", + fmt.Sprintf("dnat name: \"test\" id: \"%s\"", containerID), + "-m", "multiport", + "-p", "udp", + "--destination-ports", "8080,8082", + "a", "b"}, + })) + + Expect(ch.rules).To(Equal([][]string{ + {"-p", "tcp", "--dport", "8080", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8080", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, + {"-p", "tcp", "--dport", "8081", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8081", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, + {"-p", "udp", "--dport", "8080", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "udp", "--dport", "8080", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "udp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:81"}, + {"-p", "udp", "--dport", "8082", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "udp", "--dport", "8082", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "udp", "--dport", "8082", "-j", "DNAT", "--to-destination", "10.0.0.2:82"}, + })) + + ch.rules = nil + ch.entryRules = nil + + fillDnatRules(&ch, conf, net.ParseIP("2001:db8::2")) + + Expect(ch.rules).To(Equal([][]string{ + {"-p", "tcp", "--dport", "8080", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "[2001:db8::2]:80"}, + {"-p", "tcp", "--dport", "8081", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "[2001:db8::2]:80"}, + {"-p", "udp", "--dport", "8080", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "udp", "--dport", "8080", "-j", "DNAT", "--to-destination", "[2001:db8::2]:81"}, + {"-p", "udp", "--dport", "8082", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "udp", "--dport", "8082", "-j", "DNAT", "--to-destination", "[2001:db8::2]:82"}, + })) + + // Disable snat, generate rules + ch.rules = nil + ch.entryRules = nil + fvar := false + conf.SNAT = &fvar + + fillDnatRules(&ch, conf, net.ParseIP("10.0.0.2")) + Expect(ch.rules).To(Equal([][]string{ + {"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, + {"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, + {"-p", "udp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:81"}, + {"-p", "udp", "--dport", "8082", "-j", "DNAT", "--to-destination", "10.0.0.2:82"}, + })) + }) + + It("generates a correct chain with external mark", func() { + ch := genDnatChain(netName, containerID) + + Expect(ch).To(Equal(chain{ + table: "nat", + name: "CNI-DN-bfd599665540dd91d5d28", + entryChains: []string{TopLevelDNATChainName}, + })) + configBytes := []byte(`{ + "name": "test", + "type": "portmap", + "cniVersion": "0.3.1", + "runtimeConfig": { + "portMappings": [ + { "hostPort": 8080, "containerPort": 80, "protocol": "tcp"} + ] + }, + "externalSetMarkChain": "PLZ-SET-MARK", + "conditionsV4": ["a", "b"], + "conditionsV6": ["c", "d"] +}`) + + conf, err := parseConfig(configBytes, "foo") + Expect(err).NotTo(HaveOccurred()) + conf.ContainerID = containerID + + ch = genDnatChain(conf.Name, containerID) + fillDnatRules(&ch, conf, net.ParseIP("10.0.0.2")) + Expect(ch.rules).To(Equal([][]string{ + {"-p", "tcp", "--dport", "8080", "-s", "10.0.0.2", "-j", "PLZ-SET-MARK"}, + {"-p", "tcp", "--dport", "8080", "-s", "127.0.0.1", "-j", "PLZ-SET-MARK"}, + {"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, + })) }) It("generates a correct top-level chain", func() { ch := genToplevelDnatChain() Expect(ch).To(Equal(chain{ - table: "nat", - name: "CNI-HOSTPORT-DNAT", - entryRule: []string{ - "-m", "addrtype", - "--dst-type", "LOCAL", - }, + table: "nat", + name: "CNI-HOSTPORT-DNAT", entryChains: []string{"PREROUTING", "OUTPUT"}, + entryRules: [][]string{{"-m", "addrtype", "--dst-type", "LOCAL"}}, })) }) - }) - - Context("for SNAT", func() { - It("generates a correct container chain", func() { - ch := genSnatChain(netName, containerID) + It("generates the correct mark chains", func() { + masqBit := 5 + ch := genSetMarkChain(masqBit) Expect(ch).To(Equal(chain{ table: "nat", - name: "CNI-SN-bfd599665540dd91d5d28", - entryRule: []string{ + name: "CNI-HOSTPORT-SETMARK", + rules: [][]string{{ "-m", "comment", - "--comment", `snat name: "testNetName" id: "` + containerID + `"`, - }, - entryChains: []string{TopLevelSNATChainName}, + "--comment", "CNI portfwd masquerade mark", + "-j", "MARK", + "--set-xmark", "0x20/0x20", + }}, })) - }) - It("generates a correct top-level chain", func() { - Context("for ipv4", func() { - ch := genToplevelSnatChain(false) - Expect(ch).To(Equal(chain{ - table: "nat", - name: "CNI-HOSTPORT-SNAT", - entryRule: []string{ - "-s", "127.0.0.1", - "!", "-d", "127.0.0.1", - }, - entryChains: []string{"POSTROUTING"}, - })) - }) - }) - }) - }) - - Describe("Forwarding rules", func() { - Context("for DNAT", func() { - It("generates correct ipv4 rules", func() { - rules := dnatRules(mappings, ipv4addr) - Expect(rules).To(Equal([][]string{ - {"-p", "tcp", "--dport", "80", "-j", "DNAT", "--to-destination", "192.2.0.1:90"}, - {"-p", "udp", "--dport", "1000", "-j", "DNAT", "--to-destination", "192.2.0.1:2000"}, - })) - }) - It("generates correct ipv6 rules", func() { - rules := dnatRules(mappings, ipv6addr) - Expect(rules).To(Equal([][]string{ - {"-p", "tcp", "--dport", "80", "-j", "DNAT", "--to-destination", "[2001:db8::1]:90"}, - {"-p", "udp", "--dport", "1000", "-j", "DNAT", "--to-destination", "[2001:db8::1]:2000"}, - })) - }) - }) - - Context("for SNAT", func() { - - It("generates correct ipv4 rules", func() { - rules := snatRules(mappings, ipv4addr) - Expect(rules).To(Equal([][]string{ - {"-p", "tcp", "-s", "127.0.0.1", "-d", "192.2.0.1", "--dport", "90", "-j", "MASQUERADE"}, - {"-p", "udp", "-s", "127.0.0.1", "-d", "192.2.0.1", "--dport", "2000", "-j", "MASQUERADE"}, - })) - }) - - It("generates correct ipv6 rules", func() { - rules := snatRules(mappings, ipv6addr) - Expect(rules).To(Equal([][]string{ - {"-p", "tcp", "-s", "::1", "-d", "2001:db8::1", "--dport", "90", "-j", "MASQUERADE"}, - {"-p", "udp", "-s", "::1", "-d", "2001:db8::1", "--dport", "2000", "-j", "MASQUERADE"}, + ch = genMarkMasqChain(masqBit) + Expect(ch).To(Equal(chain{ + table: "nat", + name: "CNI-HOSTPORT-MASQ", + entryChains: []string{"POSTROUTING"}, + entryRules: [][]string{{ + "-m", "comment", + "--comment", "CNI portfwd requiring masquerade", + }}, + rules: [][]string{{ + "-m", "mark", + "--mark", "0x20/0x20", + "-j", "MASQUERADE", + }}, })) }) }) diff --git a/plugins/meta/portmap/utils.go b/plugins/meta/portmap/utils.go index a0c9b33b..6d6bc298 100644 --- a/plugins/meta/portmap/utils.go +++ b/plugins/meta/portmap/utils.go @@ -18,6 +18,8 @@ import ( "crypto/sha512" "fmt" "net" + "strconv" + "strings" "github.com/vishvananda/netlink" ) @@ -65,3 +67,51 @@ func formatChainName(prefix, name, id string) string { chain := fmt.Sprintf("CNI-%s%x", prefix, chainBytes) return chain[:maxChainNameLength] } + +// groupByProto groups port numbers by protocol +func groupByProto(entries []PortMapEntry) map[string][]int { + if len(entries) == 0 { + return map[string][]int{} + } + out := map[string][]int{} + for _, e := range entries { + _, ok := out[e.Protocol] + if ok { + out[e.Protocol] = append(out[e.Protocol], e.HostPort) + } else { + out[e.Protocol] = []int{e.HostPort} + } + } + + return out +} + +// splitPortList splits a list of integers in to one or more comma-separated +// string values, for use by multiport. Multiport only allows up to 15 ports +// per entry. +func splitPortList(l []int) []string { + out := []string{} + + acc := []string{} + for _, i := range l { + acc = append(acc, strconv.Itoa(i)) + if len(acc) == 15 { + out = append(out, strings.Join(acc, ",")) + acc = []string{} + } + } + + if len(acc) > 0 { + out = append(out, strings.Join(acc, ",")) + } + return out +} + +// trimComment makes sure no comment is over the iptables limit of 255 chars +func trimComment(val string) string { + if len(val) <= 255 { + return val + } + + return val[0:253] + "..." +} From 99f6be03192a48ecf6c3bbeece7948364da08fac Mon Sep 17 00:00:00 2001 From: Gabriel Rosenhouse Date: Tue, 31 Oct 2017 22:58:50 -0700 Subject: [PATCH 03/14] Enable Windows CI (Appveyor) - start list of linux_only plugins; ignore them when testing on Windows - Isolate linux-only code by filename suffix - Remove stub (NotImplemented) functions - other misc. fixes for Windows compatibility --- .appveyor.yml | 28 +++ pkg/ip/{addr.go => addr_linux.go} | 0 pkg/ip/{ipforward.go => ipforward_linux.go} | 0 pkg/ip/{ipmasq.go => ipmasq_linux.go} | 0 pkg/ip/{link.go => link_linux.go} | 0 pkg/ip/{link_test.go => link_linux_test.go} | 0 pkg/ip/route.go | 27 --- pkg/ip/route_linux.go | 6 + pkg/ip/route_unspecified.go | 34 ---- pkg/ipam/ipam.go | 71 ------- pkg/ipam/ipam_linux.go | 89 +++++++++ pkg/ipam/{ipam_test.go => ipam_linux_test.go} | 2 +- pkg/ns/ns.go | 178 ------------------ pkg/ns/ns_linux.go | 156 +++++++++++++++ pkg/ns/{ns_test.go => ns_linux_test.go} | 0 pkg/ns/ns_unspecified.go | 36 ---- pkg/testutils/echosvr/echosvr_test.go | 2 +- plugins/ipam/host-local/backend/disk/lock.go | 1 + plugins/linux_only.txt | 11 ++ ...{flannel_test.go => flannel_linux_test.go} | 0 .../{sample_test.go => sample_linux_test.go} | 0 21 files changed, 293 insertions(+), 348 deletions(-) create mode 100644 .appveyor.yml rename pkg/ip/{addr.go => addr_linux.go} (100%) rename pkg/ip/{ipforward.go => ipforward_linux.go} (100%) rename pkg/ip/{ipmasq.go => ipmasq_linux.go} (100%) rename pkg/ip/{link.go => link_linux.go} (100%) rename pkg/ip/{link_test.go => link_linux_test.go} (100%) delete mode 100644 pkg/ip/route.go delete mode 100644 pkg/ip/route_unspecified.go create mode 100644 pkg/ipam/ipam_linux.go rename pkg/ipam/{ipam_test.go => ipam_linux_test.go} (99%) delete mode 100644 pkg/ns/ns.go rename pkg/ns/{ns_test.go => ns_linux_test.go} (100%) delete mode 100644 pkg/ns/ns_unspecified.go create mode 100644 plugins/linux_only.txt rename plugins/meta/flannel/{flannel_test.go => flannel_linux_test.go} (100%) rename plugins/sample/{sample_test.go => sample_linux_test.go} (100%) diff --git a/.appveyor.yml b/.appveyor.yml new file mode 100644 index 00000000..ea06455d --- /dev/null +++ b/.appveyor.yml @@ -0,0 +1,28 @@ +clone_folder: c:\gopath\src\github.com\containernetworking\plugins + +environment: + GOPATH: c:\gopath + +install: + - echo %PATH% + - echo %GOPATH% + - set PATH=%GOPATH%\bin;c:\go\bin;%PATH% + - go version + - go env + +build: off + +test_script: + - ps: | + go list ./... | Select-String -Pattern (Get-Content "./plugins/linux_only.txt") -NotMatch > "to_test.txt" + echo "Will test:" + Get-Content "to_test.txt" + foreach ($pkg in Get-Content "to_test.txt") { + if ($pkg) { + echo $pkg + go test -v $pkg + if ($LastExitCode -ne 0) { + throw "test failed" + } + } + } diff --git a/pkg/ip/addr.go b/pkg/ip/addr_linux.go similarity index 100% rename from pkg/ip/addr.go rename to pkg/ip/addr_linux.go diff --git a/pkg/ip/ipforward.go b/pkg/ip/ipforward_linux.go similarity index 100% rename from pkg/ip/ipforward.go rename to pkg/ip/ipforward_linux.go diff --git a/pkg/ip/ipmasq.go b/pkg/ip/ipmasq_linux.go similarity index 100% rename from pkg/ip/ipmasq.go rename to pkg/ip/ipmasq_linux.go diff --git a/pkg/ip/link.go b/pkg/ip/link_linux.go similarity index 100% rename from pkg/ip/link.go rename to pkg/ip/link_linux.go diff --git a/pkg/ip/link_test.go b/pkg/ip/link_linux_test.go similarity index 100% rename from pkg/ip/link_test.go rename to pkg/ip/link_linux_test.go diff --git a/pkg/ip/route.go b/pkg/ip/route.go deleted file mode 100644 index 1325a47a..00000000 --- a/pkg/ip/route.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2015 CNI authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package ip - -import ( - "net" - - "github.com/vishvananda/netlink" -) - -// AddDefaultRoute sets the default route on the given gateway. -func AddDefaultRoute(gw net.IP, dev netlink.Link) error { - _, defNet, _ := net.ParseCIDR("0.0.0.0/0") - return AddRoute(defNet, gw, dev) -} diff --git a/pkg/ip/route_linux.go b/pkg/ip/route_linux.go index 8b11807d..f5c0d080 100644 --- a/pkg/ip/route_linux.go +++ b/pkg/ip/route_linux.go @@ -39,3 +39,9 @@ func AddHostRoute(ipn *net.IPNet, gw net.IP, dev netlink.Link) error { Gw: gw, }) } + +// AddDefaultRoute sets the default route on the given gateway. +func AddDefaultRoute(gw net.IP, dev netlink.Link) error { + _, defNet, _ := net.ParseCIDR("0.0.0.0/0") + return AddRoute(defNet, gw, dev) +} diff --git a/pkg/ip/route_unspecified.go b/pkg/ip/route_unspecified.go deleted file mode 100644 index 7e79fdef..00000000 --- a/pkg/ip/route_unspecified.go +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2015-2017 CNI authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// +build !linux - -package ip - -import ( - "net" - - "github.com/containernetworking/cni/pkg/types" - "github.com/vishvananda/netlink" -) - -// AddRoute adds a universally-scoped route to a device. -func AddRoute(ipn *net.IPNet, gw net.IP, dev netlink.Link) error { - return types.NotImplementedError -} - -// AddHostRoute adds a host-scoped route to a device. -func AddHostRoute(ipn *net.IPNet, gw net.IP, dev netlink.Link) error { - return types.NotImplementedError -} diff --git a/pkg/ipam/ipam.go b/pkg/ipam/ipam.go index aa72e5c5..904b2557 100644 --- a/pkg/ipam/ipam.go +++ b/pkg/ipam/ipam.go @@ -15,16 +15,8 @@ package ipam import ( - "fmt" - "net" - "os" - "github.com/containernetworking/cni/pkg/invoke" "github.com/containernetworking/cni/pkg/types" - "github.com/containernetworking/cni/pkg/types/current" - "github.com/containernetworking/plugins/pkg/ip" - - "github.com/vishvananda/netlink" ) func ExecAdd(plugin string, netconf []byte) (types.Result, error) { @@ -34,66 +26,3 @@ func ExecAdd(plugin string, netconf []byte) (types.Result, error) { func ExecDel(plugin string, netconf []byte) error { return invoke.DelegateDel(plugin, netconf) } - -// ConfigureIface takes the result of IPAM plugin and -// applies to the ifName interface -func ConfigureIface(ifName string, res *current.Result) error { - if len(res.Interfaces) == 0 { - return fmt.Errorf("no interfaces to configure") - } - - link, err := netlink.LinkByName(ifName) - if err != nil { - return fmt.Errorf("failed to lookup %q: %v", ifName, err) - } - - if err := netlink.LinkSetUp(link); err != nil { - return fmt.Errorf("failed to set %q UP: %v", ifName, err) - } - - var v4gw, v6gw net.IP - for _, ipc := range res.IPs { - if ipc.Interface == nil { - continue - } - intIdx := *ipc.Interface - if intIdx < 0 || intIdx >= len(res.Interfaces) || res.Interfaces[intIdx].Name != ifName { - // IP address is for a different interface - return fmt.Errorf("failed to add IP addr %v to %q: invalid interface index", ipc, ifName) - } - - addr := &netlink.Addr{IPNet: &ipc.Address, Label: ""} - if err = netlink.AddrAdd(link, addr); err != nil { - return fmt.Errorf("failed to add IP addr %v to %q: %v", ipc, ifName, err) - } - - gwIsV4 := ipc.Gateway.To4() != nil - if gwIsV4 && v4gw == nil { - v4gw = ipc.Gateway - } else if !gwIsV4 && v6gw == nil { - v6gw = ipc.Gateway - } - } - - ip.SettleAddresses(ifName, 10) - - for _, r := range res.Routes { - routeIsV4 := r.Dst.IP.To4() != nil - gw := r.GW - if gw == nil { - if routeIsV4 && v4gw != nil { - gw = v4gw - } else if !routeIsV4 && v6gw != nil { - gw = v6gw - } - } - if err = ip.AddRoute(&r.Dst, gw, link); err != nil { - // we skip over duplicate routes as we assume the first one wins - if !os.IsExist(err) { - return fmt.Errorf("failed to add route '%v via %v dev %v': %v", r.Dst, gw, ifName, err) - } - } - } - - return nil -} diff --git a/pkg/ipam/ipam_linux.go b/pkg/ipam/ipam_linux.go new file mode 100644 index 00000000..6c8ef20c --- /dev/null +++ b/pkg/ipam/ipam_linux.go @@ -0,0 +1,89 @@ +// Copyright 2015 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ipam + +import ( + "fmt" + "net" + "os" + + "github.com/containernetworking/cni/pkg/types/current" + "github.com/containernetworking/plugins/pkg/ip" + + "github.com/vishvananda/netlink" +) + +// ConfigureIface takes the result of IPAM plugin and +// applies to the ifName interface +func ConfigureIface(ifName string, res *current.Result) error { + if len(res.Interfaces) == 0 { + return fmt.Errorf("no interfaces to configure") + } + + link, err := netlink.LinkByName(ifName) + if err != nil { + return fmt.Errorf("failed to lookup %q: %v", ifName, err) + } + + if err := netlink.LinkSetUp(link); err != nil { + return fmt.Errorf("failed to set %q UP: %v", ifName, err) + } + + var v4gw, v6gw net.IP + for _, ipc := range res.IPs { + if ipc.Interface == nil { + continue + } + intIdx := *ipc.Interface + if intIdx < 0 || intIdx >= len(res.Interfaces) || res.Interfaces[intIdx].Name != ifName { + // IP address is for a different interface + return fmt.Errorf("failed to add IP addr %v to %q: invalid interface index", ipc, ifName) + } + + addr := &netlink.Addr{IPNet: &ipc.Address, Label: ""} + if err = netlink.AddrAdd(link, addr); err != nil { + return fmt.Errorf("failed to add IP addr %v to %q: %v", ipc, ifName, err) + } + + gwIsV4 := ipc.Gateway.To4() != nil + if gwIsV4 && v4gw == nil { + v4gw = ipc.Gateway + } else if !gwIsV4 && v6gw == nil { + v6gw = ipc.Gateway + } + } + + ip.SettleAddresses(ifName, 10) + + for _, r := range res.Routes { + routeIsV4 := r.Dst.IP.To4() != nil + gw := r.GW + if gw == nil { + if routeIsV4 && v4gw != nil { + gw = v4gw + } else if !routeIsV4 && v6gw != nil { + gw = v6gw + } + } + if err = ip.AddRoute(&r.Dst, gw, link); err != nil { + // we skip over duplicate routes as we assume the first one wins + if !os.IsExist(err) { + return fmt.Errorf("failed to add route '%v via %v dev %v': %v", r.Dst, gw, ifName, err) + } + } + } + + return nil +} diff --git a/pkg/ipam/ipam_test.go b/pkg/ipam/ipam_linux_test.go similarity index 99% rename from pkg/ipam/ipam_test.go rename to pkg/ipam/ipam_linux_test.go index cf851175..9de3fa05 100644 --- a/pkg/ipam/ipam_test.go +++ b/pkg/ipam/ipam_linux_test.go @@ -39,7 +39,7 @@ func ipNetEqual(a, b *net.IPNet) bool { return a.IP.Equal(b.IP) } -var _ = Describe("IPAM Operations", func() { +var _ = Describe("ConfigureIface", func() { var originalNS ns.NetNS var ipv4, ipv6, routev4, routev6 *net.IPNet var ipgw4, ipgw6, routegwv4, routegwv6 net.IP diff --git a/pkg/ns/ns.go b/pkg/ns/ns.go deleted file mode 100644 index c212f489..00000000 --- a/pkg/ns/ns.go +++ /dev/null @@ -1,178 +0,0 @@ -// Copyright 2015 CNI authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package ns - -import ( - "fmt" - "os" - "runtime" - "sync" - "syscall" -) - -type NetNS interface { - // Executes the passed closure in this object's network namespace, - // attempting to restore the original namespace before returning. - // However, since each OS thread can have a different network namespace, - // and Go's thread scheduling is highly variable, callers cannot - // guarantee any specific namespace is set unless operations that - // require that namespace are wrapped with Do(). Also, no code called - // from Do() should call runtime.UnlockOSThread(), or the risk - // of executing code in an incorrect namespace will be greater. See - // https://github.com/golang/go/wiki/LockOSThread for further details. - Do(toRun func(NetNS) error) error - - // Sets the current network namespace to this object's network namespace. - // Note that since Go's thread scheduling is highly variable, callers - // cannot guarantee the requested namespace will be the current namespace - // after this function is called; to ensure this wrap operations that - // require the namespace with Do() instead. - Set() error - - // Returns the filesystem path representing this object's network namespace - Path() string - - // Returns a file descriptor representing this object's network namespace - Fd() uintptr - - // Cleans up this instance of the network namespace; if this instance - // is the last user the namespace will be destroyed - Close() error -} - -type netNS struct { - file *os.File - mounted bool - closed bool -} - -// netNS implements the NetNS interface -var _ NetNS = &netNS{} - -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 - } - - switch stat.Type { - case PROCFS_MAGIC, NSFS_MAGIC: - 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 { - return nil, err - } - - fd, err := os.Open(nspath) - if err != nil { - return nil, err - } - - return &netNS{file: fd}, nil -} - -func (ns *netNS) Path() string { - return ns.file.Name() -} - -func (ns *netNS) Fd() uintptr { - return ns.file.Fd() -} - -func (ns *netNS) errorIfClosed() error { - if ns.closed { - return fmt.Errorf("%q has already been closed", ns.file.Name()) - } - return nil -} - -func (ns *netNS) Do(toRun func(NetNS) error) error { - if err := ns.errorIfClosed(); err != nil { - return err - } - - containedCall := func(hostNS NetNS) error { - threadNS, err := GetCurrentNS() - if err != nil { - return fmt.Errorf("failed to open current netns: %v", err) - } - defer threadNS.Close() - - // switch to target namespace - if err = ns.Set(); err != nil { - return fmt.Errorf("error switching to ns %v: %v", ns.file.Name(), err) - } - defer threadNS.Set() // switch back - - return toRun(hostNS) - } - - // save a handle to current network namespace - hostNS, err := GetCurrentNS() - if err != nil { - return fmt.Errorf("Failed to open current namespace: %v", err) - } - defer hostNS.Close() - - var wg sync.WaitGroup - wg.Add(1) - - var innerError error - go func() { - defer wg.Done() - runtime.LockOSThread() - innerError = containedCall(hostNS) - }() - wg.Wait() - - return innerError -} - -// WithNetNSPath executes the passed closure under the given network -// namespace, restoring the original namespace afterwards. -func WithNetNSPath(nspath string, toRun func(NetNS) error) error { - ns, err := GetNS(nspath) - if err != nil { - return err - } - defer ns.Close() - return ns.Do(toRun) -} diff --git a/pkg/ns/ns_linux.go b/pkg/ns/ns_linux.go index 8949d21b..4ce98946 100644 --- a/pkg/ns/ns_linux.go +++ b/pkg/ns/ns_linux.go @@ -21,6 +21,7 @@ import ( "path" "runtime" "sync" + "syscall" "golang.org/x/sys/unix" ) @@ -147,3 +148,158 @@ func (ns *netNS) Set() error { return nil } + +type NetNS interface { + // Executes the passed closure in this object's network namespace, + // attempting to restore the original namespace before returning. + // However, since each OS thread can have a different network namespace, + // and Go's thread scheduling is highly variable, callers cannot + // guarantee any specific namespace is set unless operations that + // require that namespace are wrapped with Do(). Also, no code called + // from Do() should call runtime.UnlockOSThread(), or the risk + // of executing code in an incorrect namespace will be greater. See + // https://github.com/golang/go/wiki/LockOSThread for further details. + Do(toRun func(NetNS) error) error + + // Sets the current network namespace to this object's network namespace. + // Note that since Go's thread scheduling is highly variable, callers + // cannot guarantee the requested namespace will be the current namespace + // after this function is called; to ensure this wrap operations that + // require the namespace with Do() instead. + Set() error + + // Returns the filesystem path representing this object's network namespace + Path() string + + // Returns a file descriptor representing this object's network namespace + Fd() uintptr + + // Cleans up this instance of the network namespace; if this instance + // is the last user the namespace will be destroyed + Close() error +} + +type netNS struct { + file *os.File + mounted bool + closed bool +} + +// netNS implements the NetNS interface +var _ NetNS = &netNS{} + +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 + } + + switch stat.Type { + case PROCFS_MAGIC, NSFS_MAGIC: + 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 { + return nil, err + } + + fd, err := os.Open(nspath) + if err != nil { + return nil, err + } + + return &netNS{file: fd}, nil +} + +func (ns *netNS) Path() string { + return ns.file.Name() +} + +func (ns *netNS) Fd() uintptr { + return ns.file.Fd() +} + +func (ns *netNS) errorIfClosed() error { + if ns.closed { + return fmt.Errorf("%q has already been closed", ns.file.Name()) + } + return nil +} + +func (ns *netNS) Do(toRun func(NetNS) error) error { + if err := ns.errorIfClosed(); err != nil { + return err + } + + containedCall := func(hostNS NetNS) error { + threadNS, err := GetCurrentNS() + if err != nil { + return fmt.Errorf("failed to open current netns: %v", err) + } + defer threadNS.Close() + + // switch to target namespace + if err = ns.Set(); err != nil { + return fmt.Errorf("error switching to ns %v: %v", ns.file.Name(), err) + } + defer threadNS.Set() // switch back + + return toRun(hostNS) + } + + // save a handle to current network namespace + hostNS, err := GetCurrentNS() + if err != nil { + return fmt.Errorf("Failed to open current namespace: %v", err) + } + defer hostNS.Close() + + var wg sync.WaitGroup + wg.Add(1) + + var innerError error + go func() { + defer wg.Done() + runtime.LockOSThread() + innerError = containedCall(hostNS) + }() + wg.Wait() + + return innerError +} + +// WithNetNSPath executes the passed closure under the given network +// namespace, restoring the original namespace afterwards. +func WithNetNSPath(nspath string, toRun func(NetNS) error) error { + ns, err := GetNS(nspath) + if err != nil { + return err + } + defer ns.Close() + return ns.Do(toRun) +} diff --git a/pkg/ns/ns_test.go b/pkg/ns/ns_linux_test.go similarity index 100% rename from pkg/ns/ns_test.go rename to pkg/ns/ns_linux_test.go diff --git a/pkg/ns/ns_unspecified.go b/pkg/ns/ns_unspecified.go deleted file mode 100644 index 41b44686..00000000 --- a/pkg/ns/ns_unspecified.go +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2015-2017 CNI authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// +build !linux - -package ns - -import "github.com/containernetworking/cni/pkg/types" - -// Returns an object representing the current OS thread's network namespace -func GetCurrentNS() (NetNS, error) { - return nil, types.NotImplementedError -} - -func NewNS() (NetNS, error) { - return nil, types.NotImplementedError -} - -func (ns *netNS) Close() error { - return types.NotImplementedError -} - -func (ns *netNS) Set() error { - return types.NotImplementedError -} diff --git a/pkg/testutils/echosvr/echosvr_test.go b/pkg/testutils/echosvr/echosvr_test.go index 6a4c3bf8..901febd0 100644 --- a/pkg/testutils/echosvr/echosvr_test.go +++ b/pkg/testutils/echosvr/echosvr_test.go @@ -37,7 +37,7 @@ var _ = Describe("Echosvr", func() { }) AfterEach(func() { - session.Terminate().Wait() + session.Kill().Wait() }) It("starts and doesn't terminate immediately", func() { diff --git a/plugins/ipam/host-local/backend/disk/lock.go b/plugins/ipam/host-local/backend/disk/lock.go index 72414825..bd4fce27 100644 --- a/plugins/ipam/host-local/backend/disk/lock.go +++ b/plugins/ipam/host-local/backend/disk/lock.go @@ -1,3 +1,4 @@ +// +build !windows // Copyright 2015 CNI authors // // Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/plugins/linux_only.txt b/plugins/linux_only.txt new file mode 100644 index 00000000..f4904b38 --- /dev/null +++ b/plugins/linux_only.txt @@ -0,0 +1,11 @@ +plugins/host-device +plugins/ipam/dhcp +plugins/ipam/host-local +plugins/main/bridge +plugins/main/ipvlan +plugins/main/loopback +plugins/main/macvlan +plugins/main/ptp +plugins/main/vlan +plugins/meta/portmap +plugins/meta/tuning diff --git a/plugins/meta/flannel/flannel_test.go b/plugins/meta/flannel/flannel_linux_test.go similarity index 100% rename from plugins/meta/flannel/flannel_test.go rename to plugins/meta/flannel/flannel_linux_test.go diff --git a/plugins/sample/sample_test.go b/plugins/sample/sample_linux_test.go similarity index 100% rename from plugins/sample/sample_test.go rename to plugins/sample/sample_linux_test.go From 47668f6d6450f1efe04c7ee51e00ec196ea73f76 Mon Sep 17 00:00:00 2001 From: Rakesh Kelkar Date: Fri, 6 Oct 2017 19:00:19 -0700 Subject: [PATCH 04/14] host-local: Update host-local IPAM to support Windows --- Godeps/Godeps.json | 4 + .../ipam/host-local/backend/disk/backend.go | 17 +- .../backend/disk/disk_suite_test.go | 27 ++ plugins/ipam/host-local/backend/disk/lock.go | 24 +- .../ipam/host-local/backend/disk/lock_test.go | 63 ++++ plugins/ipam/host-local/host_local_test.go | 294 +++++++++--------- plugins/linux_only.txt | 1 - .../github.com/alexflint/go-filemutex/LICENSE | 21 ++ .../alexflint/go-filemutex/README.md | 31 ++ .../alexflint/go-filemutex/filemutex_flock.go | 67 ++++ .../go-filemutex/filemutex_windows.go | 102 ++++++ 11 files changed, 496 insertions(+), 155 deletions(-) create mode 100644 plugins/ipam/host-local/backend/disk/disk_suite_test.go create mode 100644 plugins/ipam/host-local/backend/disk/lock_test.go create mode 100644 vendor/github.com/alexflint/go-filemutex/LICENSE create mode 100644 vendor/github.com/alexflint/go-filemutex/README.md create mode 100644 vendor/github.com/alexflint/go-filemutex/filemutex_flock.go create mode 100644 vendor/github.com/alexflint/go-filemutex/filemutex_windows.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index bcc07be2..4951496c 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -6,6 +6,10 @@ "./..." ], "Deps": [ + { + "ImportPath": "github.com/alexflint/go-filemutex", + "Rev": "72bdc8eae2aef913234599b837f5dda445ca9bd9" + }, { "ImportPath": "github.com/containernetworking/cni/libcni", "Comment": "v0.6.0-rc1", diff --git a/plugins/ipam/host-local/backend/disk/backend.go b/plugins/ipam/host-local/backend/disk/backend.go index 0f5a5f53..08bb4eb9 100644 --- a/plugins/ipam/host-local/backend/disk/backend.go +++ b/plugins/ipam/host-local/backend/disk/backend.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/containernetworking/plugins/plugins/ipam/host-local/backend" + "runtime" ) const lastIPFilePrefix = "last_reserved_ip." @@ -55,7 +56,8 @@ func New(network, dataDir string) (*Store, error) { } func (s *Store) Reserve(id string, ip net.IP, rangeID string) (bool, error) { - fname := filepath.Join(s.dataDir, ip.String()) + fname := GetEscapedPath(s.dataDir, ip.String()) + f, err := os.OpenFile(fname, os.O_RDWR|os.O_EXCL|os.O_CREATE, 0644) if os.IsExist(err) { return false, nil @@ -73,7 +75,7 @@ func (s *Store) Reserve(id string, ip net.IP, rangeID string) (bool, error) { return false, err } // store the reserved ip in lastIPFile - ipfile := filepath.Join(s.dataDir, lastIPFilePrefix+rangeID) + ipfile := GetEscapedPath(s.dataDir, lastIPFilePrefix+rangeID) err = ioutil.WriteFile(ipfile, []byte(ip.String()), 0644) if err != nil { return false, err @@ -83,7 +85,7 @@ func (s *Store) Reserve(id string, ip net.IP, rangeID string) (bool, error) { // LastReservedIP returns the last reserved IP if exists func (s *Store) LastReservedIP(rangeID string) (net.IP, error) { - ipfile := filepath.Join(s.dataDir, lastIPFilePrefix+rangeID) + ipfile := GetEscapedPath(s.dataDir, lastIPFilePrefix+rangeID) data, err := ioutil.ReadFile(ipfile) if err != nil { return nil, err @@ -92,7 +94,7 @@ func (s *Store) LastReservedIP(rangeID string) (net.IP, error) { } func (s *Store) Release(ip net.IP) error { - return os.Remove(filepath.Join(s.dataDir, ip.String())) + return os.Remove(GetEscapedPath(s.dataDir, ip.String())) } // N.B. This function eats errors to be tolerant and @@ -115,3 +117,10 @@ func (s *Store) ReleaseByID(id string) error { }) return err } + +func GetEscapedPath(dataDir string, fname string) string { + if runtime.GOOS == "windows" { + fname = strings.Replace(fname, ":", "_", -1) + } + return filepath.Join(dataDir, fname) +} diff --git a/plugins/ipam/host-local/backend/disk/disk_suite_test.go b/plugins/ipam/host-local/backend/disk/disk_suite_test.go new file mode 100644 index 00000000..ffdd2f22 --- /dev/null +++ b/plugins/ipam/host-local/backend/disk/disk_suite_test.go @@ -0,0 +1,27 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package disk + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "testing" +) + +func TestLock(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Disk Suite") +} diff --git a/plugins/ipam/host-local/backend/disk/lock.go b/plugins/ipam/host-local/backend/disk/lock.go index bd4fce27..fe7b4803 100644 --- a/plugins/ipam/host-local/backend/disk/lock.go +++ b/plugins/ipam/host-local/backend/disk/lock.go @@ -1,4 +1,3 @@ -// +build !windows // Copyright 2015 CNI authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -16,18 +15,28 @@ package disk import ( + "github.com/alexflint/go-filemutex" "os" - "syscall" + "path" ) // FileLock wraps os.File to be used as a lock using flock type FileLock struct { - f *os.File + f *filemutex.FileMutex } // NewFileLock opens file/dir at path and returns unlocked FileLock object -func NewFileLock(path string) (*FileLock, error) { - f, err := os.Open(path) +func NewFileLock(lockPath string) (*FileLock, error) { + fi, err := os.Stat(lockPath) + if err != nil { + return nil, err + } + + if fi.IsDir() { + lockPath = path.Join(lockPath, "lock") + } + + f, err := filemutex.New(lockPath) if err != nil { return nil, err } @@ -35,17 +44,16 @@ func NewFileLock(path string) (*FileLock, error) { return &FileLock{f}, nil } -// Close closes underlying file func (l *FileLock) Close() error { return l.f.Close() } // Lock acquires an exclusive lock func (l *FileLock) Lock() error { - return syscall.Flock(int(l.f.Fd()), syscall.LOCK_EX) + return l.f.Lock() } // Unlock releases the lock func (l *FileLock) Unlock() error { - return syscall.Flock(int(l.f.Fd()), syscall.LOCK_UN) + return l.f.Unlock() } diff --git a/plugins/ipam/host-local/backend/disk/lock_test.go b/plugins/ipam/host-local/backend/disk/lock_test.go new file mode 100644 index 00000000..f76dafb6 --- /dev/null +++ b/plugins/ipam/host-local/backend/disk/lock_test.go @@ -0,0 +1,63 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package disk + +import ( + "io/ioutil" + "os" + "path/filepath" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Lock Operations", func() { + It("locks a file path", func() { + dir, err := ioutil.TempDir("", "") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(dir) + + // create a dummy file to lock + path := filepath.Join(dir, "x") + f, err := os.OpenFile(path, os.O_RDONLY|os.O_CREATE, 0666) + Expect(err).ToNot(HaveOccurred()) + err = f.Close() + Expect(err).ToNot(HaveOccurred()) + + // now use it to lock + m, err := NewFileLock(path) + Expect(err).ToNot(HaveOccurred()) + + err = m.Lock() + Expect(err).ToNot(HaveOccurred()) + err = m.Unlock() + Expect(err).ToNot(HaveOccurred()) + }) + + It("locks a folder path", func() { + dir, err := ioutil.TempDir("", "") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(dir) + + // use the folder to lock + m, err := NewFileLock(dir) + Expect(err).ToNot(HaveOccurred()) + + err = m.Lock() + Expect(err).ToNot(HaveOccurred()) + err = m.Unlock() + Expect(err).ToNot(HaveOccurred()) + }) +}) diff --git a/plugins/ipam/host-local/host_local_test.go b/plugins/ipam/host-local/host_local_test.go index 3e4b0d86..15c63651 100644 --- a/plugins/ipam/host-local/host_local_test.go +++ b/plugins/ipam/host-local/host_local_test.go @@ -28,6 +28,7 @@ import ( "github.com/containernetworking/cni/pkg/types/current" "github.com/containernetworking/plugins/pkg/testutils" + "github.com/containernetworking/plugins/plugins/ipam/host-local/backend/disk" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -37,7 +38,7 @@ var _ = Describe("host-local Operations", func() { const ifname string = "eth0" const nspath string = "/some/where" - tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + tmpDir, err := getTmpDir() Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(tmpDir) @@ -45,26 +46,26 @@ var _ = Describe("host-local Operations", func() { Expect(err).NotTo(HaveOccurred()) conf := fmt.Sprintf(`{ -"cniVersion": "0.3.1", -"name": "mynet", -"type": "ipvlan", -"master": "foo0", - "ipam": { - "type": "host-local", - "dataDir": "%s", - "resolvConf": "%s/resolv.conf", - "ranges": [ - [{ "subnet": "10.1.2.0/24" }, {"subnet": "10.2.2.0/24"}], - [{ "subnet": "2001:db8:1::0/64" }] - ], - "routes": [ - {"dst": "0.0.0.0/0"}, - {"dst": "::/0"}, - {"dst": "192.168.0.0/16", "gw": "1.1.1.1"}, - {"dst": "2001:db8:2::0/64", "gw": "2001:db8:3::1"} - ] - } -}`, tmpDir, tmpDir) + "cniVersion": "0.3.1", + "name": "mynet", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "dataDir": "%s", + "resolvConf": "%s/resolv.conf", + "ranges": [ + [{ "subnet": "10.1.2.0/24" }, {"subnet": "10.2.2.0/24"}], + [{ "subnet": "2001:db8:1::0/64" }] + ], + "routes": [ + {"dst": "0.0.0.0/0"}, + {"dst": "::/0"}, + {"dst": "192.168.0.0/16", "gw": "1.1.1.1"}, + {"dst": "2001:db8:2::0/64", "gw": "2001:db8:3::1"} + ] + } + }`, tmpDir, tmpDir) args := &skel.CmdArgs{ ContainerID: "dummy", @@ -112,7 +113,7 @@ var _ = Describe("host-local Operations", func() { Expect(err).NotTo(HaveOccurred()) Expect(string(contents)).To(Equal("dummy")) - ipFilePath2 := filepath.Join(tmpDir, "mynet", "2001:db8:1::2") + ipFilePath2 := filepath.Join(tmpDir, disk.GetEscapedPath("mynet", "2001:db8:1::2")) contents, err = ioutil.ReadFile(ipFilePath2) Expect(err).NotTo(HaveOccurred()) Expect(string(contents)).To(Equal("dummy")) @@ -142,21 +143,21 @@ var _ = Describe("host-local Operations", func() { const ifname string = "eth0" const nspath string = "/some/where" - tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + tmpDir, err := getTmpDir() Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(tmpDir) conf := fmt.Sprintf(`{ - "cniVersion": "0.3.0", - "name": "mynet", - "type": "ipvlan", - "master": "foo0", - "ipam": { - "type": "host-local", - "subnet": "10.1.2.0/24", - "dataDir": "%s" - } -}`, tmpDir) + "cniVersion": "0.3.0", + "name": "mynet", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "subnet": "10.1.2.0/24", + "dataDir": "%s" + } + }`, tmpDir) args := &skel.CmdArgs{ ContainerID: "dummy", @@ -176,7 +177,7 @@ var _ = Describe("host-local Operations", func() { const ifname string = "eth0" const nspath string = "/some/where" - tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + tmpDir, err := getTmpDir() Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(tmpDir) @@ -184,17 +185,17 @@ var _ = Describe("host-local Operations", func() { Expect(err).NotTo(HaveOccurred()) conf := fmt.Sprintf(`{ - "cniVersion": "0.1.0", - "name": "mynet", - "type": "ipvlan", - "master": "foo0", - "ipam": { - "type": "host-local", - "subnet": "10.1.2.0/24", - "dataDir": "%s", - "resolvConf": "%s/resolv.conf" - } -}`, tmpDir, tmpDir) + "cniVersion": "0.1.0", + "name": "mynet", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "subnet": "10.1.2.0/24", + "dataDir": "%s", + "resolvConf": "%s/resolv.conf" + } + }`, tmpDir, tmpDir) args := &skel.CmdArgs{ ContainerID: "dummy", @@ -245,21 +246,21 @@ var _ = Describe("host-local Operations", func() { const ifname string = "eth0" const nspath string = "/some/where" - tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + tmpDir, err := getTmpDir() Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(tmpDir) conf := fmt.Sprintf(`{ - "cniVersion": "0.3.1", - "name": "mynet", - "type": "ipvlan", - "master": "foo0", - "ipam": { - "type": "host-local", - "subnet": "10.1.2.0/24", - "dataDir": "%s" - } -}`, tmpDir) + "cniVersion": "0.3.1", + "name": "mynet", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "subnet": "10.1.2.0/24", + "dataDir": "%s" + } + }`, tmpDir) args := &skel.CmdArgs{ ContainerID: " dummy\n ", @@ -296,21 +297,21 @@ var _ = Describe("host-local Operations", func() { const ifname string = "eth0" const nspath string = "/some/where" - tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + tmpDir, err := getTmpDir() Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(tmpDir) conf := fmt.Sprintf(`{ - "cniVersion": "0.2.0", - "name": "mynet", - "type": "ipvlan", - "master": "foo0", - "ipam": { - "type": "host-local", - "subnet": "10.1.2.0/24", - "dataDir": "%s" - } -}`, tmpDir) + "cniVersion": "0.2.0", + "name": "mynet", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "subnet": "10.1.2.0/24", + "dataDir": "%s" + } + }`, tmpDir) args := &skel.CmdArgs{ ContainerID: "testing", @@ -331,28 +332,28 @@ var _ = Describe("host-local Operations", func() { const ifname string = "eth0" const nspath string = "/some/where" - tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + tmpDir, err := getTmpDir() Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(tmpDir) conf := fmt.Sprintf(`{ - "cniVersion": "0.3.1", - "name": "mynet", - "type": "ipvlan", - "master": "foo0", - "ipam": { - "type": "host-local", - "dataDir": "%s", - "ranges": [ - [{ "subnet": "10.1.2.0/24" }] - ] - }, - "args": { - "cni": { - "ips": ["10.1.2.88"] - } - } -}`, tmpDir) + "cniVersion": "0.3.1", + "name": "mynet", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "dataDir": "%s", + "ranges": [ + [{ "subnet": "10.1.2.0/24" }] + ] + }, + "args": { + "cni": { + "ips": ["10.1.2.88"] + } + } + }`, tmpDir) args := &skel.CmdArgs{ ContainerID: "dummy", @@ -376,7 +377,7 @@ var _ = Describe("host-local Operations", func() { const ifname string = "eth0" const nspath string = "/some/where" - tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + tmpDir, err := getTmpDir() Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(tmpDir) @@ -384,24 +385,24 @@ var _ = Describe("host-local Operations", func() { Expect(err).NotTo(HaveOccurred()) conf := fmt.Sprintf(`{ - "cniVersion": "0.3.1", - "name": "mynet", - "type": "ipvlan", - "master": "foo0", - "ipam": { - "type": "host-local", - "dataDir": "%s", - "ranges": [ - [{ "subnet": "10.1.2.0/24" }], - [{ "subnet": "10.1.3.0/24" }] - ] - }, - "args": { - "cni": { - "ips": ["10.1.2.88", "10.1.3.77"] - } - } -}`, tmpDir) + "cniVersion": "0.3.1", + "name": "mynet", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "dataDir": "%s", + "ranges": [ + [{ "subnet": "10.1.2.0/24" }], + [{ "subnet": "10.1.3.0/24" }] + ] + }, + "args": { + "cni": { + "ips": ["10.1.2.88", "10.1.3.77"] + } + } + }`, tmpDir) args := &skel.CmdArgs{ ContainerID: "dummy", @@ -426,7 +427,7 @@ var _ = Describe("host-local Operations", func() { const ifname string = "eth0" const nspath string = "/some/where" - tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + tmpDir, err := getTmpDir() Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(tmpDir) @@ -434,24 +435,24 @@ var _ = Describe("host-local Operations", func() { Expect(err).NotTo(HaveOccurred()) conf := fmt.Sprintf(`{ - "cniVersion": "0.3.1", - "name": "mynet", - "type": "ipvlan", - "master": "foo0", - "ipam": { - "type": "host-local", - "dataDir": "%s", - "ranges": [ - [{"subnet":"172.16.1.0/24"}, { "subnet": "10.1.2.0/24" }], - [{ "subnet": "2001:db8:1::/24" }] - ] - }, - "args": { - "cni": { - "ips": ["10.1.2.88", "2001:db8:1::999"] - } - } -}`, tmpDir) + "cniVersion": "0.3.1", + "name": "mynet", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "dataDir": "%s", + "ranges": [ + [{"subnet":"172.16.1.0/24"}, { "subnet": "10.1.2.0/24" }], + [{ "subnet": "2001:db8:1::/24" }] + ] + }, + "args": { + "cni": { + "ips": ["10.1.2.88", "2001:db8:1::999"] + } + } + }`, tmpDir) args := &skel.CmdArgs{ ContainerID: "dummy", @@ -476,29 +477,29 @@ var _ = Describe("host-local Operations", func() { const ifname string = "eth0" const nspath string = "/some/where" - tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + tmpDir, err := getTmpDir() Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(tmpDir) conf := fmt.Sprintf(`{ - "cniVersion": "0.3.1", - "name": "mynet", - "type": "ipvlan", - "master": "foo0", - "ipam": { - "type": "host-local", - "dataDir": "%s", - "ranges": [ - [{ "subnet": "10.1.2.0/24" }], - [{ "subnet": "10.1.3.0/24" }] - ] - }, - "args": { - "cni": { - "ips": ["10.1.2.88", "10.1.2.77"] - } - } -}`, tmpDir) + "cniVersion": "0.3.1", + "name": "mynet", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "dataDir": "%s", + "ranges": [ + [{ "subnet": "10.1.2.0/24" }], + [{ "subnet": "10.1.3.0/24" }] + ] + }, + "args": { + "cni": { + "ips": ["10.1.2.88", "10.1.2.77"] + } + } + }`, tmpDir) args := &skel.CmdArgs{ ContainerID: "dummy", @@ -517,6 +518,15 @@ var _ = Describe("host-local Operations", func() { }) }) +func getTmpDir() (string, error) { + tmpDir, err := ioutil.TempDir("", "host_local_artifacts") + if err == nil { + tmpDir = filepath.ToSlash(tmpDir) + } + + return tmpDir, err +} + func mustCIDR(s string) net.IPNet { ip, n, err := net.ParseCIDR(s) n.IP = ip diff --git a/plugins/linux_only.txt b/plugins/linux_only.txt index f4904b38..e9b5948d 100644 --- a/plugins/linux_only.txt +++ b/plugins/linux_only.txt @@ -1,6 +1,5 @@ plugins/host-device plugins/ipam/dhcp -plugins/ipam/host-local plugins/main/bridge plugins/main/ipvlan plugins/main/loopback diff --git a/vendor/github.com/alexflint/go-filemutex/LICENSE b/vendor/github.com/alexflint/go-filemutex/LICENSE new file mode 100644 index 00000000..b48c6735 --- /dev/null +++ b/vendor/github.com/alexflint/go-filemutex/LICENSE @@ -0,0 +1,21 @@ +The MIT License + +Copyright (c) 2010-2017 Alex Flint. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/vendor/github.com/alexflint/go-filemutex/README.md b/vendor/github.com/alexflint/go-filemutex/README.md new file mode 100644 index 00000000..30d05ff5 --- /dev/null +++ b/vendor/github.com/alexflint/go-filemutex/README.md @@ -0,0 +1,31 @@ +# FileMutex + +FileMutex is similar to `sync.RWMutex`, but also synchronizes across processes. +On Linux, OSX, and other POSIX systems it uses the flock system call. On windows +it uses the LockFileEx and UnlockFileEx system calls. + +```go +import ( + "log" + "github.com/alexflint/go-filemutex" +) + +func main() { + m, err := filemutex.New("/tmp/foo.lock") + if err != nil { + log.Fatalln("Directory did not exist or file could not created") + } + + m.Lock() // Will block until lock can be acquired + + // Code here is protected by the mutex + + m.Unlock() +} +``` + +### Installation + + go get github.com/alexflint/go-filemutex + +Forked from https://github.com/golang/build/tree/master/cmd/builder/filemutex_*.go diff --git a/vendor/github.com/alexflint/go-filemutex/filemutex_flock.go b/vendor/github.com/alexflint/go-filemutex/filemutex_flock.go new file mode 100644 index 00000000..2bb77520 --- /dev/null +++ b/vendor/github.com/alexflint/go-filemutex/filemutex_flock.go @@ -0,0 +1,67 @@ +// Copyright 2013 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build darwin dragonfly freebsd linux netbsd openbsd + +package filemutex + +import ( + "syscall" +) + +const ( + mkdirPerm = 0750 +) + +// FileMutex is similar to sync.RWMutex, but also synchronizes across processes. +// This implementation is based on flock syscall. +type FileMutex struct { + fd int +} + +func New(filename string) (*FileMutex, error) { + fd, err := syscall.Open(filename, syscall.O_CREAT|syscall.O_RDONLY, mkdirPerm) + if err != nil { + return nil, err + } + return &FileMutex{fd: fd}, nil +} + +func (m *FileMutex) Lock() error { + if err := syscall.Flock(m.fd, syscall.LOCK_EX); err != nil { + return err + } + return nil +} + +func (m *FileMutex) Unlock() error { + if err := syscall.Flock(m.fd, syscall.LOCK_UN); err != nil { + return err + } + return nil +} + +func (m *FileMutex) RLock() error { + if err := syscall.Flock(m.fd, syscall.LOCK_SH); err != nil { + return err + } + return nil +} + +func (m *FileMutex) RUnlock() error { + if err := syscall.Flock(m.fd, syscall.LOCK_UN); err != nil { + return err + } + return nil +} + +// Close does an Unlock() combined with closing and unlinking the associated +// lock file. You should create a New() FileMutex for every Lock() attempt if +// using Close(). +func (m *FileMutex) Close() error { + if err := syscall.Flock(m.fd, syscall.LOCK_UN); err != nil { + return err + } + return syscall.Close(m.fd) +} diff --git a/vendor/github.com/alexflint/go-filemutex/filemutex_windows.go b/vendor/github.com/alexflint/go-filemutex/filemutex_windows.go new file mode 100644 index 00000000..28797d26 --- /dev/null +++ b/vendor/github.com/alexflint/go-filemutex/filemutex_windows.go @@ -0,0 +1,102 @@ +// Copyright 2013 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package filemutex + +import ( + "syscall" + "unsafe" +) + +var ( + modkernel32 = syscall.NewLazyDLL("kernel32.dll") + procLockFileEx = modkernel32.NewProc("LockFileEx") + procUnlockFileEx = modkernel32.NewProc("UnlockFileEx") +) + +const ( + lockfileExclusiveLock = 2 +) + +func lockFileEx(h syscall.Handle, flags, reserved, locklow, lockhigh uint32, ol *syscall.Overlapped) (err error) { + r1, _, e1 := syscall.Syscall6(procLockFileEx.Addr(), 6, uintptr(h), uintptr(flags), uintptr(reserved), uintptr(locklow), uintptr(lockhigh), uintptr(unsafe.Pointer(ol))) + if r1 == 0 { + if e1 != 0 { + err = error(e1) + } else { + err = syscall.EINVAL + } + } + return +} + +func unlockFileEx(h syscall.Handle, reserved, locklow, lockhigh uint32, ol *syscall.Overlapped) (err error) { + r1, _, e1 := syscall.Syscall6(procUnlockFileEx.Addr(), 5, uintptr(h), uintptr(reserved), uintptr(locklow), uintptr(lockhigh), uintptr(unsafe.Pointer(ol)), 0) + if r1 == 0 { + if e1 != 0 { + err = error(e1) + } else { + err = syscall.EINVAL + } + } + return +} + +// FileMutex is similar to sync.RWMutex, but also synchronizes across processes. +// This implementation is based on flock syscall. +type FileMutex struct { + fd syscall.Handle +} + +func New(filename string) (*FileMutex, error) { + fd, err := syscall.CreateFile(&(syscall.StringToUTF16(filename)[0]), syscall.GENERIC_READ|syscall.GENERIC_WRITE, + syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE, nil, syscall.OPEN_ALWAYS, syscall.FILE_ATTRIBUTE_NORMAL, 0) + if err != nil { + return nil, err + } + return &FileMutex{fd: fd}, nil +} + +func (m *FileMutex) Lock() error { + var ol syscall.Overlapped + if err := lockFileEx(m.fd, lockfileExclusiveLock, 0, 1, 0, &ol); err != nil { + return err + } + return nil +} + +func (m *FileMutex) Unlock() error { + var ol syscall.Overlapped + if err := unlockFileEx(m.fd, 0, 1, 0, &ol); err != nil { + return err + } + return nil +} + +func (m *FileMutex) RLock() error { + var ol syscall.Overlapped + if err := lockFileEx(m.fd, 0, 0, 1, 0, &ol); err != nil { + return err + } + return nil +} + +func (m *FileMutex) RUnlock() error { + var ol syscall.Overlapped + if err := unlockFileEx(m.fd, 0, 1, 0, &ol); err != nil { + return err + } + return nil +} + +// Close does an Unlock() combined with closing and unlinking the associated +// lock file. You should create a New() FileMutex for every Lock() attempt if +// using Close(). +func (m *FileMutex) Close() error { + var ol syscall.Overlapped + if err := unlockFileEx(m.fd, 0, 1, 0, &ol); err != nil { + return err + } + return syscall.Close(m.fd) +} From d07d2aaf71c215654545a34691e82a5e17fb259f Mon Sep 17 00:00:00 2001 From: Casey Callendrello Date: Mon, 13 Nov 2017 18:50:20 +0100 Subject: [PATCH 05/14] plugins/host_device: move to "main" folder --- plugins/linux_only.txt | 2 +- plugins/{ => main}/host-device/host-device.go | 0 plugins/{ => main}/host-device/host-device_suite_test.go | 0 plugins/{ => main}/host-device/host-device_test.go | 0 4 files changed, 1 insertion(+), 1 deletion(-) rename plugins/{ => main}/host-device/host-device.go (100%) rename plugins/{ => main}/host-device/host-device_suite_test.go (100%) rename plugins/{ => main}/host-device/host-device_test.go (100%) diff --git a/plugins/linux_only.txt b/plugins/linux_only.txt index e9b5948d..c644e20f 100644 --- a/plugins/linux_only.txt +++ b/plugins/linux_only.txt @@ -1,6 +1,6 @@ -plugins/host-device plugins/ipam/dhcp plugins/main/bridge +plugins/main/host-device plugins/main/ipvlan plugins/main/loopback plugins/main/macvlan diff --git a/plugins/host-device/host-device.go b/plugins/main/host-device/host-device.go similarity index 100% rename from plugins/host-device/host-device.go rename to plugins/main/host-device/host-device.go diff --git a/plugins/host-device/host-device_suite_test.go b/plugins/main/host-device/host-device_suite_test.go similarity index 100% rename from plugins/host-device/host-device_suite_test.go rename to plugins/main/host-device/host-device_suite_test.go diff --git a/plugins/host-device/host-device_test.go b/plugins/main/host-device/host-device_test.go similarity index 100% rename from plugins/host-device/host-device_test.go rename to plugins/main/host-device/host-device_test.go From 5e830efb206bd1636dbd13daed1563a1ffe04e28 Mon Sep 17 00:00:00 2001 From: Casey Callendrello Date: Wed, 15 Nov 2017 16:00:32 +0100 Subject: [PATCH 06/14] plugins/main/host-local: generate result, fix DEL, other cleanups This plugin needed some cleaning up: it didn't generate output, and didn't test DEL. Add those things, plus a README. --- plugins/main/host-device/README.md | 21 +++++++ plugins/main/host-device/host-device.go | 61 +++++++++++++++----- plugins/main/host-device/host-device_test.go | 36 +++++++++++- 3 files changed, 99 insertions(+), 19 deletions(-) create mode 100644 plugins/main/host-device/README.md diff --git a/plugins/main/host-device/README.md b/plugins/main/host-device/README.md new file mode 100644 index 00000000..1e0af84a --- /dev/null +++ b/plugins/main/host-device/README.md @@ -0,0 +1,21 @@ +# host-device +Move an already-existing device in to a container. + +This simple plugin will move the requested device from the host's network namespace +to the container's. Nothing else will be done - no IPAM, no addresses. + +The device can be specified with any one of three properties: +* `device`: The device name, e.g. `eth0`, `can0` +* `hwaddr`: A MAC address +* `kernelpath`: The kernel device kobj, e.g. `/sys/devices/pci0000:00/0000:00:1f.6` + +For this plugin, `CNI_IFNAME` will be ignored. Upon DEL, the device will be moved back. + +A sample configuration might look like: + +```json +{ + "cniVersion": "0.3.1", + "device": "enp0s1" +} +``` diff --git a/plugins/main/host-device/host-device.go b/plugins/main/host-device/host-device.go index 954b7778..8f3d482a 100644 --- a/plugins/main/host-device/host-device.go +++ b/plugins/main/host-device/host-device.go @@ -25,6 +25,7 @@ import ( "strings" "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/types/current" "github.com/containernetworking/cni/pkg/version" "github.com/containernetworking/plugins/pkg/ns" @@ -32,6 +33,7 @@ import ( ) type NetConf struct { + types.NetConf Device string `json:"device"` // Device-Name, something like eth0 or can0 etc. HWAddr string `json:"hwaddr"` // MAC Address of target network interface KernelPath string `json:"kernelpath"` // Kernelpath of the device @@ -65,8 +67,12 @@ func cmdAdd(args *skel.CmdArgs) error { return fmt.Errorf("failed to open netns %q: %v", args.Netns, err) } defer containerNs.Close() - defer (¤t.Result{}).Print() - return addLink(cfg.Device, cfg.HWAddr, cfg.KernelPath, containerNs) + + dev, err := moveLinkIn(cfg.Device, cfg.HWAddr, cfg.KernelPath, containerNs) + if err != nil { + return fmt.Errorf("failed to move link %v", err) + } + return printLink(dev, cfg.CNIVersion, containerNs) } func cmdDel(args *skel.CmdArgs) error { @@ -80,35 +86,58 @@ func cmdDel(args *skel.CmdArgs) error { } defer containerNs.Close() defer fmt.Println(`{}`) - return removeLink(cfg.Device, cfg.HWAddr, cfg.KernelPath, containerNs) + return moveLinkOut(cfg.Device, cfg.HWAddr, cfg.KernelPath, containerNs) } -func addLink(device, hwAddr, kernelPath string, containerNs ns.NetNS) error { +func moveLinkIn(device, hwAddr, kernelPath string, containerNs ns.NetNS) (netlink.Link, error) { dev, err := getLink(device, hwAddr, kernelPath) if err != nil { - return err + return nil, err + } + if err := netlink.LinkSetNsFd(dev, int(containerNs.Fd())); err != nil { + return nil, err } - return netlink.LinkSetNsFd(dev, int(containerNs.Fd())) -} -func removeLink(device, hwAddr, kernelPath string, containerNs ns.NetNS) error { - var dev netlink.Link - err := containerNs.Do(func(_ ns.NetNS) error { - d, err := getLink(device, hwAddr, kernelPath) + if err := containerNs.Do(func(_ ns.NetNS) error { + dev, err = netlink.LinkByName(dev.Attrs().Name) if err != nil { return err } - dev = d return nil - }) - if err != nil { - return err + }); err != nil { + return nil, err } + + return dev, nil +} + +func moveLinkOut(device, hwAddr, kernelPath string, containerNs ns.NetNS) error { defaultNs, err := ns.GetCurrentNS() if err != nil { return err } - return netlink.LinkSetNsFd(dev, int(defaultNs.Fd())) + + return containerNs.Do(func(_ ns.NetNS) error { + dev, err := getLink(device, hwAddr, kernelPath) + if err != nil { + return err + } + return netlink.LinkSetNsFd(dev, int(defaultNs.Fd())) + }) +} + +func printLink(dev netlink.Link, cniVersion string, containerNs ns.NetNS) error { + result := current.Result{ + CNIVersion: current.ImplementedSpecVersion, + Interfaces: []*current.Interface{ + { + Name: dev.Attrs().Name, + Mac: dev.Attrs().HardwareAddr.String(), + Sandbox: containerNs.Path(), + }, + }, + } + return types.PrintResult(&result, cniVersion) } func getLink(devname, hwaddr, kernelpath string) (netlink.Link, error) { diff --git a/plugins/main/host-device/host-device_test.go b/plugins/main/host-device/host-device_test.go index 128ad73c..7f74fc1c 100644 --- a/plugins/main/host-device/host-device_test.go +++ b/plugins/main/host-device/host-device_test.go @@ -19,6 +19,8 @@ import ( "math/rand" "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/types/current" "github.com/containernetworking/plugins/pkg/ns" "github.com/containernetworking/plugins/pkg/testutils" . "github.com/onsi/ginkgo" @@ -44,6 +46,8 @@ var _ = Describe("base functionality", func() { It("Works with a valid config", func() { + var origLink netlink.Link + // prepare ifname in original namespace err := originalNS.Do(func(ns.NetNS) error { defer GinkgoRecover() @@ -53,9 +57,9 @@ var _ = Describe("base functionality", func() { }, }) Expect(err).NotTo(HaveOccurred()) - link, err := netlink.LinkByName(ifname) + origLink, err = netlink.LinkByName(ifname) Expect(err).NotTo(HaveOccurred()) - err = netlink.LinkSetUp(link) + err = netlink.LinkSetUp(origLink) Expect(err).NotTo(HaveOccurred()) return nil }) @@ -77,13 +81,26 @@ var _ = Describe("base functionality", func() { IfName: ifname, StdinData: []byte(conf), } + var resI types.Result err = originalNS.Do(func(ns.NetNS) error { defer GinkgoRecover() - _, _, err := testutils.CmdAddWithResult(targetNS.Path(), ifname, []byte(conf), func() error { return cmdAdd(args) }) + var err error + resI, _, err = testutils.CmdAddWithResult(targetNS.Path(), ifname, []byte(conf), func() error { return cmdAdd(args) }) return err }) Expect(err).NotTo(HaveOccurred()) + // check that the result was sane + res, err := current.NewResultFromResult(resI) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Interfaces).To(Equal([]*current.Interface{ + { + Name: ifname, + Mac: origLink.Attrs().HardwareAddr.String(), + Sandbox: targetNS.Path(), + }, + })) + // assert that dummy0 is now in the target namespace err = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() @@ -102,6 +119,19 @@ var _ = Describe("base functionality", func() { return nil }) Expect(err).NotTo(HaveOccurred()) + + // Check that deleting the device moves it back + err = originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + err = testutils.CmdDelWithResult(targetNS.Path(), ifname, func() error { return cmdDel(args) }) + Expect(err).NotTo(HaveOccurred()) + + _, err := netlink.LinkByName(ifname) + Expect(err).NotTo(HaveOccurred()) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + }) It("fails an invalid config", func() { From 1f02326d56e8824a0905bb001292d361d54d4727 Mon Sep 17 00:00:00 2001 From: oilbeater Date: Mon, 27 Nov 2017 15:26:07 +0800 Subject: [PATCH 07/14] delete link and ip if err when cmdAdd to avoid resource leak. --- plugins/main/macvlan/macvlan.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/plugins/main/macvlan/macvlan.go b/plugins/main/macvlan/macvlan.go index 0589831f..618fe96d 100644 --- a/plugins/main/macvlan/macvlan.go +++ b/plugins/main/macvlan/macvlan.go @@ -161,11 +161,28 @@ func cmdAdd(args *skel.CmdArgs) error { return err } + // Delete link if err to avoid link leak in this ns + defer func() { + if err != nil { + netns.Do(func(_ ns.NetNS) error { + return ip.DelLinkByName(args.IfName) + }) + } + }() + // run the IPAM plugin and get back the config to apply r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData) if err != nil { return err } + + // Invoke ipam del if err to avoid ip leak + defer func() { + if err != nil { + ipam.ExecDel(n.IPAM.Type, args.StdinData) + } + }() + // Convert whatever the IPAM result was into the current Result type result, err := current.NewResultFromResult(r) if err != nil { From ecdd827d3a7549f4b7bfc2e66bbc81e544870293 Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Sun, 3 Dec 2017 14:22:36 -0800 Subject: [PATCH 08/14] README: add badge for appveyor (Windows CI) --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 458ebd68..f0e44435 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,5 @@ -[![Build Status](https://travis-ci.org/containernetworking/plugins.svg?branch=master)](https://travis-ci.org/containernetworking/plugins) +[![Linux Build Status](https://travis-ci.org/containernetworking/plugins.svg?branch=master)](https://travis-ci.org/containernetworking/plugins) +[![Windows Build Status](https://ci.appveyor.com/api/projects/status/kcuubx0chr76ev86/branch/master?svg=true)](https://ci.appveyor.com/project/cni-bot/plugins/branch/master) # plugins Some CNI network plugins, maintained by the containernetworking team. For more information, see the individual READMEs. From b03d23a4fa2ee560f5a2ab1d26033eb947fe31ea Mon Sep 17 00:00:00 2001 From: Casey Callendrello Date: Fri, 8 Dec 2017 13:15:16 +0100 Subject: [PATCH 09/14] ipam/host-local: Accept ip ranges as a runtime argument This allows for the runtime to dynamically request IP ranges. Fixes: #95 --- plugins/ipam/host-local/README.md | 4 ++++ .../host-local/backend/allocator/config.go | 19 ++++++++++++++----- .../backend/allocator/config_test.go | 19 ++++++++++++++++++- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/plugins/ipam/host-local/README.md b/plugins/ipam/host-local/README.md index 7deb3555..ee81c864 100644 --- a/plugins/ipam/host-local/README.md +++ b/plugins/ipam/host-local/README.md @@ -120,6 +120,10 @@ The following [args conventions](https://github.com/containernetworking/cni/blob * `ips` (array of strings): A list of custom IPs to attempt to allocate +The following [Capability Args](https://github.com/containernetworking/cni/blob/master/CONVENTIONS.md) are supported: + +* `ipRanges`: The exact same as the `ranges` array - a list of address pools + ### Custom IP allocation For every requested custom IP, the `host-local` allocator will request that IP if it falls within one of the `range` objects. Thus it is possible to specify diff --git a/plugins/ipam/host-local/backend/allocator/config.go b/plugins/ipam/host-local/backend/allocator/config.go index 845dad0b..004054fe 100644 --- a/plugins/ipam/host-local/backend/allocator/config.go +++ b/plugins/ipam/host-local/backend/allocator/config.go @@ -23,12 +23,16 @@ import ( types020 "github.com/containernetworking/cni/pkg/types/020" ) -// The top-level network config, just so we can get the IPAM block +// The top-level network config - IPAM plugins are passed the full configuration +// of the calling plugin, not just the IPAM section. type Net struct { - Name string `json:"name"` - CNIVersion string `json:"cniVersion"` - IPAM *IPAMConfig `json:"ipam"` - Args *struct { + Name string `json:"name"` + CNIVersion string `json:"cniVersion"` + IPAM *IPAMConfig `json:"ipam"` + RuntimeConfig struct { // The capability arg + IPRanges []RangeSet `json:"ipRanges,omitempty"` + } `json:"runtimeConfig,omitempty"` + Args *struct { A *IPAMArgs `json:"cni"` } `json:"args"` } @@ -106,6 +110,11 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) { } n.IPAM.Range = nil + // If a range is supplied as a runtime config, prepend it to the Ranges + if len(n.RuntimeConfig.IPRanges) > 0 { + n.IPAM.Ranges = append(n.RuntimeConfig.IPRanges, n.IPAM.Ranges...) + } + if len(n.IPAM.Ranges) == 0 { return nil, "", fmt.Errorf("no IP ranges specified") } diff --git a/plugins/ipam/host-local/backend/allocator/config_test.go b/plugins/ipam/host-local/backend/allocator/config_test.go index 4631123b..cbae3d15 100644 --- a/plugins/ipam/host-local/backend/allocator/config_test.go +++ b/plugins/ipam/host-local/backend/allocator/config_test.go @@ -132,12 +132,18 @@ var _ = Describe("IPAM config", func() { })) }) - It("Should parse a mixed config", func() { + It("Should parse a mixed config with runtime args", func() { input := `{ "cniVersion": "0.3.1", "name": "mynet", "type": "ipvlan", "master": "foo0", + "runtimeConfig": { + "irrelevant": "a", + "ipRanges": [ + [{ "subnet": "12.1.3.0/24" }] + ] + }, "ipam": { "type": "host-local", "subnet": "10.1.2.0/24", @@ -162,6 +168,17 @@ var _ = Describe("IPAM config", func() { Name: "mynet", Type: "host-local", Ranges: []RangeSet{ + { // The RuntimeConfig should always be first + { + RangeStart: net.IP{12, 1, 3, 1}, + RangeEnd: net.IP{12, 1, 3, 254}, + Gateway: net.IP{12, 1, 3, 1}, + Subnet: types.IPNet{ + IP: net.IP{12, 1, 3, 0}, + Mask: net.CIDRMask(24, 32), + }, + }, + }, { { RangeStart: net.IP{10, 1, 2, 9}, From 97664d8a6af0d70303b5015b22e472a853423764 Mon Sep 17 00:00:00 2001 From: alice02 Date: Tue, 19 Dec 2017 00:46:17 +0900 Subject: [PATCH 10/14] pkg/ipam: Skip ip.SettleAddresses if only IPv4 is used This change improves the performance of the ipam.ConfigureIface. Some plugins are slow because of the ip.SettleAddress in ipam.ConfigureIface. It seems to be only needed for IPv6, so should be skipped if only IPv4 is used. --- pkg/ipam/ipam_linux.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/ipam/ipam_linux.go b/pkg/ipam/ipam_linux.go index 6c8ef20c..1fe89d8b 100644 --- a/pkg/ipam/ipam_linux.go +++ b/pkg/ipam/ipam_linux.go @@ -65,7 +65,9 @@ func ConfigureIface(ifName string, res *current.Result) error { } } - ip.SettleAddresses(ifName, 10) + if v6gw != nil { + ip.SettleAddresses(ifName, 10) + } for _, r := range res.Routes { routeIsV4 := r.Dst.IP.To4() != nil From 8ebea58550e28e24f87c8b8b1a70ccde19d59401 Mon Sep 17 00:00:00 2001 From: Jing Ai Date: Fri, 12 Jan 2018 12:06:48 -0800 Subject: [PATCH 11/14] Update IPVLAN modes by adding l3s in README. --- plugins/main/ipvlan/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/main/ipvlan/README.md b/plugins/main/ipvlan/README.md index 988555ef..f4a3a75a 100644 --- a/plugins/main/ipvlan/README.md +++ b/plugins/main/ipvlan/README.md @@ -28,7 +28,7 @@ Because all ipvlan interfaces share the MAC address with the host interface, DHC * `name` (string, required): the name of the network. * `type` (string, required): "ipvlan". * `master` (string, required): name of the host interface to enslave. -* `mode` (string, optional): one of "l2", "l3". Defaults to "l2". +* `mode` (string, optional): one of "l2", "l3", "l3s". Defaults to "l2". * `mtu` (integer, optional): explicitly set MTU to the specified value. Defaults to the value chosen by the kernel. * `ipam` (dictionary, required): IPAM configuration to be used for this network. From 59f997601733c4355b240137a686b3ed4c0e44ba Mon Sep 17 00:00:00 2001 From: Shengjing Zhu Date: Thu, 11 Jan 2018 22:47:39 +0800 Subject: [PATCH 12/14] pkg/ip: don't write to /proc/sys if ipforward enabled This enables setup in a container env like systemd nspawn where /proc/sys is mouted as read only. Signed-off-by: Shengjing Zhu --- pkg/ip/ipforward_linux.go | 6 ++++++ pkg/ip/ipforward_linux_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 pkg/ip/ipforward_linux_test.go diff --git a/pkg/ip/ipforward_linux.go b/pkg/ip/ipforward_linux.go index abab3ecf..8216a2c3 100644 --- a/pkg/ip/ipforward_linux.go +++ b/pkg/ip/ipforward_linux.go @@ -15,6 +15,7 @@ package ip import ( + "bytes" "io/ioutil" "github.com/containernetworking/cni/pkg/types/current" @@ -51,5 +52,10 @@ func EnableForward(ips []*current.IPConfig) error { } func echo1(f string) error { + if content, err := ioutil.ReadFile(f); err == nil { + if bytes.Equal(bytes.TrimSpace(content), []byte("1")) { + return nil + } + } return ioutil.WriteFile(f, []byte("1"), 0644) } diff --git a/pkg/ip/ipforward_linux_test.go b/pkg/ip/ipforward_linux_test.go new file mode 100644 index 00000000..eeedcc2e --- /dev/null +++ b/pkg/ip/ipforward_linux_test.go @@ -0,0 +1,31 @@ +package ip + +import ( + "io/ioutil" + "os" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("IpforwardLinux", func() { + It("echo1 must not write the file if content is 1", func() { + file, err := ioutil.TempFile(os.TempDir(), "containernetworking") + defer os.Remove(file.Name()) + err = echo1(file.Name()) + Expect(err).NotTo(HaveOccurred()) + statBefore, err := file.Stat() + Expect(err).NotTo(HaveOccurred()) + + // take a duration here, otherwise next file modification operation time + // will be same as previous one. + time.Sleep(100 * time.Millisecond) + + err = echo1(file.Name()) + Expect(err).NotTo(HaveOccurred()) + statAfter, err := file.Stat() + Expect(err).NotTo(HaveOccurred()) + Expect(statBefore.ModTime()).To(Equal(statAfter.ModTime())) + }) +}) From ffc591e24228f026bbc4b2afdb23652bdc8140fa Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 17 Jan 2018 10:27:54 -0600 Subject: [PATCH 13/14] host-device: respect CNI_IFNAME/args.IfName On ADD save the host device's name into its IFLA_ALIAS property and rename the device to the requested CNI_IFNAME inside the container to conform to the CNI specification. On DEL rename the device to the original name and move it back into the host namespace. --- plugins/main/host-device/host-device.go | 64 ++++++++++++++------ plugins/main/host-device/host-device_test.go | 16 ++--- 2 files changed, 54 insertions(+), 26 deletions(-) diff --git a/plugins/main/host-device/host-device.go b/plugins/main/host-device/host-device.go index 8f3d482a..0b9e7a81 100644 --- a/plugins/main/host-device/host-device.go +++ b/plugins/main/host-device/host-device.go @@ -68,15 +68,20 @@ func cmdAdd(args *skel.CmdArgs) error { } defer containerNs.Close() - dev, err := moveLinkIn(cfg.Device, cfg.HWAddr, cfg.KernelPath, containerNs) + hostDev, err := getLink(cfg.Device, cfg.HWAddr, cfg.KernelPath) + if err != nil { + return fmt.Errorf("failed to find host device: %v", err) + } + + contDev, err := moveLinkIn(hostDev, containerNs, args.IfName) if err != nil { return fmt.Errorf("failed to move link %v", err) } - return printLink(dev, cfg.CNIVersion, containerNs) + return printLink(contDev, cfg.CNIVersion, containerNs) } func cmdDel(args *skel.CmdArgs) error { - cfg, err := loadConf(args.StdinData) + _, err := loadConf(args.StdinData) if err != nil { return err } @@ -85,44 +90,67 @@ func cmdDel(args *skel.CmdArgs) error { return fmt.Errorf("failed to open netns %q: %v", args.Netns, err) } defer containerNs.Close() - defer fmt.Println(`{}`) - return moveLinkOut(cfg.Device, cfg.HWAddr, cfg.KernelPath, containerNs) + + if err := moveLinkOut(containerNs, args.IfName); err != nil { + return err + } + + return nil } -func moveLinkIn(device, hwAddr, kernelPath string, containerNs ns.NetNS) (netlink.Link, error) { - dev, err := getLink(device, hwAddr, kernelPath) - if err != nil { - return nil, err - } - if err := netlink.LinkSetNsFd(dev, int(containerNs.Fd())); err != nil { +func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, ifName string) (netlink.Link, error) { + if err := netlink.LinkSetNsFd(hostDev, int(containerNs.Fd())); err != nil { return nil, err } + var contDev netlink.Link if err := containerNs.Do(func(_ ns.NetNS) error { - dev, err = netlink.LinkByName(dev.Attrs().Name) + var err error + contDev, err = netlink.LinkByName(hostDev.Attrs().Name) if err != nil { - return err + return fmt.Errorf("failed to find %q: %v", hostDev.Attrs().Name, err) + } + // Save host device name into the container device's alias property + if err := netlink.LinkSetAlias(contDev, hostDev.Attrs().Name); err != nil { + return fmt.Errorf("failed to set alias to %q: %v", hostDev.Attrs().Name, err) + } + // Rename container device to respect args.IfName + if err := netlink.LinkSetName(contDev, ifName); err != nil { + return fmt.Errorf("failed to rename device %q to %q: %v", hostDev.Attrs().Name, ifName, err) + } + // Retrieve link again to get up-to-date name and attributes + contDev, err = netlink.LinkByName(ifName) + if err != nil { + return fmt.Errorf("failed to find %q: %v", ifName, err) } return nil }); err != nil { return nil, err } - return dev, nil + return contDev, nil } -func moveLinkOut(device, hwAddr, kernelPath string, containerNs ns.NetNS) error { +func moveLinkOut(containerNs ns.NetNS, ifName string) error { defaultNs, err := ns.GetCurrentNS() if err != nil { return err } + defer defaultNs.Close() return containerNs.Do(func(_ ns.NetNS) error { - dev, err := getLink(device, hwAddr, kernelPath) + dev, err := netlink.LinkByName(ifName) if err != nil { - return err + return fmt.Errorf("failed to find %q: %v", ifName, err) } - return netlink.LinkSetNsFd(dev, int(defaultNs.Fd())) + // Rename device to it's original name + if err := netlink.LinkSetName(dev, dev.Attrs().Alias); err != nil { + return fmt.Errorf("failed to restore %q to original name %q: %v", ifName, dev.Attrs().Alias, err) + } + if err := netlink.LinkSetNsFd(dev, int(defaultNs.Fd())); err != nil { + return fmt.Errorf("failed to move %q to host netns: %v", dev.Attrs().Alias, err) + } + return nil }) } diff --git a/plugins/main/host-device/host-device_test.go b/plugins/main/host-device/host-device_test.go index 7f74fc1c..218fc29d 100644 --- a/plugins/main/host-device/host-device_test.go +++ b/plugins/main/host-device/host-device_test.go @@ -45,7 +45,6 @@ var _ = Describe("base functionality", func() { }) It("Works with a valid config", func() { - var origLink netlink.Link // prepare ifname in original namespace @@ -69,6 +68,7 @@ var _ = Describe("base functionality", func() { targetNS, err := ns.NewNS() Expect(err).NotTo(HaveOccurred()) + CNI_IFNAME := "eth0" conf := fmt.Sprintf(`{ "cniVersion": "0.3.0", "name": "cni-plugin-host-device-test", @@ -78,14 +78,14 @@ var _ = Describe("base functionality", func() { args := &skel.CmdArgs{ ContainerID: "dummy", Netns: targetNS.Path(), - IfName: ifname, + IfName: CNI_IFNAME, StdinData: []byte(conf), } var resI types.Result err = originalNS.Do(func(ns.NetNS) error { defer GinkgoRecover() var err error - resI, _, err = testutils.CmdAddWithResult(targetNS.Path(), ifname, []byte(conf), func() error { return cmdAdd(args) }) + resI, _, err = testutils.CmdAddWithResult(targetNS.Path(), CNI_IFNAME, []byte(conf), func() error { return cmdAdd(args) }) return err }) Expect(err).NotTo(HaveOccurred()) @@ -95,7 +95,7 @@ var _ = Describe("base functionality", func() { Expect(err).NotTo(HaveOccurred()) Expect(res.Interfaces).To(Equal([]*current.Interface{ { - Name: ifname, + Name: CNI_IFNAME, Mac: origLink.Attrs().HardwareAddr.String(), Sandbox: targetNS.Path(), }, @@ -104,9 +104,9 @@ var _ = Describe("base functionality", func() { // assert that dummy0 is now in the target namespace err = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() - link, err := netlink.LinkByName(ifname) + link, err := netlink.LinkByName(CNI_IFNAME) Expect(err).NotTo(HaveOccurred()) - Expect(link.Attrs().Name).To(Equal(ifname)) + Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr)) return nil }) Expect(err).NotTo(HaveOccurred()) @@ -120,10 +120,10 @@ var _ = Describe("base functionality", func() { }) Expect(err).NotTo(HaveOccurred()) - // Check that deleting the device moves it back + // Check that deleting the device moves it back and restores the name err = originalNS.Do(func(ns.NetNS) error { defer GinkgoRecover() - err = testutils.CmdDelWithResult(targetNS.Path(), ifname, func() error { return cmdDel(args) }) + err = testutils.CmdDelWithResult(targetNS.Path(), CNI_IFNAME, func() error { return cmdDel(args) }) Expect(err).NotTo(HaveOccurred()) _, err := netlink.LinkByName(ifname) From 9604565b2296ac704aaba8b2bbf2345b71969ff5 Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Tue, 23 Jan 2018 12:59:03 +0900 Subject: [PATCH 14/14] Add -hostprefix in DHCP daemon to run the daemon as container This diff adds -hostprefix option in dhcp daemon. This option could be used to run dhcp daemon as container because container cannot touch host's netns directly. The diff changes dhcp daemon to touch procfs mounted to another path, like '/hostfs/proc'. --- plugins/ipam/dhcp/README.md | 1 + plugins/ipam/dhcp/daemon.go | 11 +++++++---- plugins/ipam/dhcp/main.go | 4 +++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/plugins/ipam/dhcp/README.md b/plugins/ipam/dhcp/README.md index 55e5396c..0ec4c4cb 100644 --- a/plugins/ipam/dhcp/README.md +++ b/plugins/ipam/dhcp/README.md @@ -18,6 +18,7 @@ $ ./dhcp daemon If given `-pidfile ` arguments after 'daemon', the dhcp plugin will write its PID to the given file. +If given `-hostprefix ` arguments after 'daemon', the dhcp plugin will use this prefix for netns as `/`. It could be used in case of running dhcp daemon as container. Alternatively, you can use systemd socket activation protocol. Be sure that the .socket file uses /run/cni/dhcp.sock as the socket path. diff --git a/plugins/ipam/dhcp/daemon.go b/plugins/ipam/dhcp/daemon.go index 7aa84208..a5316d75 100644 --- a/plugins/ipam/dhcp/daemon.go +++ b/plugins/ipam/dhcp/daemon.go @@ -39,8 +39,9 @@ const resendCount = 3 var errNoMoreTries = errors.New("no more tries") type DHCP struct { - mux sync.Mutex - leases map[string]*DHCPLease + mux sync.Mutex + leases map[string]*DHCPLease + hostNetnsPrefix string } func newDHCP() *DHCP { @@ -58,7 +59,8 @@ func (d *DHCP) Allocate(args *skel.CmdArgs, result *current.Result) error { } clientID := args.ContainerID + "/" + conf.Name - l, err := AcquireLease(clientID, args.Netns, args.IfName) + hostNetns := d.hostNetnsPrefix + args.Netns + l, err := AcquireLease(clientID, hostNetns, args.IfName) if err != nil { return err } @@ -140,7 +142,7 @@ func getListener() (net.Listener, error) { } } -func runDaemon(pidfilePath string) error { +func runDaemon(pidfilePath string, hostPrefix string) error { // since other goroutines (on separate threads) will change namespaces, // ensure the RPC server does not get scheduled onto those runtime.LockOSThread() @@ -161,6 +163,7 @@ func runDaemon(pidfilePath string) error { } dhcp := newDHCP() + dhcp.hostNetnsPrefix = hostPrefix rpc.Register(dhcp) rpc.HandleHTTP() http.Serve(l, nil) diff --git a/plugins/ipam/dhcp/main.go b/plugins/ipam/dhcp/main.go index 2e55c27a..73ba318a 100644 --- a/plugins/ipam/dhcp/main.go +++ b/plugins/ipam/dhcp/main.go @@ -33,11 +33,13 @@ const socketPath = "/run/cni/dhcp.sock" func main() { if len(os.Args) > 1 && os.Args[1] == "daemon" { var pidfilePath string + var hostPrefix string daemonFlags := flag.NewFlagSet("daemon", flag.ExitOnError) daemonFlags.StringVar(&pidfilePath, "pidfile", "", "optional path to write daemon PID to") + daemonFlags.StringVar(&hostPrefix, "hostprefix", "", "optional prefix to netns") daemonFlags.Parse(os.Args[2:]) - if err := runDaemon(pidfilePath); err != nil { + if err := runDaemon(pidfilePath, hostPrefix); err != nil { log.Printf(err.Error()) os.Exit(1) }