OCPBUGS-8464,OCPBUGS-8222,OCPBUGS-8397: [DownstreamMerge] 7 Mar 2023

contrib/kind.sh

@@ -149,6 +149,7 @@ usage() {
     echo "-ric | --run-in-container           Configure the script to be run from a docker container, allowing it to still communicate with the kind controlplane" 
     echo "-ehp | --egress-ip-healthcheck-port TCP port used for gRPC session by egress IP node check. DEFAULT: 9107 (Use "0" for legacy dial to port 9)."
     echo "-is  | --ipsec                      Enable IPsec encryption (spawns ovn-ipsec pods)"
+    echo "-sm  | --scale-metrics              Enable scale metrics"
     echo "--isolated                          Deploy with an isolated environment (no default gateway)"
     echo "--delete                            Delete current cluster"
     echo "--deploy                            Deploy ovn kubernetes without restarting kind"
@@ -295,6 +296,8 @@ parse_args() {
                                                 fi
                                                 OVN_EGRESSIP_HEALTHCHECK_PORT=$1
                                                 ;;
+           -sm  | --scale-metrics )             OVN_METRICS_SCALE_ENABLE=true
+                                                ;;
             --isolated )                        OVN_ISOLATED=true
                                                 ;;
             -mne | --multi-network-enable )     shift
@@ -361,6 +364,7 @@ print_params() {
      echo "OVN_EX_GW_NETWORK_INTERFACE = $OVN_EX_GW_NETWORK_INTERFACE"
      echo "OVN_EGRESSIP_HEALTHCHECK_PORT = $OVN_EGRESSIP_HEALTHCHECK_PORT"
      echo "OVN_DEPLOY_PODS = $OVN_DEPLOY_PODS"
+     echo "OVN_METRICS_SCALE_ENABLE = $OVN_METRICS_SCALE_ENABLE"
      echo "OVN_ISOLATED = $OVN_ISOLATED"
      echo "ENABLE_MULTI_NET = $ENABLE_MULTI_NET"
      echo "OVN_SEPARATE_CLUSTER_MANAGER = $OVN_SEPARATE_CLUSTER_MANAGER"
@@ -510,6 +514,7 @@ set_default_params() {
   OVN_EGRESSIP_HEALTHCHECK_PORT=${OVN_EGRESSIP_HEALTHCHECK_PORT:-9107}
   OCI_BIN=${KIND_EXPERIMENTAL_PROVIDER:-docker}
   OVN_DEPLOY_PODS=${OVN_DEPLOY_PODS:-"ovnkube-master ovnkube-node"}
+  OVN_METRICS_SCALE_ENABLE=${OVN_METRICS_SCALE_ENABLE:-false}
   OVN_ISOLATED=${OVN_ISOLATED:-false}
   OVN_GATEWAY_OPTS=""
   if [ "$OVN_ISOLATED" == true ]; then
@@ -721,7 +726,8 @@ create_ovn_kube_manifests() {
     --v4-join-subnet="${JOIN_SUBNET_IPV4}" \
     --v6-join-subnet="${JOIN_SUBNET_IPV6}" \
     --ex-gw-network-interface="${OVN_EX_GW_NETWORK_INTERFACE}" \
-    --multi-network-enable=${ENABLE_MULTI_NET}
+    --multi-network-enable="${ENABLE_MULTI_NET}" \
+    --ovnkube-metrics-scale-enable="${OVN_METRICS_SCALE_ENABLE}"
   popd
 }
 

dist/images/daemonset.sh

@@ -77,6 +77,7 @@ OVN_HOST_NETWORK_NAMESPACE=""
 OVN_EX_GW_NETWORK_INTERFACE=""
 OVNKUBE_NODE_MGMT_PORT_NETDEV=""
 OVNKUBE_CONFIG_DURATION_ENABLE=
+OVNKUBE_METRICS_SCALE_ENABLE=
 # IN_UPGRADE is true only if called by upgrade-ovn.sh during the upgrade test,
 # it will render only the parts in ovn-setup.yaml related to RBAC permissions.
 IN_UPGRADE=
@@ -263,6 +264,9 @@ while [ "$1" != "" ]; do
   --ovnkube-config-duration-enable)
     OVNKUBE_CONFIG_DURATION_ENABLE=$VALUE
     ;;
+  --ovnkube-metrics-scale-enable)
+    OVNKUBE_METRICS_SCALE_ENABLE=$VALUE
+    ;;
   --in-upgrade)
     IN_UPGRADE=true
     ;;
