[release-4.15] OCPBUGS-43605: backport fix for network policy during live migration

pkg/network/node/networkpolicy.go

@@ -3,6 +3,9 @@ package node
 import (
 	"context"
 	"fmt"
+	"math"
+	"net"
+	"os"
 	"reflect"
 	"sort"
 	"strings"
@@ -35,6 +38,8 @@ const HostNetworkNamespace = "openshift-host-network"
 // only used by network policy functions
 const NetPolIsolationCookie = 1147582955
 
+const migrationEnvVar = "NODE_CNI"
+
 type networkPolicyPlugin struct {
 	node   *OsdnNode
 	vnids  *nodeVNIDMap
@@ -63,6 +68,9 @@ type networkPolicyPlugin struct {
 	// into unProcessedVNIDs in EnsureVNIDRules() method and it can be looked up later in
 	// AddNetNamespace() method so that required nw policy flow rules programmed into OVS.
 	unProcessedVNIDs sets.Int32
+
+	// indicate if the node is running in SDN live migration mode
+	inMigrationMode bool
 }
 
 // npNamespace tracks NetworkPolicy-related data for a Namespace
@@ -116,16 +124,17 @@ const (
 )
 
 func NewNetworkPolicyPlugin() osdnPolicy {
+	_, inMigration := os.LookupEnv(migrationEnvVar)
 	return &networkPolicyPlugin{
 		namespaces:       make(map[uint32]*npNamespace),
 		namespacesByName: make(map[string]*npNamespace),
 
 		nsMatchCache: make(map[string]*npCacheEntry),
 
-		warnedPolicies:  make(map[ktypes.UID]string),
-		skippedPolicies: make(map[ktypes.UID]string),
-		localPodIPs:     make(map[ktypes.UID]string),
-
+		warnedPolicies:   make(map[ktypes.UID]string),
+		skippedPolicies:  make(map[ktypes.UID]string),
+		localPodIPs:      make(map[ktypes.UID]string),
+		inMigrationMode:  inMigration,
 		unProcessedVNIDs: sets.NewInt32(),
 	}
 }
@@ -775,6 +784,12 @@ func (np *networkPolicyPlugin) parsePeerFlows(npns *npNamespace, npp *npPolicy,
 			if dir == ingressFlow && (len(peer.PodSelector.MatchLabels) == 0 && len(peer.PodSelector.MatchExpressions) == 0) {
 				// The PodSelector is empty, meaning it selects all pods in this namespace
 				peerFlows = append(peerFlows, fmt.Sprintf("reg0=%d, ", npns.vnid))
+				if np.inMigrationMode {
+					npp.watchesOwnPods = true
+					for _, ip := range np.selectPods(npns, peer.PodSelector) {
+						peerFlows = append(peerFlows, fmt.Sprintf("ip, nw_src=%s, ", ip))
+					}
+				}
 			} else {
 				npp.watchesOwnPods = true
 				for _, ip := range np.selectPods(npns, peer.PodSelector) {
@@ -794,6 +809,31 @@ func (np *networkPolicyPlugin) parsePeerFlows(npns *npNamespace, npp *npPolicy,
 				// checking the source VNID...
 				npp.watchesNamespaces = true
 				peerFlows = append(peerFlows, np.selectNamespaces(peer.NamespaceSelector, dir)...)
+				if np.inMigrationMode {
+					// In SDN live migration mode, we can't use source VNID to
+					// distinguish namespaces for traffic from OVN nodes.
+					// So instead we pretend the rule was a combined
+					// namespaceSelector+podSelector rule with a match-all
+					// podSelector, and generate per-pod-IP match rules.
+					npp.watchesAllPods = true
+					peerFlows = append(peerFlows, np.selectPodsFromNamespaces(peer.NamespaceSelector, &metav1.LabelSelector{}, dir)...)
+
+					// If the host network namespace is selected, Add rules for
+					// the OVN mp0 IP of each node.
+					sel, _ := metav1.LabelSelectorAsSelector(peer.NamespaceSelector)
+					if _, ok := np.nsMatchCache[sel.String()].matches[HostNetworkNamespace]; ok {
+						for _, network := range np.node.networkInfo.ClusterNetworks {
+							cidrIP := make(net.IP, len(network.ClusterCIDR.IP))
+							copy(cidrIP, network.ClusterCIDR.IP)
+							cidrIP[3] = cidrIP[3] | 0x2
+							maskLen, _ := network.ClusterCIDR.Mask.Size()
+							// use a mask to match the OVN mp0 IP which is the second IP of each host subnet.
+							mask := uint32(math.MaxUint32<<(32-maskLen) | math.MaxUint32>>(32-network.HostSubnetLength))
+							flow := fmt.Sprintf("ip, nw_src=%s/%d.%d.%d.%d, ", cidrIP, (mask>>24)&0xff, (mask>>16)&0xff, (mask>>8)&0xff, mask&0xff)
+							peerFlows = append(peerFlows, flow)
+						}
+					}
+				}
 			} else {
 				// ... but for namespaceSelectors on egress, we can't just
 				// check the destination VNID because we don't know that

pkg/network/node/networkpolicy_test.go

@@ -3,6 +3,7 @@ package node
 import (
 	"context"
 	"fmt"
+	"net"
 	"strings"
 	"sync/atomic"
 	"testing"
@@ -21,6 +22,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/async"
 
 	osdnv1 "github.com/openshift/api/network/v1"
+	"github.com/openshift/sdn/pkg/network/common"
 	"github.com/openshift/sdn/pkg/util/ovs"
 )
 
@@ -932,6 +934,162 @@ func TestNetworkPolicy(t *testing.T) {
 	}
 }
 
+func TestNetworkPolicyInMigrationMode(t *testing.T) {
+	var err error
+	np, _, synced, stopCh := newTestNPP()
+	np.inMigrationMode = true
+	_, clusterCIDR, _ := net.ParseCIDR("10.128.0.0/14")
+	np.node.networkInfo = &common.ParsedClusterNetwork{
+		ClusterNetworks: []common.ParsedClusterNetworkEntry{
+			{
+				ClusterCIDR:      clusterCIDR,
+				HostSubnetLength: 9,
+			},
+		},
+	}
+	defer close(stopCh)
+
+	// Create some Namespaces
+	addNamespace(np, "default", 0, map[string]string{"default": "true"})
+	addNamespace(np, "one", 1, map[string]string{"parity": "odd"})
+	addNamespace(np, "two", 2, map[string]string{"parity": "even"})
+
+	npns1 := np.namespaces[1]
+	npns2 := np.namespaces[2]
+
+	addPods(np, npns1)
+	addPods(np, npns2)
+
+	// Allow client pods in the same namespace to connect to the server in namespace "one"
+	synced.Store(false)
+	addNetworkPolicy(np, &networkingv1.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "allow-from-samenamespace",
+			UID:       uid(npns1, "allow-from-samenamespace"),
+			Namespace: "one",
+		},
+		Spec: networkingv1.NetworkPolicySpec{
+			PodSelector: metav1.LabelSelector{
+				MatchLabels: map[string]string{},
+			},
+			PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
+			Ingress: []networkingv1.NetworkPolicyIngressRule{{
+				From: []networkingv1.NetworkPolicyPeer{{
+					PodSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{},
+					},
+				}},
+			}},
+		},
+	})
+	waitForSync(np, synced, "networkpolicy sync")
+
+	err = assertPolicies(np, npns1, 1, map[string]*npPolicy{
+		"allow-from-samenamespace": {
+			watchesNamespaces: false,
+			watchesAllPods:    false,
+			watchesOwnPods:    true,
+			ingressFlows: []string{
+				fmt.Sprintf("reg0=%d", npns1.vnid),
+				fmt.Sprintf("ip, nw_src=%s", clientIP(npns1)),
+				fmt.Sprintf("ip, nw_src=%s", serverIP(npns1)),
+			},
+		},
+	})
+	if err != nil {
+		t.Error(err.Error())
+	}
+
+	// Allow client pods in even namespaces to connect to the server in namespace "one"
+	synced.Store(false)
+	addNetworkPolicy(np, &networkingv1.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "allow-from-even",
+			UID:       uid(npns1, "allow-from-even"),
+			Namespace: "one",
+		},
+		Spec: networkingv1.NetworkPolicySpec{
+			PodSelector: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"kind": "server",
+				},
+			},
+			PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
+			Ingress: []networkingv1.NetworkPolicyIngressRule{{
+				From: []networkingv1.NetworkPolicyPeer{{
+					NamespaceSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"parity": "even",
+						},
+					},
+				}},
+			}},
+		},
+	})
+	waitForSync(np, synced, "networkpolicy sync")
+
+	err = assertPolicies(np, npns1, 2, map[string]*npPolicy{
+		"allow-from-even": {
+			watchesNamespaces: true,
+			watchesAllPods:    true,
+			watchesOwnPods:    true,
+			ingressFlows: []string{
+				fmt.Sprintf("ip, nw_dst=%s, reg0=%d", serverIP(npns1), npns2.vnid),
+				fmt.Sprintf("ip, nw_dst=%s, ip, nw_src=%s", serverIP(npns1), clientIP(npns2)),
+				fmt.Sprintf("ip, nw_dst=%s, ip, nw_src=%s", serverIP(npns1), serverIP(npns2)),
+			},
+		},
+	})
+	if err != nil {
+		t.Error(err.Error())
+	}
+
+	// Create the special namespace that indicates host network traffic
+	addNamespace(np, "openshift-host-network", 200, map[string]string{"network.openshift.io/policy-group": "ingress"})
+	// Create the namespace to add network policy for
+	addNamespace(np, "host-network-target", 10, map[string]string{"foo": "bar"})
+	npns := np.namespaces[10]
+	synced.Store(false)
+	addNetworkPolicy(np, &networkingv1.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "allow-from-host-network-ns",
+			UID:       uid(npns, "allow-from-host-network-ns"),
+			Namespace: npns.name,
+		},
+		Spec: networkingv1.NetworkPolicySpec{
+			PodSelector: metav1.LabelSelector{},
+			PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
+			Ingress: []networkingv1.NetworkPolicyIngressRule{{
+				From: []networkingv1.NetworkPolicyPeer{{
+					NamespaceSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"network.openshift.io/policy-group": "ingress",
+						},
+					},
+				}},
+			}},
+		},
+	})
+	waitForSync(np, synced, "host-network NP addition")
+
+	// make sure we add the right flows
+	err = assertPolicies(np, npns, 1, map[string]*npPolicy{
+		"allow-from-host-network-ns": {
+			watchesNamespaces: true,
+			watchesAllPods:    true,
+			watchesOwnPods:    false,
+			ingressFlows: []string{
+				"reg0=0", //make sure host network namespace is classified into vnid 0
+				// allow the traffic from the second IP of the node subnet which is the onv-k mp0 interface IP
+				"ip, nw_src=10.128.0.2/255.252.1.255",
+			},
+		},
+	})
+	if err != nil {
+		t.Error(err.Error())
+	}
+}
+
 func TestNetworkPolicy_ipBlock(t *testing.T) {
 	np, _, synced, stopCh := newTestNPP()
 	defer close(stopCh)

pkg/network/node/ovscontroller.go

@@ -183,7 +183,7 @@ const (
 	Vxlan0 = "vxlan0"
 
 	// rule versioning; increment each time flow rules change
-	ruleVersion = 13
+	ruleVersion = 14
 
 	ruleVersionTable = 253
 )

pkg/network/node/ovscontroller_test.go

@@ -1060,7 +1060,7 @@ var expectedFlows = []string{
 	" cookie=0, table=111, priority=100, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:10.0.123.45->tun_dst,output:1,set_field:10.0.45.123->tun_dst,output:1,goto_table:120",
 	" cookie=0, table=120, priority=100, reg0=99, actions=output:4,output:5,output:6",
 	" cookie=0, table=120, priority=0, actions=drop",
-	" cookie=0, table=253, actions=note:00.0D",
+	" cookie=0, table=253, actions=note:00.0E",
 }
 
 // Ensure that we do not change the OVS flows without bumping ruleVersion