From d93d11ad2cc417f6f99ccfa9bc7fcc8c83ca5cee Mon Sep 17 00:00:00 2001 From: Jacob Tanenbaum Date: Tue, 8 Sep 2020 13:09:49 -0400 Subject: [PATCH 01/15] fix ipBlock except for network policy our current implementation of Ipblock is incorrect. creating a deny rule for the except blocks will not allow ips that overlap another IPblock to be allowed. I changed this so that we only create one ACL rule per ipBlock so if you have one policy that says - from: - ipBlock: cidr: 10.0.0.0/8 except: - 10.0.1.0/24 and another that says - from: - ipBlock: cidr: 10.0.0.0/8 it will translate two acls that are match=ip4.src == 10.0.0.0/8 && ip4.src !={10.0.1.0/24} match=ip4.src == 10.0.0.0/8 the first rule will not evaluate to true with a packet from 10.0.1.0 it will cascade to the second ACL and correctly be allowed. If the except is not overlapping an additional range it will be denied by the default deny ACL Signed-off-by: Jacob Tanenbaum --- go-controller/pkg/ovn/gress_policy.go | 158 ++++++--------------- go-controller/pkg/ovn/gress_policy_test.go | 119 ++++++++++++++++ go-controller/pkg/ovn/policy.go | 2 - test/scripts/e2e-kind.sh | 6 +- 4 files changed, 165 insertions(+), 120 deletions(-) create mode 100644 go-controller/pkg/ovn/gress_policy_test.go diff --git a/go-controller/pkg/ovn/gress_policy.go b/go-controller/pkg/ovn/gress_policy.go index a4a2c43e5d..4378cb418d 100644 --- a/go-controller/pkg/ovn/gress_policy.go +++ b/go-controller/pkg/ovn/gress_policy.go @@ -33,10 +33,7 @@ type gressPolicy struct { // the rule in question. portPolicies []*portPolicy - // ipBlockCidr represents the CIDR from which traffic is allowed - // except the IP block in the except, which should be dropped. - ipBlockCidr []string - ipBlockExcept []string + ipBlock []*knet.IPBlock } type portPolicy struct { @@ -73,8 +70,6 @@ func newGressPolicy(policyType knet.PolicyType, idx int, namespace, name string) peerV4AddressSets: sets.String{}, peerV6AddressSets: sets.String{}, portPolicies: make([]*portPolicy, 0), - ipBlockCidr: make([]string, 0), - ipBlockExcept: make([]string, 0), } } @@ -128,8 +123,7 @@ func (gp *gressPolicy) addPortPolicy(portJSON *knet.NetworkPolicyPort) { } func (gp *gressPolicy) addIPBlock(ipblockJSON *knet.IPBlock) { - gp.ipBlockCidr = append(gp.ipBlockCidr, ipblockJSON.CIDR) - gp.ipBlockExcept = append(gp.ipBlockExcept, ipblockJSON.Except...) + gp.ipBlock = append(gp.ipBlock, ipblockJSON) } func (gp *gressPolicy) getL3MatchFromAddressSet() string { @@ -186,28 +180,14 @@ func (gp *gressPolicy) sizeOfAddressSet() int { return gp.peerV4AddressSets.Len() + gp.peerV6AddressSets.Len() } -func (gp *gressPolicy) getMatchFromIPBlock(lportMatch, l4Match string) string { - var match string +func (gp *gressPolicy) getMatchFromIPBlock(lportMatch, l4Match string) []string { + var ipBlockMatches []string if gp.policyType == knet.PolicyTypeIngress { - ipBlockMatch := constructIPStringForACL("src", gp.ipBlockCidr) - if l4Match == noneMatch { - match = fmt.Sprintf("match=\"%s && %s\"", - ipBlockMatch, lportMatch) - } else { - match = fmt.Sprintf("match=\"%s && %s && %s\"", - ipBlockMatch, l4Match, lportMatch) - } + ipBlockMatches = constructIPBlockStringsForACL("src", gp.ipBlock, lportMatch, l4Match) } else { - ipBlockMatch := constructIPStringForACL("dst", gp.ipBlockCidr) - if l4Match == noneMatch { - match = fmt.Sprintf("match=\"%s && %s\"", - ipBlockMatch, lportMatch) - } else { - match = fmt.Sprintf("match=\"%s && %s && %s\"", - ipBlockMatch, l4Match, lportMatch) - } + ipBlockMatches = constructIPBlockStringsForACL("dst", gp.ipBlock, lportMatch, l4Match) } - return match + return ipBlockMatches } // addNamespaceAddressSet adds a new namespace address set to the gress policy @@ -252,35 +232,30 @@ func (gp *gressPolicy) delNamespaceAddressSet(name, portGroupName string) { // by the parent NetworkPolicy) func (gp *gressPolicy) localPodAddACL(portGroupName, portGroupUUID string) { l3Match := gp.getL3MatchFromAddressSet() - var lportMatch, cidrMatch string + var lportMatch string + var cidrMatches []string if gp.policyType == knet.PolicyTypeIngress { lportMatch = fmt.Sprintf("outport == @%s", portGroupName) } else { lportMatch = fmt.Sprintf("inport == @%s", portGroupName) } - // If IPBlock CIDR is not empty and except string [] is not empty, - // add deny acl rule with priority ipBlockDenyPriority (1010). - if len(gp.ipBlockCidr) > 0 && len(gp.ipBlockExcept) > 0 { - if err := gp.addIPBlockACLDeny(ipBlockDenyPriority, portGroupName, portGroupUUID); err != nil { - klog.Warningf(err.Error()) - } - } - if len(gp.portPolicies) == 0 { match := fmt.Sprintf("match=\"%s && %s\"", l3Match, lportMatch) l4Match := noneMatch - if len(gp.ipBlockCidr) > 0 { + if len(gp.ipBlock) > 0 { // Add ACL allow rule for IPBlock CIDR - cidrMatch = gp.getMatchFromIPBlock(lportMatch, l4Match) - if err := gp.addACLAllow(cidrMatch, l4Match, portGroupUUID, true); err != nil { - klog.Warningf(err.Error()) + cidrMatches = gp.getMatchFromIPBlock(lportMatch, l4Match) + for _, cidrMatch := range cidrMatches { + if err := gp.addACLAllow(cidrMatch, l4Match, portGroupUUID, true); err != nil { + klog.Warningf(err.Error()) + } } } // if there are pod/namespace selector, then allow packets from/to that address_set or // if the NetworkPolicyPeer is empty, then allow from all sources or to all destinations. - if gp.sizeOfAddressSet() > 0 || len(gp.ipBlockCidr) == 0 { + if gp.sizeOfAddressSet() > 0 || len(gp.ipBlock) == 0 { if err := gp.addACLAllow(match, l4Match, portGroupUUID, false); err != nil { klog.Warningf(err.Error()) } @@ -292,14 +267,16 @@ func (gp *gressPolicy) localPodAddACL(portGroupName, portGroupUUID string) { continue } match := fmt.Sprintf("match=\"%s && %s && %s\"", l3Match, l4Match, lportMatch) - if len(gp.ipBlockCidr) > 0 { + if len(gp.ipBlock) > 0 { // Add ACL allow rule for IPBlock CIDR - cidrMatch = gp.getMatchFromIPBlock(lportMatch, l4Match) - if err := gp.addACLAllow(cidrMatch, l4Match, portGroupUUID, true); err != nil { - klog.Warningf(err.Error()) + cidrMatches = gp.getMatchFromIPBlock(lportMatch, l4Match) + for _, cidrMatch := range cidrMatches { + if err := gp.addACLAllow(cidrMatch, l4Match, portGroupUUID, true); err != nil { + klog.Warningf(err.Error()) + } } } - if gp.sizeOfAddressSet() > 0 || len(gp.ipBlockCidr) == 0 { + if gp.sizeOfAddressSet() > 0 || len(gp.ipBlock) == 0 { if err := gp.addACLAllow(match, l4Match, portGroupUUID, false); err != nil { klog.Warningf(err.Error()) } @@ -382,79 +359,30 @@ func (gp *gressPolicy) modifyACLAllow(oldMatch, newMatch string) error { return nil } -func constructIPStringForACL(direction string, ipCIDRs []string) string { - var v4MatchStr, v6MatchStr, matchStr string - v4CIDRs := make([]string, 0) - v6CIDRs := make([]string, 0) - - for _, cidr := range ipCIDRs { - if utilnet.IsIPv6CIDRString(cidr) { - v6CIDRs = append(v6CIDRs, cidr) +func constructIPBlockStringsForACL(direction string, ipBlocks []*knet.IPBlock, lportMatch, l4Match string) []string { + var matchStrings []string + var matchStr, ipVersion string + for _, ipBlock := range ipBlocks { + if utilnet.IsIPv6CIDRString(ipBlock.CIDR) { + ipVersion = "ip6" } else { - v4CIDRs = append(v4CIDRs, cidr) + ipVersion = "ip4" } - } - if len(v4CIDRs) > 0 { - v4AddressSetStr := strings.Join(v4CIDRs, ", ") - v4MatchStr = fmt.Sprintf("%s.%s == {%s}", "ip4", direction, v4AddressSetStr) - matchStr = v4MatchStr - } - if len(v6CIDRs) > 0 { - v6AddressSetStr := strings.Join(v6CIDRs, ", ") - v6MatchStr = fmt.Sprintf("%s.%s == {%s}", "ip6", direction, v6AddressSetStr) - matchStr = v6MatchStr - } - if len(v4CIDRs) > 0 && len(v6CIDRs) > 0 { - matchStr = fmt.Sprintf("(%s || %s)", v4MatchStr, v6MatchStr) - } - return matchStr -} - -// addIPBlockACLDeny adds an IPBlock deny ACL to the given Port Group -func (gp *gressPolicy) addIPBlockACLDeny(priority, portGroupName, portGroupUUID string) error { - var match, l3Match, direction, lportMatch string - direction = toLport - if gp.policyType == knet.PolicyTypeIngress { - lportMatch = fmt.Sprintf("outport == @%s", portGroupName) - l3Match = constructIPStringForACL("src", gp.ipBlockExcept) - match = fmt.Sprintf("match=\"%s && %s\"", lportMatch, l3Match) - } else { - lportMatch = fmt.Sprintf("inport == @%s", portGroupName) - l3Match = constructIPStringForACL("dst", gp.ipBlockExcept) - match = fmt.Sprintf("match=\"%s && %s\"", lportMatch, l3Match) - } - - uuid, stderr, err := util.RunOVNNbctl("--data=bare", "--no-heading", - "--columns=_uuid", "find", "ACL", match, "action=drop", - fmt.Sprintf("external-ids:ipblock-deny-policy-type=%s", gp.policyType), - fmt.Sprintf("external-ids:namespace=%s", gp.policyNamespace), - fmt.Sprintf("external-ids:%s_num=%d", gp.policyType, gp.idx), - fmt.Sprintf("external-ids:policy=%s", gp.policyName)) - if err != nil { - return fmt.Errorf("find failed to get the ipblock default deny rule for "+ - "namespace=%s, policy=%s stderr: %q, (%v)", - gp.policyNamespace, gp.policyName, stderr, err) - } + if len(ipBlock.Except) == 0 { + matchStr = fmt.Sprintf("match=\"%s.%s == %s", ipVersion, direction, ipBlock.CIDR) - if uuid != "" { - return nil - } - - _, stderr, err = util.RunOVNNbctl("--id=@acl", "create", "acl", - fmt.Sprintf("priority=%s", priority), - fmt.Sprintf("direction=%s", direction), match, "action=drop", - fmt.Sprintf("external-ids:ipblock-deny-policy-type=%s", gp.policyType), - fmt.Sprintf("external-ids:%s_num=%d", gp.policyType, gp.idx), - fmt.Sprintf("external-ids:namespace=%s", gp.policyNamespace), - fmt.Sprintf("external-ids:policy=%s", gp.policyName), - "--", "add", "port_group", portGroupUUID, - "acls", "@acl") - if err != nil { - return fmt.Errorf("error executing create ACL command, stderr: %q, %+v", - stderr, err) + } else { + matchStr = fmt.Sprintf("match=\"%s.%s == %s && %s.%s != {%s}", ipVersion, direction, ipBlock.CIDR, + ipVersion, direction, strings.Join(ipBlock.Except, ", ")) + } + if l4Match == noneMatch { + matchStr = fmt.Sprintf("%s && %s\"", matchStr, lportMatch) + } else { + matchStr = fmt.Sprintf("%s && %s && %s\"", matchStr, l4Match, lportMatch) + } + matchStrings = append(matchStrings, matchStr) } - - return nil + return matchStrings } // localPodUpdateACL updates an existing ACL that implements the gress policy's rules diff --git a/go-controller/pkg/ovn/gress_policy_test.go b/go-controller/pkg/ovn/gress_policy_test.go new file mode 100644 index 0000000000..9086e08e1e --- /dev/null +++ b/go-controller/pkg/ovn/gress_policy_test.go @@ -0,0 +1,119 @@ +package ovn + +import ( + "github.com/stretchr/testify/assert" + knet "k8s.io/api/networking/v1" + "testing" +) + +func TestGetMatchFromIPBlock(t *testing.T) { + testcases := []struct { + desc string + ipBlocks []*knet.IPBlock + lportMatch string + l4Match string + expected []string + }{ + { + desc: "IPv4 only no except", + ipBlocks: []*knet.IPBlock{ + { + CIDR: "0.0.0.0/0", + }, + }, + lportMatch: "fake", + l4Match: "input", + expected: []string{"match=\"ip4.src == 0.0.0.0/0 && input && fake\""}, + }, + { + desc: "multiple IPv4 only no except", + ipBlocks: []*knet.IPBlock{ + { + CIDR: "0.0.0.0/0", + }, + { + CIDR: "10.1.0.0/16", + }, + }, + lportMatch: "fake", + l4Match: "input", + expected: []string{"match=\"ip4.src == 0.0.0.0/0 && input && fake\"", + "match=\"ip4.src == 10.1.0.0/16 && input && fake\""}, + }, + { + desc: "IPv6 only no except", + ipBlocks: []*knet.IPBlock{ + { + CIDR: "fd00:10:244:3::49/32", + }, + }, + lportMatch: "fake", + l4Match: "input", + expected: []string{"match=\"ip6.src == fd00:10:244:3::49/32 && input && fake\""}, + }, + { + desc: "mixed IPv4 and IPv6 no except", + ipBlocks: []*knet.IPBlock{ + { + CIDR: "::/0", + }, + { + CIDR: "0.0.0.0/0", + }, + }, + lportMatch: "fake", + l4Match: "input", + expected: []string{"match=\"ip6.src == ::/0 && input && fake\"", + "match=\"ip4.src == 0.0.0.0/0 && input && fake\""}, + }, + { + desc: "IPv4 only with except", + ipBlocks: []*knet.IPBlock{ + { + CIDR: "0.0.0.0/0", + Except: []string{"10.1.0.0/16"}, + }, + }, + lportMatch: "fake", + l4Match: "input", + expected: []string{"match=\"ip4.src == 0.0.0.0/0 && ip4.src != {10.1.0.0/16} && input && fake\""}, + }, + { + desc: "multiple IPv4 with except", + ipBlocks: []*knet.IPBlock{ + { + CIDR: "0.0.0.0/0", + Except: []string{"10.1.0.0/16"}, + }, + { + CIDR: "10.1.0.0/16", + }, + }, + lportMatch: "fake", + l4Match: "input", + expected: []string{"match=\"ip4.src == 0.0.0.0/0 && ip4.src != {10.1.0.0/16} && input && fake\"", + "match=\"ip4.src == 10.1.0.0/16 && input && fake\""}, + }, + { + desc: "IPv4 with IPv4 except", + ipBlocks: []*knet.IPBlock{ + { + CIDR: "0.0.0.0/0", + Except: []string{"10.1.0.0/16"}, + }, + }, + lportMatch: "fake", + l4Match: "input", + expected: []string{"match=\"ip4.src == 0.0.0.0/0 && ip4.src != {10.1.0.0/16} && input && fake\""}, + }, + } + + for _, tc := range testcases { + gressPolicy := newGressPolicy(knet.PolicyTypeIngress, 5, "testing", "test") + for _, ipBlock := range tc.ipBlocks { + gressPolicy.addIPBlock(ipBlock) + } + output := gressPolicy.getMatchFromIPBlock(tc.lportMatch, tc.l4Match) + assert.Equal(t, tc.expected, output) + } +} diff --git a/go-controller/pkg/ovn/policy.go b/go-controller/pkg/ovn/policy.go index 21ce036650..c9493602a1 100644 --- a/go-controller/pkg/ovn/policy.go +++ b/go-controller/pkg/ovn/policy.go @@ -50,8 +50,6 @@ const ( defaultDenyPriority = "1000" // Default allow acl rule priority defaultAllowPriority = "1001" - // IP Block except deny acl rule priority - ipBlockDenyPriority = "1010" // Default multicast deny acl rule priority defaultMcastDenyPriority = "1011" // Default multicast allow acl rule priority diff --git a/test/scripts/e2e-kind.sh b/test/scripts/e2e-kind.sh index b99173293e..504cd01982 100755 --- a/test/scripts/e2e-kind.sh +++ b/test/scripts/e2e-kind.sh @@ -32,9 +32,6 @@ Services.+session affinity # TO BE IMPLEMENTED: https://github.com/ovn-org/ovn-kubernetes/issues/1116 EndpointSlices -# TO BE IMPLEMENTED: https://github.com/ovn-org/ovn-kubernetes/issues/1663 -IPBlock.CIDR and IPBlock.Except - # TO BE IMPLEMENTED: https://github.com/ovn-org/ovn-kubernetes/issues/1664 should be able to preserve UDP traffic when server pod cycles for a NodePort service @@ -62,6 +59,9 @@ IPV4_ONLY_TESTS=" # The following tests currently fail for IPv6 only, but should be passing. # They will be removed as they are resolved. +# See: https://github.com/ovn-org/ovn-kubernetes/issues/1683 +IPBlock.CIDR and IPBlock.Except + # shard-n Tests # See: https://github.com/kubernetes/kubernetes/pull/94136 Network.+should resolve connection reset issue From 19d0eed2261c96c96fc85f8d9c20f2d4ef53e3e1 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Tue, 15 Sep 2020 10:13:01 +0200 Subject: [PATCH 02/15] bump kind to 0.9.0 Signed-off-by: Antonio Ojea --- test/scripts/install-kind.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/scripts/install-kind.sh b/test/scripts/install-kind.sh index 87aef20f6c..6926412b21 100755 --- a/test/scripts/install-kind.sh +++ b/test/scripts/install-kind.sh @@ -13,7 +13,7 @@ if [[ ! -f /usr/local/bin/e2e.test ]]; then fi popd -go get sigs.k8s.io/kind@c58694155106d0ac6e9612b7af5d0ac4f0559ba3 +go get sigs.k8s.io/kind@v0.9.0 pushd ../contrib ./kind.sh popd From 561d4f59f3f8ff6bc9a42567c90e7d0f67390aab Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Mon, 14 Sep 2020 16:31:46 +0200 Subject: [PATCH 03/15] disable informers factory resync period current resync period was set to 12 hours, instead of having such large period with the risk of overwhelming ovn with events every 12 hours, just disable the resync period. AddEventHandlerWithResyncPeriod can specify a per handler resync if necessary. Signed-off-by: Antonio Ojea --- go-controller/pkg/factory/factory.go | 5 +++-- go-controller/pkg/informer/const.go | 7 +++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go-controller/pkg/factory/factory.go b/go-controller/pkg/factory/factory.go index ed17ba6428..12a90cd247 100644 --- a/go-controller/pkg/factory/factory.go +++ b/go-controller/pkg/factory/factory.go @@ -441,7 +441,7 @@ type ObjectCacheInterface interface { var _ ObjectCacheInterface = &WatchFactory{} const ( - resyncInterval = 12 * time.Hour + resyncInterval = 0 handlerAlive uint32 = 0 handlerDead uint32 = 1 numEventQueues int = 15 @@ -461,11 +461,12 @@ var ( // NewWatchFactory initializes a new watch factory func NewWatchFactory(c kubernetes.Interface, eip egressipclientset.Interface, ec egressfirewallclientset.Interface, crd apiextensionsclientset.Interface) (*WatchFactory, error) { - // resync time is 12 hours, none of the resources being watched in ovn-kubernetes have + // resync time is 0, none of the resources being watched in ovn-kubernetes have // any race condition where a resync may be required e.g. cni executable on node watching for // events on pods and assuming that an 'ADD' event will contain the annotations put in by // ovnkube master (currently, it is just a 'get' loop) // the downside of making it tight (like 10 minutes) is needless spinning on all resources + // However, AddEventHandlerWithResyncPeriod can specify a per handler resync period wf := &WatchFactory{ iFactory: informerfactory.NewSharedInformerFactory(c, resyncInterval), eipFactory: egressipinformerfactory.NewSharedInformerFactory(eip, resyncInterval), diff --git a/go-controller/pkg/informer/const.go b/go-controller/pkg/informer/const.go index 4277feac87..dc520e8ca2 100644 --- a/go-controller/pkg/informer/const.go +++ b/go-controller/pkg/informer/const.go @@ -1,15 +1,14 @@ package informer -import "time" - // These constants can be removed at some point // They are here to provide backwards-compatibility with the // pkg/factory code which provided defaults. // The package consumer should make these decisions instead. const ( // DefaultResyncInterval is the default interval that all caches should - // periodically resync - DefaultResyncInterval = time.Hour * 12 + // periodically resync. AddEventHandlerWithResyncPeriod can specify a + // per handler resync period if necessary + DefaultResyncInterval = 0 // DefaultNodeInformerThreadiness is the number of worker routines spawned // to services the Node event queue DefaultNodeInformerThreadiness = 10 From 276d4365d297cf628e8fc8fd557bcf8ed2dcd4c7 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 16 Sep 2020 22:40:27 -0500 Subject: [PATCH 04/15] kind.sh: ignore directories when copying executables Avoids the copy exiting with code 1 when it tries to copy the 'windows' directory if you ever ran 'make windows'. + pushd ../dist/images ~/Development/containers/ovn-kubernetes/dist/images ~/Development/containers/ovn-kubernetes/contrib + sudo cp -f ../../go-controller/_output/go/bin/hybrid-overlay-node ../../go-controller/_output/go/bin/ovn-k8s-cni-overlay ../../go-controller/_output/go/bin/ovnkube ../../go-controller/_output/go/bin/ovn-kube-util ../../go-controller/_output/go/bin/windows . cp: -r not specified; omitting directory '../../go-controller/_output/go/bin/windows' Signed-off-by: Dan Williams --- contrib/kind.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/kind.sh b/contrib/kind.sh index e92fb94646..ad8b3d1978 100755 --- a/contrib/kind.sh +++ b/contrib/kind.sh @@ -316,7 +316,9 @@ if [ "$OVN_IMAGE" == local ]; then # Build ovn kube image pushd ../dist/images - sudo cp -f ../../go-controller/_output/go/bin/* . + # Find all built executables, but ignore the 'windows' directory if it exists + BINS=$(find ../../go-controller/_output/go/bin/ -maxdepth 1 -type f | xargs) + sudo cp -f ${BINS} . echo "ref: $(git rev-parse --symbolic-full-name HEAD) commit: $(git rev-parse HEAD)" > git_info docker build -t ovn-daemonset-f:dev -f Dockerfile.fedora . OVN_IMAGE=ovn-daemonset-f:dev From 85151ff041ef4149b5667b6d18e5abf41b269432 Mon Sep 17 00:00:00 2001 From: Alexey Roytman Date: Mon, 14 Sep 2020 17:46:46 +0300 Subject: [PATCH 05/15] Ubdate Ubuntu to 20.04 Ubuntu 18.04 repository contains old OVS/OVN packages, (2.9.5-0), which are not compatable with current implementation. With Ubuntu 20.04 `apt-get` installs OVS 2.13 and OVN 20.03 Signed-off-by: Alexey Roytman --- dist/images/Dockerfile.ubuntu | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dist/images/Dockerfile.ubuntu b/dist/images/Dockerfile.ubuntu index 9116d3133f..d89fac228d 100644 --- a/dist/images/Dockerfile.ubuntu +++ b/dist/images/Dockerfile.ubuntu @@ -8,11 +8,11 @@ # # So this file will change over time. -FROM ubuntu:18.04 +FROM ubuntu:20.04 USER root -RUN apt-get update && apt-get install -y arping iproute2 curl software-properties-common setpriv +RUN apt-get update && apt-get install -y arping iproute2 curl software-properties-common util-linux RUN echo "deb https://apt.kubernetes.io/ kubernetes-xenial main" | tee -a /etc/apt/sources.list.d/kubernetes.list RUN curl -s https://packages.cloud.google.com/apt/doc/apt-key.gpg | apt-key add - From 0ec4f5d74a25fb2de6bc00bf36db65d4785803b8 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Thu, 17 Sep 2020 19:36:39 +0200 Subject: [PATCH 06/15] fix a bug on kind deployment we have to untaint the master nodes so we can schedule pods on them, however, the kind get nodes does not report the nodes in the right order. Signed-off-by: Antonio Ojea --- contrib/kind.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/kind.sh b/contrib/kind.sh index ad8b3d1978..14b47595ac 100755 --- a/contrib/kind.sh +++ b/contrib/kind.sh @@ -358,7 +358,7 @@ pushd ../dist/yaml run_kubectl apply -f k8s.ovn.org_egressfirewalls.yaml run_kubectl apply -f k8s.ovn.org_egressips.yaml run_kubectl apply -f ovn-setup.yaml -MASTER_NODES=$(kind get nodes --name ${KIND_CLUSTER_NAME} | head -n ${KIND_NUM_MASTER}) +MASTER_NODES=$(kind get nodes --name ${KIND_CLUSTER_NAME} | sort | head -n ${KIND_NUM_MASTER}) # We want OVN HA not Kubernetes HA # leverage the kubeadm well-known label node-role.kubernetes.io/master= # to choose the nodes where ovn master components will be placed From e303c2e118074a64b2decba209e090698bfdfe0c Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Fri, 18 Sep 2020 12:04:53 -0400 Subject: [PATCH 07/15] Fix setting ovn-k8s-gw0 mac address for ipv6 We were setting it correctly in the mac_binding table for OVN, but the mac did not match the address configured on the host. This caused host destined packets from OVN to be dropped in IPv6. Signed-off-by: Tim Rozet --- go-controller/pkg/node/gateway_shared_intf_linux.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/go-controller/pkg/node/gateway_shared_intf_linux.go b/go-controller/pkg/node/gateway_shared_intf_linux.go index b6d41420f8..034a78ed25 100644 --- a/go-controller/pkg/node/gateway_shared_intf_linux.go +++ b/go-controller/pkg/node/gateway_shared_intf_linux.go @@ -38,7 +38,13 @@ func setupLocalNodeAccessBridge(nodeName string, subnets []*net.IPNet) error { return err } - macAddress := util.IPAddrToHWAddr(net.ParseIP(util.V4NodeLocalNatSubnetNextHop)).String() + var macAddress string + if config.IPv4Mode { + macAddress = util.IPAddrToHWAddr(net.ParseIP(util.V4NodeLocalNatSubnetNextHop)).String() + } else { + macAddress = util.IPAddrToHWAddr(net.ParseIP(util.V6NodeLocalNatSubnetNextHop)).String() + } + _, stderr, err = util.RunOVSVsctl( "--may-exist", "add-port", localBridgeName, localnetGatewayNextHopPort, "--", "set", "interface", localnetGatewayNextHopPort, "type=internal", From f8ba620064f944ee0fbf576e01bb7d9b4e20205c Mon Sep 17 00:00:00 2001 From: Jacob Tanenbaum Date: Thu, 17 Sep 2020 16:20:03 -0400 Subject: [PATCH 08/15] make egressfirewall use logical_router_policies Change egressfirewall to use logical_router_policies on the ovn_cluster_router as oppsed to using ACLs on each nodes join switches. This changes optimizes the generation of egressfirewall policies because we only have to create one object and put it in one place the ovn_cluster_router applies to all namespaces so we only need to attach the policy there. because the single router applies to all namespaces the test "correctly adds an existing egressFirewall to a new node" is no longer needed Signed-off-by: Jacob Tanenbaum --- go-controller/pkg/ovn/egressfirewall.go | 79 ++++-------- go-controller/pkg/ovn/egressfirewall_test.go | 125 ++----------------- go-controller/pkg/ovn/ovn.go | 22 ---- 3 files changed, 34 insertions(+), 192 deletions(-) diff --git a/go-controller/pkg/ovn/egressfirewall.go b/go-controller/pkg/ovn/egressfirewall.go index 7af9326f41..291a460ea1 100644 --- a/go-controller/pkg/ovn/egressfirewall.go +++ b/go-controller/pkg/ovn/egressfirewall.go @@ -15,7 +15,7 @@ import ( ) const ( - defaultStartPriority = 2000 + defaultStartPriority = 100 egressFirewallAppliedCorrectly = "EgressFirewall Rules applied" egressFirewallAddError = "EgressFirewall Rules not correctly added" egressFirewallUpdateError = "EgressFirewall Rules did not update correctly" @@ -83,6 +83,10 @@ func (oc *Controller) addEgressFirewall(egressFirewall *egressfirewallapi.Egress var errList []error for i, egressFirewallRule := range egressFirewall.Spec.Egress { //process Rules into egressFirewallRules for egressFirewall struct + if i > defaultStartPriority { + klog.Warningf("egressFirewall for namespace %s has to many rules, the rest will be ignored", egressFirewall.Namespace) + break + } efr, err := newEgressFirewallRule(egressFirewallRule, i) if err != nil { errList = append(errList, fmt.Errorf("error: cannot create EgressFirewall Rule for destination %s to namespace %s - %v", @@ -96,15 +100,7 @@ func (oc *Controller) addEgressFirewall(egressFirewall *egressfirewallapi.Egress return errList } - existingNodes, err := oc.kube.GetNodes() - if err != nil { - return []error{fmt.Errorf("error unable to add egressfirewall %s, cannot list nodes: %s", egressFirewall.Name, err)} - } - var joinSwitches []string - for _, node := range existingNodes.Items { - joinSwitches = append(joinSwitches, joinSwitch(node.Name)) - } - err = ef.addACLToJoinSwitch(joinSwitches, nsInfo.addressSet.GetIPv4HashName(), nsInfo.addressSet.GetIPv6HashName()) + err = ef.addLogicalRouterPolicyToClusterRouter(nsInfo.addressSet.GetIPv4HashName(), nsInfo.addressSet.GetIPv6HashName()) if err != nil { errList = append(errList, err) } @@ -131,29 +127,19 @@ func (oc *Controller) deleteEgressFirewall(egressFirewall *egressfirewallapi.Egr nsInfo.egressFirewallPolicy = nil nsInfo.Unlock() } - - existingNodes, err := oc.kube.GetNodes() - if err != nil { - return []error{fmt.Errorf("error deleting egressFirewall for namespace %s, cannot get nodes to delete ACLS %v", - egressFirewall.Namespace, err)} - } - - stdout, stderr, err := util.RunOVNNbctl("--data=bare", "--no-heading", - "--columns=_uuid", "find", "ACL", - fmt.Sprintf("external-ids:egressFirewall=%s", egressFirewall.Namespace)) + stdout, stderr, err := util.RunOVNNbctl("--data=bare", "--no-heading", "--columns=_uuid", "find", "logical_router_policy", fmt.Sprintf("external-ids:egressFirewall=%s", egressFirewall.Namespace)) if err != nil { - return []error{fmt.Errorf("error executing find ACL command, stderr: %q, %+v", stderr, err)} + return []error{fmt.Errorf("error deleting egressFirewall for namespace %s, cannot get logical router policies from LR %s - %s:%s", + egressFirewall.Namespace, ovnClusterRouter, err, stderr)} } var errList []error + uuids := strings.Fields(stdout) for _, uuid := range uuids { - for _, node := range existingNodes.Items { - _, stderr, err := util.RunOVNNbctl("remove", "logical_switch", - joinSwitch(node.Name), "acls", uuid) - if err != nil { - errList = append(errList, fmt.Errorf("failed to delete the rules for "+ - "egressFirewall in namespace %s on node %s, stderr: %q (%v)", egressFirewall.Namespace, node.Name, stderr, err)) - } + _, stderr, err := util.RunOVNNbctl("lr-policy-del", ovnClusterRouter, uuid) + if err != nil { + errList = append(errList, fmt.Errorf("failed to delete the rules for "+ + "egressFirewall in namespace %s on logical switch %s, stderr: %q (%v)", egressFirewall.Namespace, ovnClusterRouter, stderr, err)) } } return errList @@ -165,7 +151,7 @@ func (oc *Controller) updateEgressFirewallWithRetry(egressfirewall *egressfirewa }) } -func (ef *egressFirewall) addACLToJoinSwitch(joinSwitches []string, hashedAddressSetNameIPv4, hashedAddressSetNameIPv6 string) error { +func (ef *egressFirewall) addLogicalRouterPolicyToClusterRouter(hashedAddressSetNameIPv4, hashedAddressSetNameIPv6 string) error { for _, rule := range ef.egressRules { var match string var action string @@ -188,30 +174,17 @@ func (ef *egressFirewall) addACLToJoinSwitch(joinSwitches []string, hashedAddres if len(rule.ports) > 0 { match = fmt.Sprintf("%s && ( %s )\"", match[:len(match)-1], egressGetL4Match(rule.ports)) } - uuid, stderr, err := util.RunOVNNbctl("--data=bare", "--no-heading", - "--columns=_uuid", "find", "ACL", match, "action="+action, - fmt.Sprintf("external-ids:egressFirewall=%s", ef.namespace)) + _, stderr, err := util.RunOVNNbctl("--id=@logical_router_policy", "create", "logical_router_policy", + fmt.Sprintf("priority=%d", defaultStartPriority-rule.id), + match, "action="+action, fmt.Sprintf("external-ids:egressFirewall=%s", ef.namespace), + "--", "add", "logical_router", ovnClusterRouter, "policies", "@logical_router_policy") if err != nil { - return fmt.Errorf("error executing find ACL command, stderr: %q, %+v", stderr, err) - } - for _, joinSwitch := range joinSwitches { - if uuid == "" { - _, stderr, err := util.RunOVNNbctl("--id=@acl", "create", "acl", - fmt.Sprintf("priority=%d", defaultStartPriority-rule.id), - fmt.Sprintf("direction=%s", fromLport), match, "action="+action, - fmt.Sprintf("external-ids:egressFirewall=%s", ef.namespace), - "--", "add", "logical_switch", joinSwitch, - "acls", "@acl") - if err != nil { - return fmt.Errorf("error executing create ACL command, stderr: %q, %+v", stderr, err) - } - } else { - _, stderr, err := util.RunOVNNbctl("add", "logical_switch", joinSwitch, "acls", uuid) - if err != nil { - return fmt.Errorf("error adding ACL to joinsSwitch %s failed, stderr: %q, %+v", joinSwitch, stderr, err) - - } + // TODO: lr-policy-add doesn't support --may-exist, resort to this workaround for now. + // Have raised an issue against ovn repository (https://github.com/ovn-org/ovn/issues/49) + if !strings.Contains(stderr, "already existed") { + return fmt.Errorf("failed to add policy route '%s' to %s "+ + "stderr: %s, error: %v", match, ovnClusterRouter, stderr, err) } } } @@ -280,7 +253,3 @@ func egressGetL4Match(ports []egressfirewallapi.EgressFirewallPort) string { } return l4Match } - -func joinSwitch(nodeName string) string { - return fmt.Sprintf("join_%s", nodeName) -} diff --git a/go-controller/pkg/ovn/egressfirewall_test.go b/go-controller/pkg/ovn/egressfirewall_test.go index 56e8981305..b2bf0d99c4 100644 --- a/go-controller/pkg/ovn/egressfirewall_test.go +++ b/go-controller/pkg/ovn/egressfirewall_test.go @@ -69,8 +69,7 @@ var _ = Describe("OVN EgressFirewall Operations", func() { node1Name string = "node1" ) fExec.AddFakeCmdsNoOutputNoError([]string{ - "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1", - "ovn-nbctl --timeout=15 --id=@acl create acl priority=2000 direction=from-lport match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1 -- add logical_switch join_node1 acls @acl", + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", }) namespace1 := *newNamespace("namespace1") @@ -127,8 +126,7 @@ var _ = Describe("OVN EgressFirewall Operations", func() { node1Name string = "node1" ) fExec.AddFakeCmdsNoOutputNoError([]string{ - "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102 && ( (udp) )\" action=drop external-ids:egressFirewall=namespace1", - "ovn-nbctl --timeout=15 --id=@acl create acl priority=2000 direction=from-lport match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102 && ( (udp) )\" action=drop external-ids:egressFirewall=namespace1 -- add logical_switch join_node1 acls @acl", + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102 && ( (udp) )\" action=drop external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", }) namespace1 := *newNamespace("namespace1") egressFirewall := newEgressFirewallObject("default", namespace1.Name, []egressfirewallapi.EgressFirewallRule{ @@ -184,14 +182,12 @@ var _ = Describe("OVN EgressFirewall Operations", func() { node1Name string = "node1" ) fExec.AddFakeCmdsNoOutputNoError([]string{ - "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL match=\"ip4.dst == 1.2.3.5/23 && " + - "ip4.src == $a10481622940199974102 && ( (udp && ( udp.dst == 100 )) || (tcp) )\" action=allow external-ids:egressFirewall=namespace1", - "ovn-nbctl --timeout=15 --id=@acl create acl priority=2000 direction=from-lport match=\"ip4.dst == 1.2.3.5/23 && " + - "ip4.src == $a10481622940199974102 && ( (udp && ( udp.dst == 100 )) || (tcp) )\" action=allow external-ids:egressFirewall=namespace1 -- add logical_switch join_node1 acls @acl", - fmt.Sprintf("ovn-nbctl --timeout=15 remove logical_switch join_node1 acls %s", fakeUUID), + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.5/23 && " + + "ip4.src == $a10481622940199974102 && ( (udp && ( udp.dst == 100 )) || (tcp) )\" action=allow external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", + "ovn-nbctl --timeout=15 lr-policy-del ovn_cluster_router " + fmt.Sprintf("%s", fakeUUID), }) fExec.AddFakeCmd(&ovntest.ExpectedCmd{ - Cmd: "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL external-ids:egressFirewall=namespace1", + Cmd: "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find logical_router_policy external-ids:egressFirewall=namespace1", Output: fmt.Sprintf("%s", fakeUUID), }) @@ -255,14 +251,12 @@ var _ = Describe("OVN EgressFirewall Operations", func() { node1Name string = "node1" ) fExec.AddFakeCmdsNoOutputNoError([]string{ - "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1", - "ovn-nbctl --timeout=15 --id=@acl create acl priority=2000 direction=from-lport match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1 -- add logical_switch join_node1 acls @acl", - fmt.Sprintf("ovn-nbctl --timeout=15 remove logical_switch join_node1 acls %s", fakeUUID), - "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=drop external-ids:egressFirewall=namespace1", - "ovn-nbctl --timeout=15 --id=@acl create acl priority=2000 direction=from-lport match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=drop external-ids:egressFirewall=namespace1 -- add logical_switch join_node1 acls @acl", + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=drop external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", + "ovn-nbctl --timeout=15 lr-policy-del ovn_cluster_router " + fmt.Sprintf("%s", fakeUUID), }) fExec.AddFakeCmd(&ovntest.ExpectedCmd{ - Cmd: "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL external-ids:egressFirewall=namespace1", + Cmd: "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find logical_router_policy external-ids:egressFirewall=namespace1", Output: fmt.Sprintf("%s", fakeUUID), }) @@ -322,105 +316,6 @@ var _ = Describe("OVN EgressFirewall Operations", func() { Expect(err).NotTo(HaveOccurred()) }) - It("correctly adds an existing egressFirewall to a new node", func() { - app.Action = func(ctx *cli.Context) error { - const ( - node1Name string = "node1" - ) - stopChan := make(chan struct{}) - defer close(stopChan) - fExec.AddFakeCmdsNoOutputNoError([]string{ - //adding the original node commands - "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=name,other-config find logical_switch", - "ovn-nbctl --timeout=15 --if-exists lrp-del rtos-node1 -- lrp-add ovn_cluster_router rtos-node1 ", - "ovn-nbctl --timeout=15 --may-exist ls-add node1 -- set logical_switch node1", - "ovn-nbctl --timeout=15 set logical_switch node1 other-config:mcast_snoop=\"true\"", - "ovn-nbctl --timeout=15 set logical_switch node1 other-config:mcast_querier=\"false\"", - "ovn-nbctl --timeout=15 -- --may-exist lsp-add node1 stor-node1 -- set logical_switch_port stor-node1 type=router options:router-port=rtos-node1 addresses=\"\"", - "ovn-nbctl --timeout=15 set logical_switch node1 load_balancer=fakeTCPLoadBalancerUUID", - "ovn-nbctl --timeout=15 add logical_switch node1 load_balancer fakeUDPLoadBalancerUUID", - //adding the new node - "ovn-nbctl --timeout=15 --if-exists lrp-del rtos-newNode -- lrp-add ovn_cluster_router rtos-newNode ", - "ovn-nbctl --timeout=15 --may-exist ls-add newNode -- set logical_switch newNode", - "ovn-nbctl --timeout=15 set logical_switch newNode other-config:mcast_snoop=\"true\"", - "ovn-nbctl --timeout=15 set logical_switch newNode other-config:mcast_querier=\"false\"", - "ovn-nbctl --timeout=15 -- --may-exist lsp-add newNode stor-newNode -- set logical_switch_port stor-newNode type=router options:router-port=rtos-newNode addresses=\"\"", - "ovn-nbctl --timeout=15 set logical_switch newNode load_balancer=fakeTCPLoadBalancerUUID", - "ovn-nbctl --timeout=15 add logical_switch newNode load_balancer fakeUDPLoadBalancerUUID", - }) - - fExec.AddFakeCmdsNoOutputNoError([]string{ - //adding the original egressFirewall - "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1", - "ovn-nbctl --timeout=15 --id=@acl create acl priority=2000 direction=from-lport match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1 -- add logical_switch join_node1 acls @acl", - "ovn-nbctl --timeout=15 add logical_switch join_newNode acls " + fakeUUID, - }) - fExec.AddFakeCmd(&ovntest.ExpectedCmd{ - //query ovn and get the UUID of the original ACL - Cmd: "ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1", - Output: fakeUUID, - }) - - namespace1 := *newNamespace("namespace1") - egressFirewall := newEgressFirewallObject("default", namespace1.Name, []egressfirewallapi.EgressFirewallRule{ - { - Type: "Allow", - To: egressfirewallapi.EgressFirewallDestination{ - CIDRSelector: "1.2.3.4/23", - }, - }, - }) - newNode := &v1.Node{ - ObjectMeta: newObjectMeta("newNode", ""), - Status: v1.NodeStatus{ - Addresses: []v1.NodeAddress{ - {Type: v1.NodeInternalIP, Address: "10.0.0.0"}, - }, - }, - } - fakeOVN.fakeEgressClient = egressfirewallfake.NewSimpleClientset([]runtime.Object{ - &egressfirewallapi.EgressFirewallList{ - Items: []egressfirewallapi.EgressFirewall{ - *egressFirewall, - }, - }, - }...) - - fakeOVN.start(ctx, &v1.NamespaceList{ - Items: []v1.Namespace{ - namespace1, - }, - }, &v1.NodeList{ - Items: []v1.Node{ - { - Status: v1.NodeStatus{ - Phase: v1.NodeRunning, - }, - ObjectMeta: newObjectMeta(node1Name, ""), - }, - }, - }) - fakeOVN.controller.TCPLoadBalancerUUID = "fakeTCPLoadBalancerUUID" - fakeOVN.controller.UDPLoadBalancerUUID = "fakeUDPLoadBalancerUUID" - fakeOVN.controller.SCTPLoadBalancerUUID = "fakeSTCPLoadBalancerUUID" - fakeOVN.controller.WatchNodes() - fakeOVN.controller.WatchNamespaces() - fakeOVN.controller.WatchEgressFirewall() - - _, err := fakeOVN.fakeEgressClient.K8sV1().EgressFirewalls(egressFirewall.Namespace).Get(context.TODO(), egressFirewall.Name, metav1.GetOptions{}) - Expect(err).NotTo(HaveOccurred()) - - _, err = fakeOVN.fakeClient.CoreV1().Nodes().Create(context.TODO(), newNode, metav1.CreateOptions{}) - Expect(err).NotTo(HaveOccurred()) - - Eventually(fExec.CalledMatchesExpected).Should(BeTrue(), fExec.ErrorDesc) - - return nil - } - - err := app.Run([]string{app.Name}) - Expect(err).NotTo(HaveOccurred()) - }) }) diff --git a/go-controller/pkg/ovn/ovn.go b/go-controller/pkg/ovn/ovn.go index e8c00a8bf5..3ef0b382ab 100644 --- a/go-controller/pkg/ovn/ovn.go +++ b/go-controller/pkg/ovn/ovn.go @@ -940,28 +940,6 @@ func (oc *Controller) WatchNodes() { } gatewaysFailed.Store(node.Name, true) } - - //add any existing egressFirewall objects to join switch - namespaceList, err := oc.watchFactory.GetNamespaces() - if err != nil { - klog.Errorf("Error getting list of namespaces when adding node: %s", node.Name) - } - for _, namespace := range namespaceList { - nsInfo, err := oc.waitForNamespaceLocked(namespace.Name) - if err != nil { - klog.Errorf("Failed to wait for namespace %s event (%v)", - namespace.Name, err) - continue - } - if nsInfo.egressFirewallPolicy != nil { - err = nsInfo.egressFirewallPolicy.addACLToJoinSwitch([]string{joinSwitch(node.Name)}, nsInfo.addressSet.GetIPv4HashName(), nsInfo.addressSet.GetIPv6HashName()) - if err != nil { - klog.Errorf("%s", err) - } - } - - nsInfo.Unlock() - } }, UpdateFunc: func(old, new interface{}) { oldNode := old.(*kapi.Node) From 9e35514c88e6f50ee93ef08fb95f4365406219b3 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Sat, 19 Sep 2020 00:00:04 +0200 Subject: [PATCH 09/15] enable ipv6 ha local job in CI Signed-off-by: Antonio Ojea --- .github/workflows/test.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dea10851df..2e889cf33b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -179,13 +179,12 @@ jobs: # ENABLED v4 ha shared # ENABLED v4 noha local # DISABLED v4 noha shared - # DISABLED v6 ha local + # ENABLED v6 ha local # ENABLED v6 ha shared # DISABLED v6 noha local # DISABLED v6 noha shared - {"ipfamily": {"ip": ipv4}, "ha": {"enabled": "true"}, "gateway-mode": local} - {"ipfamily": {"ip": ipv4}, "ha": {"enabled": "false"}, "gateway-mode": shared} - - {"ipfamily": {"ip": ipv6}, "ha": {"enabled": "true"}, "gateway-mode": local} - {"ipfamily": {"ip": ipv6}, "ha": {"enabled": "false"}, "gateway-mode": local} - {"ipfamily": {"ip": ipv6}, "ha": {"enabled": "false"}, "gateway-mode": shared} needs: [build, k8s] From 67040b79404863a28af8eb84057dceac0aecf557 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 17 Sep 2020 15:18:25 -0400 Subject: [PATCH 10/15] node: ignore "deprecated" IPv6 IPs when determining primary IP Bare metal OCP adds temporary IPs to the primary interface for use with external load-balancing. These should be ignored when determining the primary node IP. Signed-off-by: Dan Winship --- go-controller/pkg/node/gateway_init.go | 25 ++++++++------------- go-controller/pkg/util/net_linux.go | 30 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/go-controller/pkg/node/gateway_init.go b/go-controller/pkg/node/gateway_init.go index c36f51d05f..404fd12d87 100644 --- a/go-controller/pkg/node/gateway_init.go +++ b/go-controller/pkg/node/gateway_init.go @@ -73,30 +73,23 @@ func bridgedGatewayNodeSetup(nodeName, bridgeName, bridgeInterface, physicalNetw // getNetworkInterfaceIPAddresses returns the IP addresses for the network interface 'iface'. func getNetworkInterfaceIPAddresses(iface string) ([]*net.IPNet, error) { - intf, err := net.InterfaceByName(iface) + allIPs, err := util.GetNetworkInterfaceIPs(iface) if err != nil { - return nil, err + return nil, fmt.Errorf("could not find IP addresses: %v", err) } - addrs, err := intf.Addrs() - if err != nil { - return nil, err - } var ips []*net.IPNet var foundIPv4 bool var foundIPv6 bool - for _, addr := range addrs { - switch ip := addr.(type) { - case *net.IPNet: - if utilnet.IsIPv6CIDR(ip) { - if config.IPv6Mode && !foundIPv6 { - ips = append(ips, ip) - foundIPv6 = true - } - } else if config.IPv4Mode && !foundIPv4 { + for _, ip := range allIPs { + if utilnet.IsIPv6CIDR(ip) { + if config.IPv6Mode && !foundIPv6 { ips = append(ips, ip) - foundIPv4 = true + foundIPv6 = true } + } else if config.IPv4Mode && !foundIPv4 { + ips = append(ips, ip) + foundIPv4 = true } } if config.IPv4Mode && !foundIPv4 { diff --git a/go-controller/pkg/util/net_linux.go b/go-controller/pkg/util/net_linux.go index 07f9ccab4d..4d2324ee89 100644 --- a/go-controller/pkg/util/net_linux.go +++ b/go-controller/pkg/util/net_linux.go @@ -11,6 +11,7 @@ import ( kapi "k8s.io/api/core/v1" "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" utilnet "k8s.io/utils/net" ) @@ -318,3 +319,32 @@ func DeleteConntrack(ip string, port int32, protocol kapi.Protocol) error { } return nil } + +// GetNetworkInterfaceIPs returns the IP addresses for the network interface 'iface'. +func GetNetworkInterfaceIPs(iface string) ([]*net.IPNet, error) { + link, err := netLinkOps.LinkByName(iface) + if err != nil { + return nil, fmt.Errorf("failed to lookup link %s: %v", iface, err) + } + + addrs, err := netLinkOps.AddrList(link, netlink.FAMILY_ALL) + if err != nil { + return nil, fmt.Errorf("failed to list addresses for %q: %v", iface, err) + } + + var ips []*net.IPNet + for _, addr := range addrs { + if addr.IP.IsLinkLocalUnicast() { + continue + } + // Ignore addresses marked as secondary or deprecated since they may + // disappear. (In bare metal clusters using MetalLB or similar, these + // flags are used to mark load balancer IPs that aren't permanently owned + // by the node). + if (addr.Flags & (unix.IFA_F_SECONDARY | unix.IFA_F_DEPRECATED)) != 0 { + continue + } + ips = append(ips, addr.IPNet) + } + return ips, nil +} From 996f2d288aa7f2709df29c2d42a2639d6435a356 Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Fri, 18 Sep 2020 21:12:56 -0400 Subject: [PATCH 11/15] Fixes inter node DGP policy for local gw mode The previous code assumed that the host network pod backend to a service would always be in the same subnet as the host accessing the service, which is not always the case. Additionally, the code used the mask on the host interface to determine the host subnet, which is not correct for IPv6. IPv6 addresses may be assigned with /128 prefixes. Since we really just care about non-pod subnet traffic from mp0 hitting this policy and going out DGP, we can just inverse the match to match not on pod subnet. Signed-off-by: Tim Rozet --- go-controller/pkg/ovn/gateway_init.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/go-controller/pkg/ovn/gateway_init.go b/go-controller/pkg/ovn/gateway_init.go index 8ea48f144b..bdfb43494c 100644 --- a/go-controller/pkg/ovn/gateway_init.go +++ b/go-controller/pkg/ovn/gateway_init.go @@ -495,11 +495,22 @@ func addPolicyBasedRoutes(nodeName, mgmtPortIP string, hostIfAddr *net.IPNet) er } if config.Gateway.Mode == config.GatewayModeLocal { - ipMask := hostIfAddr.IP.Mask(hostIfAddr.Mask) - hostNet := &net.IPNet{IP: ipMask, Mask: hostIfAddr.Mask} + var matchDst string // Local gw mode needs to use DGP to do hostA -> service -> hostB - matchStr = fmt.Sprintf("%s.src == %s && %s.dst == %s /* inter-%s */", - l3Prefix, mgmtPortIP, l3Prefix, hostNet.String(), nodeName) + var clusterL3Prefix string + for _, clusterSubnet := range config.Default.ClusterSubnets { + if utilnet.IsIPv6CIDR(clusterSubnet.CIDR) { + clusterL3Prefix = "ip6" + } else { + clusterL3Prefix = "ip4" + } + if l3Prefix != clusterL3Prefix { + continue + } + matchDst += fmt.Sprintf(" && %s.dst != %s", clusterL3Prefix, clusterSubnet.CIDR) + } + matchStr = fmt.Sprintf("%s.src == %s %s /* inter-%s */", + l3Prefix, mgmtPortIP, matchDst, nodeName) _, stderr, err = util.RunOVNNbctl("lr-policy-add", ovnClusterRouter, interNodePolicyPriority, matchStr, "reroute", natSubnetNextHop) if err != nil { From 8f3ce3888246d5945c649588fea345d1c61d60d0 Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Fri, 18 Sep 2020 22:26:53 -0400 Subject: [PATCH 12/15] Fixes lr-policy for egress gw Pods configured with egress gws were unable to reach other pods in other subnets, via service or otherwise. This is because the lr-policy for routing traffic to GR was not specific enough. This patch adds criteria for matching not cluster subnet, so only external traffic is forwarded to GR. Signed-off-by: Tim Rozet --- go-controller/pkg/ovn/egressgw.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/go-controller/pkg/ovn/egressgw.go b/go-controller/pkg/ovn/egressgw.go index 530a7569e6..ce171f8e10 100644 --- a/go-controller/pkg/ovn/egressgw.go +++ b/go-controller/pkg/ovn/egressgw.go @@ -349,7 +349,22 @@ func (oc *Controller) addHybridRoutePolicyForPod(podIP net.IP, node string) erro if err != nil { return fmt.Errorf("failed to parse gateway router join interface IP: %s, err: %v", grJoinIP, err) } + var matchDst string + var clusterL3Prefix string + for _, clusterSubnet := range config.Default.ClusterSubnets { + if utilnet.IsIPv6CIDR(clusterSubnet.CIDR) { + clusterL3Prefix = "ip6" + } else { + clusterL3Prefix = "ip4" + } + if l3Prefix != clusterL3Prefix { + continue + } + matchDst += fmt.Sprintf(" && %s.dst != %s", clusterL3Prefix, clusterSubnet.CIDR) + } + // traffic destined outside of cluster subnet go to GR matchStr := fmt.Sprintf(`inport == "rtos-%s" && %s.src == %s`, node, l3Prefix, podIP) + matchStr += matchDst _, stderr, err = util.RunOVNNbctl("lr-policy-add", ovnClusterRouter, "501", matchStr, "reroute", grJoinIP.String()) if err != nil { @@ -375,7 +390,21 @@ func (oc *Controller) delHybridRoutePolicyForPod(podIP net.IP, node string) erro } else { l3Prefix = "ip4" } + var matchDst string + var clusterL3Prefix string + for _, clusterSubnet := range config.Default.ClusterSubnets { + if utilnet.IsIPv6CIDR(clusterSubnet.CIDR) { + clusterL3Prefix = "ip6" + } else { + clusterL3Prefix = "ip4" + } + if l3Prefix != clusterL3Prefix { + continue + } + matchDst += fmt.Sprintf(" && %s.dst != %s", l3Prefix, clusterSubnet.CIDR) + } matchStr := fmt.Sprintf(`inport == "rtos-%s" && %s.src == %s`, node, l3Prefix, podIP) + matchStr += matchDst _, stderr, err := util.RunOVNNbctl("lr-policy-del", ovnClusterRouter, "501", matchStr) if err != nil { klog.Errorf("Failed to remove policy: %s, on: %s, stderr: %s, err: %v", From 82750b52898dbcf3496489580939394953acaa01 Mon Sep 17 00:00:00 2001 From: Jacob Tanenbaum Date: Mon, 21 Sep 2020 13:53:57 -0400 Subject: [PATCH 13/15] increase the priority of egressfirewall router policies Increase the priority of egressfirewall router policies to ensure that they get processed first. In order to make sure that no cluster traffic is interrrupted by the egressfirewall rules ensure that cluster traffic is explicitily excluded from the pattern match move all the constants for the priority level on the ovn_logical_router into one place so that all specified priority levels are centralized Signed-off-by: Jacob Tanenbaum --- go-controller/pkg/ovn/egressfirewall.go | 39 ++++++++++++++++---- go-controller/pkg/ovn/egressfirewall_test.go | 12 +++--- go-controller/pkg/ovn/egressip.go | 2 - go-controller/pkg/ovn/gateway.go | 14 +++++-- 4 files changed, 47 insertions(+), 20 deletions(-) diff --git a/go-controller/pkg/ovn/egressfirewall.go b/go-controller/pkg/ovn/egressfirewall.go index 291a460ea1..9e791f3177 100644 --- a/go-controller/pkg/ovn/egressfirewall.go +++ b/go-controller/pkg/ovn/egressfirewall.go @@ -3,8 +3,10 @@ package ovn import ( "fmt" "net" + "strconv" "strings" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" egressfirewallapi "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/egressfirewall/v1" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" @@ -15,7 +17,6 @@ import ( ) const ( - defaultStartPriority = 100 egressFirewallAppliedCorrectly = "EgressFirewall Rules applied" egressFirewallAddError = "EgressFirewall Rules not correctly added" egressFirewallUpdateError = "EgressFirewall Rules did not update correctly" @@ -81,9 +82,17 @@ func (oc *Controller) addEgressFirewall(egressFirewall *egressfirewallapi.Egress ef := newEgressFirewall(egressFirewall) nsInfo.egressFirewallPolicy = ef var errList []error + egressFirewallStartPriorityInt, err := strconv.Atoi(egressFirewallStartPriority) + if err != nil { + return []error{fmt.Errorf("failed to convert egressFirewallStartPriority to Integer: cannot add egressFirewall for namespace %s", egressFirewall.Namespace)} + } + minimumReservedEgressFirewallPriorityInt, err := strconv.Atoi(minimumReservedEgressFirewallPriority) + if err != nil { + return []error{fmt.Errorf("failed to convert minumumReservedEgressFirewallPriority to Integer: cannot add egressFirewall for namespace %s", egressFirewall.Namespace)} + } for i, egressFirewallRule := range egressFirewall.Spec.Egress { //process Rules into egressFirewallRules for egressFirewall struct - if i > defaultStartPriority { + if i > egressFirewallStartPriorityInt-minimumReservedEgressFirewallPriorityInt { klog.Warningf("egressFirewall for namespace %s has to many rules, the rest will be ignored", egressFirewall.Namespace) break } @@ -100,7 +109,7 @@ func (oc *Controller) addEgressFirewall(egressFirewall *egressfirewallapi.Egress return errList } - err = ef.addLogicalRouterPolicyToClusterRouter(nsInfo.addressSet.GetIPv4HashName(), nsInfo.addressSet.GetIPv6HashName()) + err = ef.addLogicalRouterPolicyToClusterRouter(nsInfo.addressSet.GetIPv4HashName(), nsInfo.addressSet.GetIPv6HashName(), egressFirewallStartPriorityInt) if err != nil { errList = append(errList, err) } @@ -151,7 +160,7 @@ func (oc *Controller) updateEgressFirewallWithRetry(egressfirewall *egressfirewa }) } -func (ef *egressFirewall) addLogicalRouterPolicyToClusterRouter(hashedAddressSetNameIPv4, hashedAddressSetNameIPv6 string) error { +func (ef *egressFirewall) addLogicalRouterPolicyToClusterRouter(hashedAddressSetNameIPv4, hashedAddressSetNameIPv6 string, efStartPriority int) error { for _, rule := range ef.egressRules { var match string var action string @@ -166,17 +175,19 @@ func (ef *egressFirewall) addLogicalRouterPolicyToClusterRouter(hashedAddressSet return fmt.Errorf("error rule.to.cidrSelector %s is not a valid CIDR (%+v)", rule.to.cidrSelector, err) } if !utilnet.IsIPv6(ipAddress) { - match = fmt.Sprintf("match=\"ip4.dst == %s && ip4.src == $%s\"", rule.to.cidrSelector, hashedAddressSetNameIPv4) + match = fmt.Sprintf("match=\"ip4.dst == %s && ip4.src == $%s", rule.to.cidrSelector, hashedAddressSetNameIPv4) } else { - match = fmt.Sprintf("match=\"ip6.dst == %s && ip6.src == $%s\"", rule.to.cidrSelector, hashedAddressSetNameIPv6) + match = fmt.Sprintf("match=\"ip6.dst == %s && ip6.src == $%s", rule.to.cidrSelector, hashedAddressSetNameIPv6) } if len(rule.ports) > 0 { - match = fmt.Sprintf("%s && ( %s )\"", match[:len(match)-1], egressGetL4Match(rule.ports)) + match = fmt.Sprintf("%s && ( %s )", match, egressGetL4Match(rule.ports)) } + match = fmt.Sprintf("%s && %s\"", match, getClusterSubnetsExclusion()) + _, stderr, err := util.RunOVNNbctl("--id=@logical_router_policy", "create", "logical_router_policy", - fmt.Sprintf("priority=%d", defaultStartPriority-rule.id), + fmt.Sprintf("priority=%d", efStartPriority-rule.id), match, "action="+action, fmt.Sprintf("external-ids:egressFirewall=%s", ef.namespace), "--", "add", "logical_router", ovnClusterRouter, "policies", "@logical_router_policy") if err != nil { @@ -253,3 +264,15 @@ func egressGetL4Match(ports []egressfirewallapi.EgressFirewallPort) string { } return l4Match } + +func getClusterSubnetsExclusion() string { + var exclusion string + for _, clusterSubnet := range config.Default.ClusterSubnets { + if utilnet.IsIPv6CIDR(clusterSubnet.CIDR) { + exclusion += fmt.Sprintf(" && %s.dst != %s", "ip6", clusterSubnet.CIDR) + } else { + exclusion += fmt.Sprintf(" && %s.dst != %s", "ip4", clusterSubnet.CIDR) + } + } + return exclusion[4:] +} diff --git a/go-controller/pkg/ovn/egressfirewall_test.go b/go-controller/pkg/ovn/egressfirewall_test.go index b2bf0d99c4..8570f81276 100644 --- a/go-controller/pkg/ovn/egressfirewall_test.go +++ b/go-controller/pkg/ovn/egressfirewall_test.go @@ -69,7 +69,7 @@ var _ = Describe("OVN EgressFirewall Operations", func() { node1Name string = "node1" ) fExec.AddFakeCmdsNoOutputNoError([]string{ - "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=10000 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102 && ip4.dst != 10.128.0.0/14\" action=allow external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", }) namespace1 := *newNamespace("namespace1") @@ -126,7 +126,7 @@ var _ = Describe("OVN EgressFirewall Operations", func() { node1Name string = "node1" ) fExec.AddFakeCmdsNoOutputNoError([]string{ - "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102 && ( (udp) )\" action=drop external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=10000 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102 && ( (udp) ) && ip4.dst != 10.128.0.0/14\" action=drop external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", }) namespace1 := *newNamespace("namespace1") egressFirewall := newEgressFirewallObject("default", namespace1.Name, []egressfirewallapi.EgressFirewallRule{ @@ -182,8 +182,8 @@ var _ = Describe("OVN EgressFirewall Operations", func() { node1Name string = "node1" ) fExec.AddFakeCmdsNoOutputNoError([]string{ - "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.5/23 && " + - "ip4.src == $a10481622940199974102 && ( (udp && ( udp.dst == 100 )) || (tcp) )\" action=allow external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=10000 match=\"ip4.dst == 1.2.3.5/23 && " + + "ip4.src == $a10481622940199974102 && ( (udp && ( udp.dst == 100 )) || (tcp) ) && ip4.dst != 10.128.0.0/14\" action=allow external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", "ovn-nbctl --timeout=15 lr-policy-del ovn_cluster_router " + fmt.Sprintf("%s", fakeUUID), }) fExec.AddFakeCmd(&ovntest.ExpectedCmd{ @@ -251,8 +251,8 @@ var _ = Describe("OVN EgressFirewall Operations", func() { node1Name string = "node1" ) fExec.AddFakeCmdsNoOutputNoError([]string{ - "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=allow external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", - "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=100 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102\" action=drop external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=10000 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102 && ip4.dst != 10.128.0.0/14\" action=allow external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", + "ovn-nbctl --timeout=15 --id=@logical_router_policy create logical_router_policy priority=10000 match=\"ip4.dst == 1.2.3.4/23 && ip4.src == $a10481622940199974102 && ip4.dst != 10.128.0.0/14\" action=drop external-ids:egressFirewall=namespace1 -- add logical_router ovn_cluster_router policies @logical_router_policy", "ovn-nbctl --timeout=15 lr-policy-del ovn_cluster_router " + fmt.Sprintf("%s", fakeUUID), }) fExec.AddFakeCmd(&ovntest.ExpectedCmd{ diff --git a/go-controller/pkg/ovn/egressip.go b/go-controller/pkg/ovn/egressip.go index 93afda9c84..f439b5194f 100644 --- a/go-controller/pkg/ovn/egressip.go +++ b/go-controller/pkg/ovn/egressip.go @@ -21,8 +21,6 @@ import ( ) const ( - defaultNoRereoutePriority = "101" - egressIPReroutePriority = "100" // In case we restart we need accept executing ovn-nbctl commands with this error. // The ovn-nbctl API does not support `--may-exist` for `lr-policy-add` policyAlreadyExistsMsg = "Same routing policy already existed" diff --git a/go-controller/pkg/ovn/gateway.go b/go-controller/pkg/ovn/gateway.go index 2461e23e0c..40689206a9 100644 --- a/go-controller/pkg/ovn/gateway.go +++ b/go-controller/pkg/ovn/gateway.go @@ -26,10 +26,16 @@ const ( extSwitchToGwRouterPrefix = "etor-" gwRouterToExtSwitchPrefix = "rtoe-" - nodeLocalSwitch = "node_local_switch" - interNodePolicyPriority = "1003" - nodeSubnetPolicyPriority = "1004" - mgmtPortPolicyPriority = "1005" + nodeLocalSwitch = "node_local_switch" + + // prority of logical router policies on the ovnClusterRouter + egressFirewallStartPriority = "10000" + minimumReservedEgressFirewallPriority = "2000" + mgmtPortPolicyPriority = "1005" + nodeSubnetPolicyPriority = "1004" + interNodePolicyPriority = "1003" + defaultNoRereoutePriority = "101" + egressIPReroutePriority = "100" ) func (ovn *Controller) getOvnGateways() ([]string, string, error) { From d0e548c3d06b559a800d37bb2e4f1718b531c638 Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Tue, 22 Sep 2020 14:23:10 -0400 Subject: [PATCH 14/15] Allows for deploying local gateway mode without a shared bridge Since local gateway mode only needs the shared gateway bridge for egress gw and potentially egress IP features, we can disable it by passing a gateway-interface of "none". We need this temporarily to be able to upgrade from a previous local gateway cluster to a local gateway mode using the shared topology. Signed-off-by: Tim Rozet --- go-controller/pkg/node/gateway_shared_intf.go | 38 ++++++++- go-controller/pkg/ovn/endpoints.go | 9 ++- go-controller/pkg/ovn/gateway_init.go | 79 +++++++++++++++++++ go-controller/pkg/ovn/master.go | 17 +++- 4 files changed, 136 insertions(+), 7 deletions(-) diff --git a/go-controller/pkg/node/gateway_shared_intf.go b/go-controller/pkg/node/gateway_shared_intf.go index 53ca299d72..879dceb6eb 100644 --- a/go-controller/pkg/node/gateway_shared_intf.go +++ b/go-controller/pkg/node/gateway_shared_intf.go @@ -522,7 +522,41 @@ func (n *OvnNode) initSharedGateway(subnets []*net.IPNet, gwNextHops []net.IP, g var brCreated bool var err error - if bridgeName, _, err = util.RunOVSVsctl("--", "port-to-br", gwIntf); err == nil { + // OCP HACK + if gwIntf == "none" { + err = setupLocalNodeAccessBridge(n.name, subnets) + if err != nil { + return nil, err + } + chassisID, err := util.GetNodeChassisID() + if err != nil { + return nil, err + } + // get the real default interface + defaultGatewayIntf, _, err := getDefaultGatewayInterfaceDetails() + if err != nil { + return nil, err + } + ips, err := getNetworkInterfaceIPAddresses(defaultGatewayIntf) + if err != nil { + return nil, fmt.Errorf("failed to get interface details for %s (%v)", + gwIntf, err) + } + err = util.SetL3GatewayConfig(nodeAnnotator, &util.L3GatewayConfig{ + ChassisID: chassisID, + Mode: config.GatewayModeLocal, + IPAddresses: ips, + MACAddress: util.IPAddrToHWAddr(ips[0].IP), + NextHops: gwNextHops, + NodePortEnable: config.Gateway.NodeportEnable, + }) + if err != nil { + return nil, err + } else { + return func() error { return nil }, nil + } + // END OCP HACK + } else if bridgeName, _, err = util.RunOVSVsctl("--", "port-to-br", gwIntf); err == nil { // This is an OVS bridge's internal port uplinkName, err = util.GetNicName(bridgeName) if err != nil { @@ -586,7 +620,7 @@ func (n *OvnNode) initSharedGateway(subnets []*net.IPNet, gwNextHops []net.IP, g return func() error { if config.Gateway.NodeportEnable { - if config.Gateway.Mode == config.GatewayModeLocal { + if config.Gateway.Mode == config.GatewayModeLocal && gwIntf != "None" { if err := addDefaultConntrackRulesLocal(n.name, bridgeName, uplinkName, n.stopChan); err != nil { return err } diff --git a/go-controller/pkg/ovn/endpoints.go b/go-controller/pkg/ovn/endpoints.go index 1b79541ce3..e1c69878cf 100644 --- a/go-controller/pkg/ovn/endpoints.go +++ b/go-controller/pkg/ovn/endpoints.go @@ -2,6 +2,7 @@ package ovn import ( "fmt" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" @@ -114,9 +115,13 @@ func (ovn *Controller) AddEndpoints(ep *kapi.Endpoints) error { func (ovn *Controller) handleNodePortLB(node *kapi.Node) error { gatewayRouter := gwRouterPrefix + node.Name var physicalIPs []string - if physicalIPs, _ = ovn.getGatewayPhysicalIPs(gatewayRouter); physicalIPs == nil { - return fmt.Errorf("gateway physical IP for node %q does not yet exist", node.Name) + // OCP HACK + if config.Gateway.Interface != "none" { + if physicalIPs, _ = ovn.getGatewayPhysicalIPs(gatewayRouter); physicalIPs == nil { + return fmt.Errorf("gateway physical IP for node %q does not yet exist", node.Name) + } } + // END OCP HACK namespaces, err := ovn.watchFactory.GetNamespaces() if err != nil { return fmt.Errorf("failed to get k8s namespaces: %v", err) diff --git a/go-controller/pkg/ovn/gateway_init.go b/go-controller/pkg/ovn/gateway_init.go index bdfb43494c..18098a5163 100644 --- a/go-controller/pkg/ovn/gateway_init.go +++ b/go-controller/pkg/ovn/gateway_init.go @@ -588,3 +588,82 @@ func (oc *Controller) addNodeLocalNatEntries(node *kapi.Node, mgmtPortMAC string } return nil } + +// OCP HACK +func createNodePortLoadBalancers(gatewayRouter, nodeName string, sctpSupport bool) error { + // Create 3 load-balancers for north-south traffic for each gateway + // router: UDP, TCP, SCTP + k8sNSLbTCP, k8sNSLbUDP, k8sNSLbSCTP, err := getGatewayLoadBalancers(gatewayRouter) + if err != nil { + return err + } + protoLBMap := map[kapi.Protocol]string{ + kapi.ProtocolTCP: k8sNSLbTCP, + kapi.ProtocolUDP: k8sNSLbUDP, + kapi.ProtocolSCTP: k8sNSLbSCTP, + } + enabledProtos := []kapi.Protocol{kapi.ProtocolTCP, kapi.ProtocolUDP} + if sctpSupport { + enabledProtos = append(enabledProtos, kapi.ProtocolSCTP) + } + var stdout, stderr string + for _, proto := range enabledProtos { + if protoLBMap[proto] == "" { + protoLBMap[proto], stderr, err = util.RunOVNNbctl("--", "create", + "load_balancer", + fmt.Sprintf("external_ids:%s_lb_gateway_router=%s", proto, gatewayRouter), + fmt.Sprintf("protocol=%s", strings.ToLower(string(proto)))) + if err != nil { + return fmt.Errorf("failed to create load balancer for gateway router %s for protocol %s: "+ + "stderr: %q, error: %v", gatewayRouter, proto, stderr, err) + } + } + } + + // Local gateway mode does not use GR for ingress node port traffic, it uses mp0 instead + if config.Gateway.Mode != config.GatewayModeLocal { + // Add north-south load-balancers to the gateway router. + lbString := fmt.Sprintf("%s,%s", protoLBMap[kapi.ProtocolTCP], protoLBMap[kapi.ProtocolUDP]) + if sctpSupport { + lbString = lbString + "," + protoLBMap[kapi.ProtocolSCTP] + } + stdout, stderr, err = util.RunOVNNbctl("set", "logical_router", gatewayRouter, "load_balancer="+lbString) + if err != nil { + return fmt.Errorf("failed to set north-south load-balancers to the "+ + "gateway router %s, stdout: %q, stderr: %q, error: %v", + gatewayRouter, stdout, stderr, err) + } + } + // Also add north-south load-balancers to local switches for pod -> nodePort traffic + stdout, stderr, err = util.RunOVNNbctl("get", "logical_switch", nodeName, "load_balancer") + if err != nil { + return fmt.Errorf("failed to get load-balancers on the node switch %s, stdout: %q, "+ + "stderr: %q, error: %v", nodeName, stdout, stderr, err) + } + for _, proto := range enabledProtos { + if !strings.Contains(stdout, protoLBMap[proto]) { + stdout, stderr, err = util.RunOVNNbctl("ls-lb-add", nodeName, protoLBMap[proto]) + if err != nil { + return fmt.Errorf("failed to add north-south load-balancer %s to the "+ + "node switch %s, stdout: %q, stderr: %q, error: %v", + protoLBMap[proto], nodeName, stdout, stderr, err) + } + } + } + return nil +} + +// gatewayInitMinimal sets up the minimal configuration needed for N/S traffic to work in Local gateway mode. In this +// case we do not need any GR or join switch, just nodePort load balancers on the node switch +func gatewayInitMinimal(nodeName string, l3GatewayConfig *util.L3GatewayConfig, sctpSupport bool) error { + gatewayRouter := gwRouterPrefix + nodeName + if l3GatewayConfig.NodePortEnable { + err := createNodePortLoadBalancers(gatewayRouter, nodeName, sctpSupport) + if err != nil { + return err + } + } + return nil +} + +// END OCP HACK diff --git a/go-controller/pkg/ovn/master.go b/go-controller/pkg/ovn/master.go index 3146e4ecd9..453a262232 100644 --- a/go-controller/pkg/ovn/master.go +++ b/go-controller/pkg/ovn/master.go @@ -449,9 +449,20 @@ func (oc *Controller) syncGatewayLogicalNetwork(node *kapi.Node, l3GatewayConfig return err } - err = gatewayInit(node.Name, clusterSubnets, hostSubnets, joinSubnets, l3GatewayConfig, oc.SCTPSupport) - if err != nil { - return fmt.Errorf("failed to init shared interface gateway: %v", err) + // OCP HACK + // GatewayModeLocal is only used if Local mode is specified and None shared gateway bridge is specified + // This is to allow local gateway mode without having to configure/use the shared gateway bridge + if l3GatewayConfig.Mode == config.GatewayModeLocal { + err = gatewayInitMinimal(node.Name, l3GatewayConfig, oc.SCTPSupport) + if err != nil { + return fmt.Errorf("failed to init shared interface gateway: %v", err) + } + // END OCP HACK + } else { + err = gatewayInit(node.Name, clusterSubnets, hostSubnets, joinSubnets, l3GatewayConfig, oc.SCTPSupport) + if err != nil { + return fmt.Errorf("failed to init shared interface gateway: %v", err) + } } // in the case of shared gateway mode, we need to setup From 4dbf0ae3ba7ea8a40d5a9d5ffe54e810fa8e152c Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Tue, 22 Sep 2020 16:06:09 -0400 Subject: [PATCH 15/15] DNM hack to just test local gw function no bridge Signed-off-by: Tim Rozet --- go-controller/pkg/config/config.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go-controller/pkg/config/config.go b/go-controller/pkg/config/config.go index 60b15038b5..4e5f83300e 100644 --- a/go-controller/pkg/config/config.go +++ b/go-controller/pkg/config/config.go @@ -1113,6 +1113,9 @@ func buildGatewayConfig(ctx *cli.Context, cli, file *config) error { // HACK force local gateway mode Gateway.Mode = GatewayModeLocal + // HACK force gateway interface none + Gateway.Interface = "none" + return nil }