@@ -405,6 +409,8 @@ ovnkube_node_mgmt_port_netdev=${OVNKUBE_NODE_MGMT_PORT_NETDEV}
 echo "ovnkube_node_mgmt_port_netdev: ${ovnkube_node_mgmt_port_netdev}"
 ovnkube_config_duration_enable=${OVNKUBE_CONFIG_DURATION_ENABLE}
 echo "ovnkube_config_duration_enable: ${ovnkube_config_duration_enable}"
+ovnkube_metrics_scale_enable=${OVNKUBE_METRICS_SCALE_ENABLE}
+echo "ovnkube_metrics_scale_enable: ${ovnkube_metrics_scale_enable}"
 
 ovn_image=${image} \
   ovn_image_pull_policy=${image_pull_policy} \
@@ -486,6 +492,7 @@ ovn_image=${image} \
   ovnkube_logfile_maxbackups=${ovnkube_logfile_maxbackups} \
   ovnkube_logfile_maxage=${ovnkube_logfile_maxage} \
   ovnkube_config_duration_enable=${ovnkube_config_duration_enable} \
+  ovnkube_metrics_scale_enable=${ovnkube_metrics_scale_enable} \
   ovn_acl_logging_rate_limit=${ovn_acl_logging_rate_limit} \
   ovn_hybrid_overlay_net_cidr=${ovn_hybrid_overlay_net_cidr} \
   ovn_hybrid_overlay_enable=${ovn_hybrid_overlay_enable} \

dist/images/ovnkube.sh

@@ -235,6 +235,7 @@ ovnkube_node_mode=${OVNKUBE_NODE_MODE:-"full"}
 # OVNKUBE_NODE_MGMT_PORT_NETDEV - is the net device to be used for management port
 ovnkube_node_mgmt_port_netdev=${OVNKUBE_NODE_MGMT_PORT_NETDEV:-}
 ovnkube_config_duration_enable=${OVNKUBE_CONFIG_DURATION_ENABLE:-false}
+ovnkube_metrics_scale_enable=${OVNKUBE_METRICS_SCALE_ENABLE:-false}
 # OVN_ENCAP_IP - encap IP to be used for OVN traffic on the node
 ovn_encap_ip=${OVN_ENCAP_IP:-}
 
