Backport Fixups

Commit fixing conflicts and other issues for
backporting openshift#686

to 4.7 without the smartnic code

Signed-off-by: astoycos <astoycos@redhat.com>

go-controller/pkg/cni/cni.go

@@ -1,11 +1,9 @@
 package cni
 
 import (
-	"context"
 	"encoding/json"
 	"fmt"
 	"net"
-	"time"
 
 	"k8s.io/client-go/kubernetes"
 	corev1listers "k8s.io/client-go/listers/core/v1"
@@ -17,7 +15,6 @@ import (
 	utilnet "k8s.io/utils/net"
 
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
-	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
 )
 
 var (
@@ -103,10 +100,12 @@ func (pr *PodRequest) cmdAdd(kubeAuth *KubeAPIAuth, podLister corev1listers.PodL
 		return nil, fmt.Errorf("required CNI variable missing")
 	}
 
+	annotCondFn := isOvnReady
+
 	// Get the IP address and MAC address of the pod
-	annotations, err := getPodAnnotations(pr.ctx, podLister, pr.PodNamespace, pr.PodName)
+	podUID, annotations, err := GetPodAnnotations(pr.ctx, podLister, kclient, namespace, podName, annotCondFn)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get pod annotation: %v", err)
 	}
 	if err := pr.checkOrUpdatePodUID(podUID); err != nil {
 		return nil, err
@@ -150,7 +149,9 @@ func (pr *PodRequest) cmdCheck(podLister corev1listers.PodLister, useOVSExternal
 	}
 
 	// Get the IP address and MAC address of the pod
-	annotations, err := getPodAnnotations(pr.ctx, podLister, pr.PodNamespace, pr.PodName)
+	annotCondFn := isOvnReady
+
+	podUID, annotations, err := GetPodAnnotations(pr.ctx, podLister, kclient, pr.PodNamespace, pr.PodName, annotCondFn)
 	if err != nil {
 		return nil, err
 	}
@@ -271,27 +272,3 @@ func (pr *PodRequest) getCNIResult(podLister corev1listers.PodLister, kclient ku
 		IPs:        ips,
 	}, nil
 }
-
-// getPodAnnotations obtains the pod annotation from the cache
-func getPodAnnotations(ctx context.Context, podLister corev1listers.PodLister, namespace, name string) (map[string]string, error) {
-	timeout := time.After(30 * time.Second)
-	for {
-		select {
-		case <-ctx.Done():
-			return nil, fmt.Errorf("canceled waiting for annotations")
-		case <-timeout:
-			return nil, fmt.Errorf("timed out waiting for annotations")
-		default:
-			pod, err := podLister.Pods(namespace).Get(name)
-			if err != nil {
-				return nil, fmt.Errorf("failed to get annotations: %v", err)
-			}
-			annotations := pod.ObjectMeta.Annotations
-			if _, ok := annotations[util.OvnPodAnnotationName]; ok {
-				return annotations, nil
-			}
-			// try again later
-			time.Sleep(200 * time.Millisecond)
-		}
-	}
-}

go-controller/pkg/cni/cniserver.go

@@ -47,12 +47,18 @@ import (
 // started.
 
 // NewCNIServer creates and returns a new Server object which will listen on a socket in the given path
-func NewCNIServer(rundir string, factory factory.NodeWatchFactory) *Server {
+func NewCNIServer(rundir string, useOVSExternalIDs bool, factory factory.NodeWatchFactory, kclient kubernetes.Interface) (*Server, error) {
 	if len(rundir) == 0 {
 		rundir = serverRunDir
 	}
 	router := mux.NewRouter()
 
+	// we use atomic lib to store port binding mode state, so use int32 to represent bool
+	var ovnPortBinding int32
+	if useOVSExternalIDs {
+		ovnPortBinding = 1
+	}
+
 	s := &Server{
 		Server: http.Server{
 			Handler: router,
@@ -61,7 +67,6 @@ func NewCNIServer(rundir string, factory factory.NodeWatchFactory) *Server {
 		useOVSExternalIDs: ovnPortBinding,
 		podLister:         corev1listers.NewPodLister(factory.LocalPodInformer().GetIndexer()),
 		kclient:           kclient,
-		mode:              config.OvnKubeNode.Mode,
 		kubeAuth: &KubeAPIAuth{
 			Kubeconfig:    config.Kubernetes.Kubeconfig,
 			KubeAPIServer: config.Kubernetes.APIServer,

go-controller/pkg/cni/cniserver_test.go

@@ -17,6 +17,7 @@ import (
 
 	cnitypes "github.com/containernetworking/cni/pkg/types"
 	cni020 "github.com/containernetworking/cni/pkg/types/020"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	corev1listers "k8s.io/client-go/listers/core/v1"
 	utiltesting "k8s.io/client-go/util/testing"
@@ -47,11 +48,7 @@ func clientDoCNI(t *testing.T, client *http.Client, req *Request) ([]byte, int)
 
 var expectedResult cnitypes.Result
 
-<<<<<<< HEAD
-func serverHandleCNI(request *PodRequest, podLister corev1listers.PodLister) ([]byte, error) {
-=======
 func serverHandleCNI(request *PodRequest, podLister corev1listers.PodLister, useOVSExternalIDs bool, kclient kubernetes.Interface, kubeAuth *KubeAPIAuth) ([]byte, error) {
->>>>>>> 2171779d (cni: pass Kube API auth via cnishim response, not CNI config file)
 	if request.Command == CNIAdd {
 		return json.Marshal(&expectedResult)
 	} else if request.Command == CNIDel || request.Command == CNIUpdate || request.Command == CNICheck {

go-controller/pkg/cni/helper_linux.go

@@ -302,30 +302,30 @@ func ConfigureOVS(ctx context.Context, namespace, podName, hostIfaceName string,
 
 	// Add the new sandbox's OVS port
 	ovsArgs := []string{
-		"add-port", "br-int", hostIface.Name, "--", "set",
-		"interface", hostIface.Name,
+		"add-port", "br-int", hostIfaceName, "--", "set",
+		"interface", hostIfaceName,
 		fmt.Sprintf("external_ids:attached_mac=%s", ifInfo.MAC),
 		fmt.Sprintf("external_ids:iface-id=%s", ifaceID),
 		fmt.Sprintf("external_ids:ip_addresses=%s", strings.Join(ipStrs, ",")),
-		fmt.Sprintf("external_ids:sandbox=%s", pr.SandboxID),
+		fmt.Sprintf("external_ids:sandbox=%s", sandboxID),
 	}
 
 	if out, err := ovsExec(ovsArgs...); err != nil {
-		return nil, fmt.Errorf("failure in plugging pod interface: %v\n  %q", err, out)
+		return fmt.Errorf("failure in plugging pod interface: %v\n  %q", err, out)
 	}
 
-	if err := clearPodBandwidth(pr.SandboxID); err != nil {
-		return nil, err
+	if err := clearPodBandwidth(sandboxID); err != nil {
+		return err
 	}
 
 	if ifInfo.Ingress > 0 || ifInfo.Egress > 0 {
-		l, err := netlink.LinkByName(hostIface.Name)
+		l, err := netlink.LinkByName(hostIfaceName)
 		if err != nil {
-			return nil, fmt.Errorf("failed to find host veth interface %s: %v", hostIface.Name, err)
+			return fmt.Errorf("failed to find host veth interface %s: %v", hostIfaceName, err)
 		}
 		err = netlink.LinkSetTxQLen(l, 1000)
 		if err != nil {
-			return nil, fmt.Errorf("failed to set host veth txqlen: %v", err)
+			return fmt.Errorf("failed to set host veth txqlen: %v", err)
 		}
 
 		if err := setPodBandwidth(sandboxID, hostIfaceName, ifInfo.Ingress, ifInfo.Egress); err != nil {
@@ -359,28 +359,23 @@ func (pr *PodRequest) ConfigureInterface(podLister corev1listers.PodLister, kcli
 
 	var hostIface, contIface *current.Interface
 
-	klog.V(5).Infof("CNI Conf %v", pr.CNIConf)
+	klog.Infof("CNI Conf %v", pr.CNIConf)
 	if pr.CNIConf.DeviceID != "" {
 		// SR-IOV Case
 		hostIface, contIface, err = setupSriovInterface(netns, pr.SandboxID, pr.IfName, ifInfo, pr.CNIConf.DeviceID)
 	} else {
-		if pr.IsSmartNIC {
-			return nil, fmt.Errorf("unexpected configuration, pod request on smart-nic host. " +
-				"device ID must be provided")
-		}
 		// General case
 		hostIface, contIface, err = setupInterface(netns, pr.SandboxID, pr.IfName, ifInfo)
 	}
 	if err != nil {
 		return nil, err
 	}
 
-	if !ifInfo.IsSmartNic {
-		err = ConfigureOVS(pr.ctx, pr.PodNamespace, pr.PodName, hostIface.Name, ifInfo, pr.SandboxID,
-			podLister, kclient, pr.PodUID)
-		if err != nil {
-			return nil, err
-		}
+	err = ConfigureOVS(pr.ctx, pr.PodNamespace, pr.PodName, hostIface.Name, ifInfo, pr.SandboxID,
+		podLister, kclient, pr.PodUID)
+	if err != nil {
+		pr.deletePorts()
+		return nil, err
 	}
 
 	// OCP HACK: block access to MCS/metadata; https://github.com/openshift/ovn-kubernetes/pull/19
@@ -414,15 +409,6 @@ func (pr *PodRequest) ConfigureInterface(podLister corev1listers.PodLister, kcli
 		klog.Warningf("Failed to settle addresses: %q", err)
 	}
 
-	ofPort, err := getIfaceOFPort(hostIface.Name)
-	if err != nil {
-		return nil, err
-	}
-
-	if err = waitForPodFlows(pr.ctx, ifInfo.MAC.String(), ifInfo.IPs, hostIface.Name, ifaceID, ofPort); err != nil {
-		return nil, fmt.Errorf("error while waiting on flows for pod: %v", err)
-	}
-
 	return []*current.Interface{hostIface, contIface}, nil
 }
 
@@ -453,6 +439,16 @@ func (pr *PodRequest) deletePodConntrack() {
 	}
 }
 
+func (pr *PodRequest) deletePorts() {
+	ifaceName := pr.SandboxID[:15]
+	out, err := ovsExec("del-port", "br-int", ifaceName)
+	_ = util.LinkDelete(ifaceName)
+	if err != nil && !strings.Contains(out, "no port named") {
+		// DEL should be idempotent; don't return an error just log it
+		klog.Warningf("Failed to delete OVS port %s: %v\n  %q", ifaceName, err, string(out))
+	}
+}
+
 // PlatformSpecificCleanup deletes the OVS port
 func (pr *PodRequest) PlatformSpecificCleanup() error {
 	ifaceName := pr.SandboxID[:15]

go-controller/pkg/cni/ovs.go

@@ -132,6 +132,17 @@ func isIfaceIDSet(ifaceName, ifaceID string) error {
 	return nil
 }
 
+func isIfaceOvnInstalledSet(ifaceName string) bool {
+	out, err := ovsGet("Interface", ifaceName, "external-ids", "ovn-installed")
+	if err == nil && out == "true" {
+		klog.V(5).Infof("Interface %s has ovn-installed=true", ifaceName)
+		return true
+	}
+
+	klog.V(5).Info("Still waiting for OVS port %s to have ovn-installed=true", ifaceName)
+	return false
+}
+
 // getIfaceOFPort returns the of port number for an interface
 func getIfaceOFPort(ifaceName string) (int, error) {
 	port, err := ovsGet("Interface", ifaceName, "ofport", "")
@@ -146,28 +157,14 @@ func getIfaceOFPort(ifaceName string) (int, error) {
 	return iPort, nil
 }
 
-func doPodFlowsExist(mac string, ifAddrs []*net.IPNet, ofPort int) bool {
-	// Function checks for OpenFlow flows to know the pod is ready
-	// TODO(trozet): in the future use a more stable mechanism provided by OVN:
-	// https://bugzilla.redhat.com/show_bug.cgi?id=1839102
-
-	// query represents the match criteria, and different OF tables that this query may match on
-	type query struct {
-		match  string
-		tables []int
-	}
+type openflowQuery struct {
+	match  string
+	tables []int
+}
 
+func getLegacyFlowQueries(mac string, ifAddrs []*net.IPNet, ofPort int) []openflowQuery {
 	// Query the flows by mac address for in_port_security and OF port
-	queries := []query{
-		{
-			match:  "dl_src=" + mac,
-			tables: []int{9},
-		},
-		{
-			match:  fmt.Sprintf("in_port=%d", ofPort),
-			tables: []int{0},
-		},
-	}
+	queries := getMinimalFlowQueries(mac, ofPort)
 	for _, ifAddr := range ifAddrs {
 		var ipMatch string
 		if !utilnet.IsIPv6(ifAddr.IP) {
@@ -179,9 +176,31 @@ func doPodFlowsExist(mac string, ifAddrs []*net.IPNet, ofPort int) bool {
 		// note we need to support table 48 for 20.06 OVN backwards compatibility. Table 49 is now
 		// where out_port_security lives
 		queries = append(queries,
-			query{fmt.Sprintf("%s=%s", ipMatch, ifAddr.IP), []int{48, 49}},
+			openflowQuery{fmt.Sprintf("%s=%s", ipMatch, ifAddr.IP), []int{48, 49}},
 		)
 	}
+	return queries
+}
+
+func getMinimalFlowQueries(mac string, ofPort int) []openflowQuery {
+	// Query the flows by mac address for in_port_security and OF port
+	queries := []openflowQuery{
+		{
+			match:  "dl_src=" + mac,
+			tables: []int{9},
+		},
+		{
+			match:  fmt.Sprintf("in_port=%d", ofPort),
+			tables: []int{0},
+		},
+	}
+	return queries
+}
+
+func doPodFlowsExist(queries []openflowQuery) bool {
+	// Function checks for OpenFlow flows to know the pod is ready
+	// TODO(trozet): in the future use a more stable mechanism provided by OVN:
+	// https://bugzilla.redhat.com/show_bug.cgi?id=1839102
 
 	// Must find the right flows in all queries to succeed
 	for _, query := range queries {
@@ -271,9 +290,14 @@ func waitForPodInterface(ctx context.Context, mac string, ifAddrs []*net.IPNet,
 			if err := isIfaceIDSet(ifaceName, ifaceID); err != nil {
 				return err
 			}
-			if doPodFlowsExist(mac, ifAddrs, ofPort) {
-				// success
-				return nil
+			if doPodFlowsExist(queries) {
+				if checkExternalIDs {
+					if isIfaceOvnInstalledSet(ifaceName) {
+						return nil
+					}
+				} else {
+					return nil
+				}
 			}
 
 			if err := checkCancelSandbox(mac, podLister, kclient, namespace, name, initialPodUID); err != nil {

go-controller/pkg/cni/types.go

@@ -34,9 +34,10 @@ type KubeAPIAuth struct {
 type PodInterfaceInfo struct {
 	util.PodAnnotation
 
-	MTU     int   `json:"mtu"`
-	Ingress int64 `json:"ingress"`
-	Egress  int64 `json:"egress"`
+	MTU         int   `json:"mtu"`
+	Ingress     int64 `json:"ingress"`
+	Egress      int64 `json:"egress"`
+	CheckExtIDs bool  `json:"check-external-ids"`
 }
 
 // Explicit type for CNI commands the server handles
@@ -101,8 +102,6 @@ type PodRequest struct {
 	ctx context.Context
 	// cancel should be called to cancel this request
 	cancel context.CancelFunc
-	// Interface to pod is a Smart-NIC interface
-	IsSmartNIC bool
 }
 
 type cniRequestFunc func(request *PodRequest, podLister corev1listers.PodLister, useOVSExternalIDs bool, kclient kubernetes.Interface, kubeAuth *KubeAPIAuth) ([]byte, error)