From ee2700c30cdd27dea203b7679fffc6fdec3b603f Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Tue, 27 Feb 2018 14:18:19 +0000 Subject: [PATCH 1/3] Whitelist failsafe response traffic in the raw chain. This should ensure that failsafe traffic always gets conntracked. --- fv/donottrack_test.go | 138 ++++++++++++++++++++++++++++++++++++++++++ rules/static.go | 44 +++++++++++--- rules/static_test.go | 24 +++++++- 3 files changed, 197 insertions(+), 9 deletions(-) create mode 100644 fv/donottrack_test.go diff --git a/fv/donottrack_test.go b/fv/donottrack_test.go new file mode 100644 index 0000000000..c5a7c84926 --- /dev/null +++ b/fv/donottrack_test.go @@ -0,0 +1,138 @@ +// +build fvtests + +// Copyright (c) 2017-2018 Tigera, Inc. All rights reserved. +// +// 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 fv_test + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/projectcalico/felix/fv/containers" + "github.com/projectcalico/felix/fv/workload" + api "github.com/projectcalico/libcalico-go/lib/apis/v3" + client "github.com/projectcalico/libcalico-go/lib/clientv3" + "github.com/projectcalico/libcalico-go/lib/options" +) + +var _ = Context("do-not-track policy tests; with 2 nodes", func() { + + var ( + etcd *containers.Container + felixes []*containers.Felix + hostW [2]*workload.Workload + client client.Interface + cc *workload.ConnectivityChecker + ) + + BeforeEach(func() { + options := containers.DefaultTopologyOptions() + felixes, etcd, client = containers.StartNNodeEtcdTopology(2, options) + cc = &workload.ConnectivityChecker{} + + // Start a host networked workload on each host for connectivity checks. + for ii := range felixes { + hostW[ii] = workload.Run( + felixes[ii], + fmt.Sprintf("host%d", ii), + "", // No interface name means "run in the host's namespace" + felixes[ii].IP, + "8055", + "tcp") + } + }) + + AfterEach(func() { + + if CurrentGinkgoTestDescription().Failed { + felixes[0].Exec("iptables-save", "-c") + felixes[0].Exec("ip", "r") + } + + for ii := range felixes { + felixes[ii].Stop() + } + + if CurrentGinkgoTestDescription().Failed { + etcd.Exec("etcdctl", "ls", "--recursive", "/") + } + etcd.Stop() + }) + + It("before adding policy, should have connectivity between hosts", func() { + cc.ExpectSome(felixes[0], hostW[1]) + cc.ExpectSome(felixes[1], hostW[0]) + cc.CheckConnectivity() + }) + + Context("after adding host endpoints", func() { + BeforeEach(func() { + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + for _, f := range felixes { + hep := api.NewHostEndpoint() + hep.Name = "eth0-" + f.Name + hep.Spec.Node = f.Hostname + hep.Spec.ExpectedIPs = []string{f.IP} + _, err := client.HostEndpoints().Create(ctx, hep, options.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + } + }) + + It("have no connectivity between hosts", func() { + cc.ExpectNone(felixes[0], hostW[1]) + cc.ExpectNone(felixes[1], hostW[0]) + cc.CheckConnectivity() + }) + }) + + // + //Context("with pre-DNAT policy to prevent access from outside", func() { + // BeforeEach(func() { + // policy := api.NewGlobalNetworkPolicy() + // policy.Name = "deny-ingress" + // order := float64(20) + // policy.Spec.Order = &order + // policy.Spec.PreDNAT = true + // policy.Spec.ApplyOnForward = true + // policy.Spec.Ingress = []api.Rule{{Action: api.Deny}} + // policy.Spec.Selector = "has(host-endpoint)" + // _, err := client.GlobalNetworkPolicies().Create(utils.Ctx, policy, utils.NoOptions) + // Expect(err).NotTo(HaveOccurred()) + // + // hostEp := api.NewHostEndpoint() + // hostEp.Name = "felix-eth0" + // hostEp.Spec.Node = felix.Hostname + // hostEp.Labels = map[string]string{"host-endpoint": "true"} + // hostEp.Spec.InterfaceName = "eth0" + // _, err = client.HostEndpoints().Create(utils.Ctx, hostEp, utils.NoOptions) + // Expect(err).NotTo(HaveOccurred()) + // }) + // + // It("etcd cannot connect", func() { + // cc := &workload.ConnectivityChecker{} + // cc.ExpectSome(w[0], w[1], 32011) + // cc.ExpectSome(w[1], w[0], 32010) + // cc.ExpectNone(etcd, w[1], 32011) + // cc.ExpectNone(etcd, w[0], 32010) + // cc.CheckConnectivity() + // }) + //}) +}) diff --git a/rules/static.go b/rules/static.go index 77ac7b830a..a3db5aa08d 100644 --- a/rules/static.go +++ b/rules/static.go @@ -40,7 +40,7 @@ func (r *DefaultRuleRenderer) StaticFilterInputChains(ipVersion uint8) []*Chain result = append(result, r.filterInputChain(ipVersion), r.filterWorkloadToHostChain(ipVersion), - r.failsafeInChain(), + r.failsafeInChain("filter"), ) if r.KubeIPVSSupportEnabled { result = append(result, r.StaticFilterInputForwardCheckChain(ipVersion)) @@ -329,7 +329,7 @@ func (r *DefaultRuleRenderer) filterWorkloadToHostChain(ipVersion uint8) *Chain } } -func (r *DefaultRuleRenderer) failsafeInChain() *Chain { +func (r *DefaultRuleRenderer) failsafeInChain(table string) *Chain { rules := []Rule{} for _, protoPort := range r.Config.FailsafeInboundHostPorts { @@ -341,13 +341,28 @@ func (r *DefaultRuleRenderer) failsafeInChain() *Chain { }) } + if table == "raw" { + // We're in the raw table, before conntrack, so we need to whitelist response traffic. + // Otherwise, it could fall through to some doNotTrack policy and half of the connection + // would get untracked. If we ACCEPT here then the traffic falls through to the filter + // table, where it'll only be accepted if there's a conntrack entry. + for _, protoPort := range r.Config.FailsafeOutboundHostPorts { + rules = append(rules, Rule{ + Match: Match(). + Protocol(protoPort.Protocol). + SourcePorts(protoPort.Port), + Action: AcceptAction{}, + }) + } + } + return &Chain{ Name: ChainFailsafeIn, Rules: rules, } } -func (r *DefaultRuleRenderer) failsafeOutChain() *Chain { +func (r *DefaultRuleRenderer) failsafeOutChain(table string) *Chain { rules := []Rule{} for _, protoPort := range r.Config.FailsafeOutboundHostPorts { @@ -359,6 +374,21 @@ func (r *DefaultRuleRenderer) failsafeOutChain() *Chain { }) } + if table == "raw" { + // We're in the raw table, before conntrack, so we need to whitelist response traffic. + // Otherwise, it could fall through to some doNotTrack policy and half of the connection + // would get untracked. If we ACCEPT here then the traffic falls through to the filter + // table, where it'll only be accepted if there's a conntrack entry. + for _, protoPort := range r.Config.FailsafeInboundHostPorts { + rules = append(rules, Rule{ + Match: Match(). + Protocol(protoPort.Protocol). + SourcePorts(protoPort.Port), + Action: AcceptAction{}, + }) + } + } + return &Chain{ Name: ChainFailsafeOut, Rules: rules, @@ -435,7 +465,7 @@ func (r *DefaultRuleRenderer) StaticFilterOutputChains(ipVersion uint8) []*Chain result := []*Chain{} result = append(result, r.filterOutputChain(ipVersion), - r.failsafeOutChain(), + r.failsafeOutChain("filter"), ) if r.KubeIPVSSupportEnabled { @@ -652,7 +682,7 @@ func (r *DefaultRuleRenderer) StaticNATOutputChains(ipVersion uint8) []*Chain { func (r *DefaultRuleRenderer) StaticMangleTableChains(ipVersion uint8) (chains []*Chain) { return []*Chain{ - r.failsafeInChain(), + r.failsafeInChain("mangle"), r.StaticManglePreroutingChain(ipVersion), } } @@ -721,8 +751,8 @@ func (r *DefaultRuleRenderer) StaticManglePreroutingChain(ipVersion uint8) *Chai func (r *DefaultRuleRenderer) StaticRawTableChains(ipVersion uint8) []*Chain { return []*Chain{ - r.failsafeInChain(), - r.failsafeOutChain(), + r.failsafeInChain("raw"), + r.failsafeOutChain("raw"), r.StaticRawPreroutingChain(ipVersion), r.StaticRawOutputChain(), } diff --git a/rules/static_test.go b/rules/static_test.go index ca28c57ea1..9c5fdefcaa 100644 --- a/rules/static_test.go +++ b/rules/static_test.go @@ -78,6 +78,26 @@ var _ = Describe("Static", func() { } portRanges = append(portRanges, portRange) + expRawFailsafeIn := &Chain{ + Name: "cali-failsafe-in", + Rules: []Rule{ + {Match: Match().Protocol("tcp").DestPorts(22), Action: AcceptAction{}}, + {Match: Match().Protocol("tcp").DestPorts(1022), Action: AcceptAction{}}, + {Match: Match().Protocol("tcp").SourcePorts(23), Action: AcceptAction{}}, + {Match: Match().Protocol("tcp").SourcePorts(1023), Action: AcceptAction{}}, + }, + } + + expRawFailsafeOut := &Chain{ + Name: "cali-failsafe-out", + Rules: []Rule{ + {Match: Match().Protocol("tcp").DestPorts(23), Action: AcceptAction{}}, + {Match: Match().Protocol("tcp").DestPorts(1023), Action: AcceptAction{}}, + {Match: Match().Protocol("tcp").SourcePorts(22), Action: AcceptAction{}}, + {Match: Match().Protocol("tcp").SourcePorts(1022), Action: AcceptAction{}}, + }, + } + expFailsafeIn := &Chain{ Name: "cali-failsafe-in", Rules: []Rule{ @@ -340,10 +360,10 @@ var _ = Describe("Static", func() { })) }) It("Should return expected raw failsafe in chain", func() { - Expect(findChain(rr.StaticRawTableChains(ipVersion), "cali-failsafe-in")).To(Equal(expFailsafeIn)) + Expect(findChain(rr.StaticRawTableChains(ipVersion), "cali-failsafe-in")).To(Equal(expRawFailsafeIn)) }) It("Should return expected raw failsafe out chain", func() { - Expect(findChain(rr.StaticRawTableChains(ipVersion), "cali-failsafe-out")).To(Equal(expFailsafeOut)) + Expect(findChain(rr.StaticRawTableChains(ipVersion), "cali-failsafe-out")).To(Equal(expRawFailsafeOut)) }) It("should return only the expected raw chains", func() { Expect(len(rr.StaticRawTableChains(ipVersion))).To(Equal(4)) From 7901688dd769d203f9675eba86defac17abfc83b Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Fri, 2 Mar 2018 10:28:56 +0000 Subject: [PATCH 2/3] Add FV test for failsafe port/do-not-track policy overlap. --- fv/donottrack_test.go | 249 +++++++++++++++++++++++++++++++--------- fv/workload/workload.go | 12 +- 2 files changed, 203 insertions(+), 58 deletions(-) diff --git a/fv/donottrack_test.go b/fv/donottrack_test.go index c5a7c84926..29b3c474ed 100644 --- a/fv/donottrack_test.go +++ b/fv/donottrack_test.go @@ -23,6 +23,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + log "github.com/sirupsen/logrus" "github.com/projectcalico/felix/fv/containers" "github.com/projectcalico/felix/fv/workload" @@ -34,14 +35,16 @@ import ( var _ = Context("do-not-track policy tests; with 2 nodes", func() { var ( - etcd *containers.Container - felixes []*containers.Felix - hostW [2]*workload.Workload - client client.Interface - cc *workload.ConnectivityChecker + etcd *containers.Container + felixes []*containers.Felix + hostW [2]*workload.Workload + client client.Interface + cc *workload.ConnectivityChecker + dumpedDiags bool ) BeforeEach(func() { + dumpedDiags = false options := containers.DefaultTopologyOptions() felixes, etcd, client = containers.StartNNodeEtcdTopology(2, options) cc = &workload.ConnectivityChecker{} @@ -53,42 +56,72 @@ var _ = Context("do-not-track policy tests; with 2 nodes", func() { fmt.Sprintf("host%d", ii), "", // No interface name means "run in the host's namespace" felixes[ii].IP, - "8055", + "8055,2379,22", // Extra ports are out/in and inbound failsafes. "tcp") } }) - AfterEach(func() { - - if CurrentGinkgoTestDescription().Failed { - felixes[0].Exec("iptables-save", "-c") - felixes[0].Exec("ip", "r") + // Utility function to dump diags if the test failed. Should be called in the inner-most + // AfterEach() to dump diags before the test is torn down. Only the first call for a given + // test has any effect. + dumpDiags := func() { + if !CurrentGinkgoTestDescription().Failed || dumpedDiags { + return } - for ii := range felixes { - felixes[ii].Stop() + iptSave, err := felixes[ii].ExecOutput("iptables-save", "-c") + if err == nil { + log.WithField("felix", ii).Info("iptables-save:\n" + iptSave) + } + ipR, err := felixes[ii].ExecOutput("ip", "r") + if err == nil { + log.WithField("felix", ii).Info("ip route:\n" + ipR) + } } + etcd.Exec("etcdctl", "ls", "--recursive", "/") - if CurrentGinkgoTestDescription().Failed { - etcd.Exec("etcdctl", "ls", "--recursive", "/") + } + + AfterEach(func() { + dumpDiags() + for _, f := range felixes { + f.Stop() } etcd.Stop() }) + expectFullConnectivity := func() { + cc.ResetExpectations() + cc.ExpectSome(felixes[0], hostW[1].Port(8055)) + cc.ExpectSome(felixes[1], hostW[0].Port(8055)) + cc.ExpectSome(felixes[0], hostW[1].Port(2379)) + cc.ExpectSome(felixes[1], hostW[0].Port(2379)) + cc.ExpectSome(felixes[0], hostW[1].Port(22)) + cc.ExpectSome(felixes[1], hostW[0].Port(22)) + cc.ExpectSome(etcd, hostW[1].Port(22)) + cc.ExpectSome(etcd, hostW[0].Port(22)) + cc.CheckConnectivityOffset(1) + } + It("before adding policy, should have connectivity between hosts", func() { - cc.ExpectSome(felixes[0], hostW[1]) - cc.ExpectSome(felixes[1], hostW[0]) - cc.CheckConnectivity() + expectFullConnectivity() }) Context("after adding host endpoints", func() { + var ( + ctx context.Context + cancel context.CancelFunc + ) + BeforeEach(func() { - ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) - defer cancel() + ctx, cancel = context.WithTimeout(context.Background(), 30*time.Second) for _, f := range felixes { hep := api.NewHostEndpoint() hep.Name = "eth0-" + f.Name + hep.Labels = map[string]string{ + "name": hep.Name, + } hep.Spec.Node = f.Hostname hep.Spec.ExpectedIPs = []string{f.IP} _, err := client.HostEndpoints().Create(ctx, hep, options.SetOptions{}) @@ -96,43 +129,147 @@ var _ = Context("do-not-track policy tests; with 2 nodes", func() { } }) - It("have no connectivity between hosts", func() { - cc.ExpectNone(felixes[0], hostW[1]) - cc.ExpectNone(felixes[1], hostW[0]) + AfterEach(func() { + dumpDiags() + cancel() + }) + + It("should implement untracked policy correctly", func() { + // This test covers both normal connectivity and failsafe connectivity. We combine the + // tests because we rely on the changes of normal connectivity at each step to make sure + // that the policy has actually flowed through to the dataplane. + + By("having only failsafe connectivity to start with") + cc.ExpectNone(felixes[0], hostW[1].Port(8055)) + cc.ExpectNone(felixes[1], hostW[0].Port(8055)) + cc.ExpectSome(felixes[0], hostW[1].Port(2379)) + cc.ExpectSome(felixes[1], hostW[0].Port(2379)) + // Port 22 is inbound-only so it'll be blocked by the (lack of egress policy). + cc.ExpectNone(felixes[0], hostW[1].Port(22)) + cc.ExpectNone(felixes[1], hostW[0].Port(22)) + // But etcd should still be able to access it... + cc.ExpectSome(etcd, hostW[1].Port(22)) + cc.ExpectSome(etcd, hostW[0].Port(22)) + cc.CheckConnectivity() + + host0Selector := fmt.Sprintf("name == 'eth0-%s'", felixes[0].Name) + host1Selector := fmt.Sprintf("name == 'eth0-%s'", felixes[1].Name) + + By("Having connectivity after installing bidirectional policies") + host0Pol := api.NewGlobalNetworkPolicy() + host0Pol.Name = "host-0-pol" + host0Pol.Spec.Selector = host0Selector + host0Pol.Spec.DoNotTrack = true + host0Pol.Spec.ApplyOnForward = true + host0Pol.Spec.Ingress = []api.Rule{ + { + Action: api.Allow, + Source: api.EntityRule{ + Selector: host1Selector, + }, + }, + } + host0Pol.Spec.Egress = []api.Rule{ + { + Action: api.Allow, + Destination: api.EntityRule{ + Selector: host1Selector, + }, + }, + } + host0Pol, err := client.GlobalNetworkPolicies().Create(ctx, host0Pol, options.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + host1Pol := api.NewGlobalNetworkPolicy() + host1Pol.Name = "host-1-pol" + host1Pol.Spec.Selector = host1Selector + host1Pol.Spec.DoNotTrack = true + host1Pol.Spec.ApplyOnForward = true + host1Pol.Spec.Ingress = []api.Rule{ + { + Action: api.Allow, + Source: api.EntityRule{ + Selector: host0Selector, + }, + }, + } + host1Pol.Spec.Egress = []api.Rule{ + { + Action: api.Allow, + Destination: api.EntityRule{ + Selector: host0Selector, + }, + }, + } + host1Pol, err = client.GlobalNetworkPolicies().Create(ctx, host1Pol, options.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + expectFullConnectivity() + + By("Having only failsafe connectivity after replacing host-0's egress rules with Deny") + // Since there's no conntrack, removing rules in one direction is enough to prevent + // connectivity in either direction. + host0Pol.Spec.Egress = []api.Rule{ + { + Action: api.Deny, + Destination: api.EntityRule{ + Selector: host0Selector, + }, + }, + } + host0Pol, err = client.GlobalNetworkPolicies().Update(ctx, host0Pol, options.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + host1Pol, err = client.GlobalNetworkPolicies().Update(ctx, host1Pol, options.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + cc.ResetExpectations() + cc.ExpectNone(felixes[0], hostW[1].Port(8055)) + cc.ExpectNone(felixes[1], hostW[0].Port(8055)) + cc.ExpectSome(felixes[0], hostW[1].Port(2379)) + cc.ExpectSome(felixes[1], hostW[0].Port(2379)) + cc.ExpectNone(felixes[0], hostW[1].Port(22)) // Now blocked (lack of egress). + cc.ExpectSome(felixes[1], hostW[0].Port(22)) // Still open due to failsafe. + cc.ExpectSome(etcd, hostW[1].Port(22)) // Allowed by failsafe + cc.ExpectSome(etcd, hostW[0].Port(22)) // Allowed by failsafe + cc.CheckConnectivity() + + By("Having full connectivity after putting them back") + host0Pol.Spec.Egress = []api.Rule{ + { + Action: api.Allow, + Destination: api.EntityRule{ + Selector: host1Selector, + }, + }, + } + host0Pol, err = client.GlobalNetworkPolicies().Update(ctx, host0Pol, options.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + expectFullConnectivity() + + By("Having only failsafe connectivity after replacing host-0's ingress rules with Deny") + host0Pol.Spec.Ingress = []api.Rule{ + { + Action: api.Deny, + Destination: api.EntityRule{ + Selector: host0Selector, + }, + }, + } + host0Pol, err = client.GlobalNetworkPolicies().Update(ctx, host0Pol, options.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + cc.ResetExpectations() + cc.ExpectNone(felixes[0], hostW[1].Port(8055)) + cc.ExpectNone(felixes[1], hostW[0].Port(8055)) + cc.ExpectSome(felixes[0], hostW[1].Port(2379)) + cc.ExpectSome(felixes[1], hostW[0].Port(2379)) + cc.ExpectNone(felixes[0], hostW[1].Port(22)) // Response traffic blocked by policy + cc.ExpectSome(felixes[1], hostW[0].Port(22)) // Allowed by failsafe + cc.ExpectSome(etcd, hostW[1].Port(22)) // Allowed by failsafe + cc.ExpectSome(etcd, hostW[0].Port(22)) // Allowed by failsafe cc.CheckConnectivity() }) }) - - // - //Context("with pre-DNAT policy to prevent access from outside", func() { - // BeforeEach(func() { - // policy := api.NewGlobalNetworkPolicy() - // policy.Name = "deny-ingress" - // order := float64(20) - // policy.Spec.Order = &order - // policy.Spec.PreDNAT = true - // policy.Spec.ApplyOnForward = true - // policy.Spec.Ingress = []api.Rule{{Action: api.Deny}} - // policy.Spec.Selector = "has(host-endpoint)" - // _, err := client.GlobalNetworkPolicies().Create(utils.Ctx, policy, utils.NoOptions) - // Expect(err).NotTo(HaveOccurred()) - // - // hostEp := api.NewHostEndpoint() - // hostEp.Name = "felix-eth0" - // hostEp.Spec.Node = felix.Hostname - // hostEp.Labels = map[string]string{"host-endpoint": "true"} - // hostEp.Spec.InterfaceName = "eth0" - // _, err = client.HostEndpoints().Create(utils.Ctx, hostEp, utils.NoOptions) - // Expect(err).NotTo(HaveOccurred()) - // }) - // - // It("etcd cannot connect", func() { - // cc := &workload.ConnectivityChecker{} - // cc.ExpectSome(w[0], w[1], 32011) - // cc.ExpectSome(w[1], w[0], 32010) - // cc.ExpectNone(etcd, w[1], 32011) - // cc.ExpectNone(etcd, w[0], 32010) - // cc.CheckConnectivity() - // }) - //}) }) diff --git a/fv/workload/workload.go b/fv/workload/workload.go index b9ccc5bd9d..40a7122e17 100644 --- a/fv/workload/workload.go +++ b/fv/workload/workload.go @@ -458,11 +458,19 @@ func (c *ConnectivityChecker) ExpectedConnectivity() []string { return result } +func (c *ConnectivityChecker) CheckConnectivityOffset(offset int, optionalDescription ...interface{}) { + c.CheckConnectivityWithTimeoutOffset(offset+2, 10*time.Second, optionalDescription...) +} + func (c *ConnectivityChecker) CheckConnectivity(optionalDescription ...interface{}) { - c.CheckConnectivityWithTimeout(10*time.Second, optionalDescription...) + c.CheckConnectivityWithTimeoutOffset(2, 10*time.Second, optionalDescription...) } func (c *ConnectivityChecker) CheckConnectivityWithTimeout(timeout time.Duration, optionalDescription ...interface{}) { + c.CheckConnectivityWithTimeoutOffset(2, timeout, optionalDescription...) +} + +func (c *ConnectivityChecker) CheckConnectivityWithTimeoutOffset(callerSkip int, timeout time.Duration, optionalDescription ...interface{}) { expConnectivity := c.ExpectedConnectivity() start := time.Now() @@ -492,5 +500,5 @@ func (c *ConnectivityChecker) CheckConnectivityWithTimeout(timeout time.Duration strings.Join(actualConn, "\n "), strings.Join(expConnectivity, "\n "), ) - Fail(message, 1) + Fail(message, callerSkip) } From 752aabdb372b56f7b2b2b9c22b3601a5fc56d69d Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Mon, 5 Mar 2018 16:17:51 +0000 Subject: [PATCH 3/3] Code review markups. --- fv/donottrack_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/fv/donottrack_test.go b/fv/donottrack_test.go index 29b3c474ed..0b87e7752c 100644 --- a/fv/donottrack_test.go +++ b/fv/donottrack_test.go @@ -1,6 +1,6 @@ // +build fvtests -// Copyright (c) 2017-2018 Tigera, Inc. All rights reserved. +// Copyright (c) 2018 Tigera, Inc. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -51,12 +51,19 @@ var _ = Context("do-not-track policy tests; with 2 nodes", func() { // Start a host networked workload on each host for connectivity checks. for ii := range felixes { + // We tell each workload to open: + // - its normal (uninteresting) port, 8055 + // - port 2379, which is both an inbound and an outbound failsafe port + // - port 22, which is an inbound failsafe port. + // This allows us to test the interaction between do-not-track policy and failsafe + // ports. + const portsToOpen = "8055,2379,22" hostW[ii] = workload.Run( felixes[ii], fmt.Sprintf("host%d", ii), "", // No interface name means "run in the host's namespace" felixes[ii].IP, - "8055,2379,22", // Extra ports are out/in and inbound failsafes. + portsToOpen, "tcp") } }) @@ -220,9 +227,6 @@ var _ = Context("do-not-track policy tests; with 2 nodes", func() { host0Pol, err = client.GlobalNetworkPolicies().Update(ctx, host0Pol, options.SetOptions{}) Expect(err).NotTo(HaveOccurred()) - host1Pol, err = client.GlobalNetworkPolicies().Update(ctx, host1Pol, options.SetOptions{}) - Expect(err).NotTo(HaveOccurred()) - cc.ResetExpectations() cc.ExpectNone(felixes[0], hostW[1].Port(8055)) cc.ExpectNone(felixes[1], hostW[0].Port(8055))