@@ -993,6 +994,12 @@ ovn-master() {
   fi
   echo "ovnkube_config_duration_enable_flag: ${ovnkube_config_duration_enable_flag}"
 
+  ovnkube_metrics_scale_enable_flag=
+  if [[ ${ovnkube_metrics_scale_enable} == "true" ]]; then
+    ovnkube_metrics_scale_enable_flag="--metrics-enable-scale"
+  fi
+  echo "ovnkube_metrics_scale_enable_flag: ${ovnkube_metrics_scale_enable_flag}"
+
   echo "=============== ovn-master ========== MASTER ONLY"
   /usr/bin/ovnkube \
     --init-master ${K8S_NODE} \
@@ -1019,6 +1026,7 @@ ovn-master() {
     ${egressfirewall_enabled_flag} \
     ${egressqos_enabled_flag} \
     ${ovnkube_config_duration_enable_flag} \
+    ${ovnkube_metrics_scale_enable_flag} \
     ${multi_network_enabled_flag} \
     --metrics-bind-address ${ovnkube_master_metrics_bind_address} \
     --host-network-namespace ${ovn_host_network_namespace} &

dist/templates/ovnkube-master.yaml.j2

@@ -164,6 +164,8 @@ spec:
           value: "{{ ovnkube_logfile_maxage }}"
         - name: OVNKUBE_CONFIG_DURATION_ENABLE
           value: "{{ ovnkube_config_duration_enable }}"
+        - name: OVNKUBE_METRICS_SCALE_ENABLE
+          value: "{{ ovnkube_metrics_scale_enable }}"
         - name: OVN_NET_CIDR
           valueFrom:
             configMapKeyRef:

go-controller/pkg/cni/cni.go

@@ -2,10 +2,12 @@ package cni
 
 import (
 	"fmt"
+	"net"
+
+	kapi "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/klog/v2"
 	utilnet "k8s.io/utils/net"
-	"net"
 
 	current "github.com/containernetworking/cni/pkg/types/100"
 	"github.com/k8snetworkplumbingwg/govdpa/pkg/kvdpa"
@@ -80,11 +82,17 @@ func (pr *PodRequest) String() string {
 // and the best we can do is use the given UID for the duration of the request.
 // But if the existing UID is valid and does not match the given UID then the
 // sandbox request is for a different pod instance and should be terminated.
-func (pr *PodRequest) checkOrUpdatePodUID(podUID string) error {
-	if pr.PodUID == "" {
-		// Runtime didn't pass UID, use the one we got from the pod object
-		pr.PodUID = podUID
-	} else if podUID != pr.PodUID {
+// Static pod UID is a hash of the pod itself that does not match
+// the UID of the mirror kubelet creates on the api /server.
+// We will use the UID of the mirror.
+// The hash is annotated in the mirror pod (kubernetes.io/config.hash)
+// and we could match against it, but let's avoid that for now as it is not
+// a published standard.
+func (pr *PodRequest) checkOrUpdatePodUID(pod *kapi.Pod) error {
+	if pr.PodUID == "" || IsStaticPod(pod) {
+		// Runtime didn't pass UID, or the pod is a static pod, use the one we got from the pod object
+		pr.PodUID = string(pod.UID)
+	} else if string(pod.UID) != pr.PodUID {
 		// Exit early if the pod was deleted and recreated already
 		return fmt.Errorf("pod deleted before sandbox %v operation began", pr.Command)
 	}
@@ -142,12 +150,12 @@ func (pr *PodRequest) cmdAdd(kubeAuth *KubeAPIAuth, clientset *ClientSet, useOVS
 	}
 	// Get the IP address and MAC address of the pod
 	// for DPU, ensure connection-details is present
-	podUID, annotations, podNADAnnotation, err := GetPodAnnotations(pr.ctx, clientset, namespace, podName,
+	pod, annotations, podNADAnnotation, err := GetPodWithAnnotations(pr.ctx, clientset, namespace, podName,
 		pr.nadName, annotCondFn)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get pod annotation: %v", err)
 	}
-	if err := pr.checkOrUpdatePodUID(podUID); err != nil {
+	if err = pr.checkOrUpdatePodUID(pod); err != nil {
 		return nil, err
 	}
 	podInterfaceInfo, err := PodAnnotation2PodInfo(annotations, podNADAnnotation, useOVSExternalIDs, pr.PodUID, vfNetdevName,

go-controller/pkg/cni/helper_linux.go

@@ -355,7 +355,6 @@ func ConfigureOVS(ctx context.Context, namespace, podName, hostIfaceName string,
 		ifaceID = util.GetSecondaryNetworkIfaceId(namespace, podName, ifInfo.NADName)
 	}
 	initialPodUID := ifInfo.PodUID
-
 	ipStrs := make([]string, len(ifInfo.IPs))
 	for i, ip := range ifInfo.IPs {
 		ipStrs[i] = ip.String()

go-controller/pkg/cni/utils.go

@@ -60,8 +60,8 @@ func (c *ClientSet) getPod(namespace, name string) (*kapi.Pod, error) {
 }
 
 // GetPodAnnotations obtains the pod UID and annotation from the cache or apiserver
-func GetPodAnnotations(ctx context.Context, getter PodInfoGetter,
-	namespace, name, nadName string, annotCond podAnnotWaitCond) (string, map[string]string, *util.PodAnnotation, error) {
+func GetPodWithAnnotations(ctx context.Context, getter PodInfoGetter,
+	namespace, name, nadName string, annotCond podAnnotWaitCond) (*kapi.Pod, map[string]string, *util.PodAnnotation, error) {
 	var notFoundCount uint
 
 	for {
@@ -71,23 +71,23 @@ func GetPodAnnotations(ctx context.Context, getter PodInfoGetter,
 			if ctx.Err() == context.Canceled {
 				detail = "canceled while"
 			}
-			return "", nil, nil, fmt.Errorf("%s waiting for annotations: %w", detail, ctx.Err())
+			return nil, nil, nil, fmt.Errorf("%s waiting for annotations: %w", detail, ctx.Err())
 		default:
 			pod, err := getter.getPod(namespace, name)
 			if err != nil {
 				if !apierrors.IsNotFound(err) {
-					return "", nil, nil, fmt.Errorf("failed to get pod for annotations: %v", err)
+					return nil, nil, nil, fmt.Errorf("failed to get pod for annotations: %v", err)
 				}
 				// Allow up to 1 second for pod to be found
 				notFoundCount++
 				if notFoundCount >= 5 {
-					return "", nil, nil, fmt.Errorf("timed out waiting for pod after 1s: %v", err)
+					return nil, nil, nil, fmt.Errorf("timed out waiting for pod after 1s: %v", err)
 				}
 				// drop through to try again
 			} else if pod != nil {
 				podNADAnnotation, ready := annotCond(pod.Annotations, nadName)
 				if ready {
-					return string(pod.UID), pod.Annotations, podNADAnnotation, nil
+					return pod, pod.Annotations, podNADAnnotation, nil
 				}
 			}
 
@@ -133,3 +133,28 @@ func PodAnnotation2PodInfo(podAnnotation map[string]string, podNADAnnotation *ut
 	}
 	return podInterfaceInfo, nil
 }
+
+//START taken from https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/types/pod_update.go
+const (
+	ConfigSourceAnnotationKey = "kubernetes.io/config.source"
+	// ApiserverSource identifies updates from Kubernetes API Server.
+	ApiserverSource = "api"
+)
+
+// GetPodSource returns the source of the pod based on the annotation.
+func GetPodSource(pod *kapi.Pod) (string, error) {
+	if pod.Annotations != nil {
+		if source, ok := pod.Annotations[ConfigSourceAnnotationKey]; ok {
+			return source, nil
+		}
+	}
+	return "", fmt.Errorf("cannot get source of pod %q", pod.UID)
+}
+
+// IsStaticPod returns true if the pod is a static pod.
+func IsStaticPod(pod *kapi.Pod) bool {
+	source, err := GetPodSource(pod)
+	return err == nil && source != ApiserverSource
+}
+
+//END taken from https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/types/pod_update.go

go-controller/pkg/cni/utils_test.go

@@ -3,10 +3,11 @@ package cni
 import (
 	"context"
 	"fmt"
+	"time"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/mock"
-	"time"
 
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
 	ovntypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
@@ -113,7 +114,7 @@ var _ = Describe("CNI Utils tests", func() {
 		})
 	})
 
-	Context("GetPodAnnotations", func() {
+	Context("GetPodWithAnnotations", func() {
 		var podNamespaceLister mocks.PodNamespaceLister
 		var pod *v1.Pod
 
@@ -143,10 +144,10 @@ var _ = Describe("CNI Utils tests", func() {
 			clientset := newFakeClientSet(pod, &podNamespaceLister)
 
 			podNamespaceLister.On("Get", mock.AnythingOfType("string")).Return(pod, nil)
-			uid, annot, _, err := GetPodAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
+			returnedPod, annot, _, err := GetPodWithAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(annot).To(Equal(podAnnot))
-			Expect(uid).To(Equal(string(pod.UID)))
+			Expect(string(returnedPod.UID)).To(Equal(string(pod.UID)))
 		})
 
 		It("Returns with Error if context is canceled", func() {
@@ -164,7 +165,7 @@ var _ = Describe("CNI Utils tests", func() {
 			clientset := newFakeClientSet(pod, &podNamespaceLister)
 
 			podNamespaceLister.On("Get", mock.AnythingOfType("string")).Return(pod, nil)
-			_, _, _, err := GetPodAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
+			_, _, _, err := GetPodWithAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
 			Expect(err).To(HaveOccurred())
 		})
 
@@ -184,7 +185,7 @@ var _ = Describe("CNI Utils tests", func() {
 			clientset := newFakeClientSet(pod, &podNamespaceLister)
 
 			podNamespaceLister.On("Get", mock.AnythingOfType("string")).Return(pod, nil)
-			_, _, _, err := GetPodAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
+			_, _, _, err := GetPodWithAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -199,7 +200,7 @@ var _ = Describe("CNI Utils tests", func() {
 			clientset := newFakeClientSet(pod, &podNamespaceLister)
 
 			podNamespaceLister.On("Get", mock.AnythingOfType("string")).Return(nil, fmt.Errorf("failed to list pods"))
-			_, _, _, err := GetPodAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
+			_, _, _, err := GetPodWithAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("failed to list pods"))
 		})
@@ -220,7 +221,7 @@ var _ = Describe("CNI Utils tests", func() {
 			clientset := newFakeClientSet(pod, &podNamespaceLister)
 
 			podNamespaceLister.On("Get", mock.AnythingOfType("string")).Return(nil, errors.NewNotFound(v1.Resource("pod"), name))
-			_, _, _, err := GetPodAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
+			_, _, _, err := GetPodWithAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -235,7 +236,7 @@ var _ = Describe("CNI Utils tests", func() {
 			clientset := newFakeClientSet(nil, &podNamespaceLister)
 
 			podNamespaceLister.On("Get", mock.AnythingOfType("string")).Return(nil, errors.NewNotFound(v1.Resource("pod"), name))
-			_, _, _, err := GetPodAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
+			_, _, _, err := GetPodWithAnnotations(ctx, clientset, namespace, podName, ovntypes.DefaultNetworkName, cond)
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("timed out waiting for pod after 1s"))
 		})

go-controller/pkg/config/config.go

@@ -333,8 +333,8 @@ type MetricsConfig struct {
 	NodeServerCert        string `gcfg:"node-server-cert"`
 	// EnableConfigDuration holds the boolean flag to enable OVN-Kubernetes master to monitor OVN-Kubernetes master
 	// configuration duration and optionally, its application to all nodes
-	EnableConfigDuration  bool `gcfg:"enable-config-duration"`
-	EnableEIPScaleMetrics bool `gcfg:"enable-eip-scale-metrics"`
+	EnableConfigDuration bool `gcfg:"enable-config-duration"`
+	EnableScaleMetrics   bool `gcfg:"enable-scale-metrics"`
 }
 
 // OVNKubernetesFeatureConfig holds OVN-Kubernetes feature enhancement config file parameters and command-line overrides
@@ -1041,9 +1041,9 @@ var MetricsFlags = []cli.Flag{
 		Destination: &cliConfig.Metrics.EnableConfigDuration,
 	},
 	&cli.BoolFlag{
-		Name:        "metrics-enable-eip-scale",
-		Usage:       "Enables metrics related to Egress IP scaling",
-		Destination: &cliConfig.Metrics.EnableEIPScaleMetrics,
+		Name:        "metrics-enable-scale",
+		Usage:       "Enables metrics related to scaling",
+		Destination: &cliConfig.Metrics.EnableScaleMetrics,
 	},
 }
 

go-controller/pkg/config/config_test.go

@@ -158,7 +158,7 @@ enable-pprof=true
 node-server-privkey=/path/to/node-metrics-private.key
 node-server-cert=/path/to/node-metrics.crt
 enable-config-duration=true
-enable-eip-scale-metrics=true
+enable-scale-metrics=true
 
 [logging]
 loglevel=5
@@ -585,7 +585,7 @@ var _ = Describe("Config Operations", func() {
 			gomega.Expect(Metrics.NodeServerPrivKey).To(gomega.Equal("/path/to/node-metrics-private.key"))
 			gomega.Expect(Metrics.NodeServerCert).To(gomega.Equal("/path/to/node-metrics.crt"))
 			gomega.Expect(Metrics.EnableConfigDuration).To(gomega.Equal(true))
-			gomega.Expect(Metrics.EnableEIPScaleMetrics).To(gomega.Equal(true))
+			gomega.Expect(Metrics.EnableScaleMetrics).To(gomega.Equal(true))
 
 			gomega.Expect(OvnNorth.Scheme).To(gomega.Equal(OvnDBSchemeSSL))
 			gomega.Expect(OvnNorth.PrivKey).To(gomega.Equal("/path/to/nb-client-private.key"))
@@ -673,7 +673,7 @@ var _ = Describe("Config Operations", func() {
 			gomega.Expect(Metrics.NodeServerPrivKey).To(gomega.Equal("/tls/nodeprivkey"))
 			gomega.Expect(Metrics.NodeServerCert).To(gomega.Equal("/tls/nodecert"))
 			gomega.Expect(Metrics.EnableConfigDuration).To(gomega.Equal(true))
-			gomega.Expect(Metrics.EnableEIPScaleMetrics).To(gomega.Equal(true))
+			gomega.Expect(Metrics.EnableScaleMetrics).To(gomega.Equal(true))
 
 			gomega.Expect(OvnNorth.Scheme).To(gomega.Equal(OvnDBSchemeSSL))
 			gomega.Expect(OvnNorth.PrivKey).To(gomega.Equal("/client/privkey"))

go-controller/pkg/libovsdbops/acl.go

@@ -102,6 +102,11 @@ func CreateOrUpdateACLsOps(nbClient libovsdbclient.Client, ops []libovsdb.Operat
 	for i := range acls {
 		// can't use i in the predicate, for loop replaces it in-memory
 		acl := acls[i]
+		// ensure names are truncated (let's cover our bases from snippets that don't call BuildACL and call this directly)
+		if acl.Name != nil {
+			// node ACLs won't have names set
+			*acl.Name = fmt.Sprintf("%.63s", *acl.Name)
+		}
 		opModel := operationModel{
 			Model:          acl,
 			ModelPredicate: func(item *nbdb.ACL) bool { return isEquivalentACL(item, acl) },