diff --git a/npm/namespace.go b/npm/namespace.go index 75629ff6d8..de1a4c58a5 100644 --- a/npm/namespace.go +++ b/npm/namespace.go @@ -9,21 +9,21 @@ import ( "github.com/Azure/azure-container-networking/npm/ipsm" "github.com/Azure/azure-container-networking/npm/iptm" "github.com/Azure/azure-container-networking/npm/util" + "k8s.io/apimachinery/pkg/types" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" ) type namespace struct { - name string - labelsMap map[string]string // NameSpace labels - setMap map[string]string - podMap map[string]*npmPod // Key is PodUID - rawNpMap map[string]*networkingv1.NetworkPolicy - processedNpMap map[string]*networkingv1.NetworkPolicy - ipsMgr *ipsm.IpsetManager - iptMgr *iptm.IptablesManager - resourceVersion uint64 // NameSpace ResourceVersion + name string + labelsMap map[string]string + setMap map[string]string + podMap map[types.UID]*corev1.Pod + rawNpMap map[string]*networkingv1.NetworkPolicy + processedNpMap map[string]*networkingv1.NetworkPolicy + ipsMgr *ipsm.IpsetManager + iptMgr *iptm.IptablesManager } // newNS constructs a new namespace object. @@ -32,24 +32,16 @@ func newNs(name string) (*namespace, error) { name: name, labelsMap: make(map[string]string), setMap: make(map[string]string), - podMap: make(map[string]*npmPod), + podMap: make(map[types.UID]*corev1.Pod), rawNpMap: make(map[string]*networkingv1.NetworkPolicy), processedNpMap: make(map[string]*networkingv1.NetworkPolicy), ipsMgr: ipsm.NewIpsetManager(), iptMgr: iptm.NewIptablesManager(), - // resource version is converted to uint64 - // so make sure it is initialized to "0" - resourceVersion: 0, } return ns, nil } -// setResourceVersion setter func for RV -func setResourceVersion(nsObj *namespace, rv string) { - nsObj.resourceVersion = util.ParseResourceVersion(rv) -} - func isSystemNs(nsObj *corev1.Namespace) bool { return nsObj.ObjectMeta.Name == util.KubeSystemFlag } @@ -64,22 +56,10 @@ func isInvalidNamespaceUpdate(oldNsObj, newNsObj *corev1.Namespace) (isInvalidUp } func (ns *namespace) policyExists(npObj *networkingv1.NetworkPolicy) bool { - np, exists := ns.rawNpMap[npObj.ObjectMeta.Name] - if !exists { - return false - } - - if !util.CompareResourceVersions(np.ObjectMeta.ResourceVersion, npObj.ObjectMeta.ResourceVersion) { - log.Logf("Cached Network Policy has larger ResourceVersion number than new Obj. Name: %s Cached RV: %d New RV: %d\n", - npObj.ObjectMeta.Name, - np.ObjectMeta.ResourceVersion, - npObj.ObjectMeta.ResourceVersion, - ) - return true - } - - if isSamePolicy(np, npObj) { - return true + if np, exists := ns.rawNpMap[npObj.ObjectMeta.Name]; exists { + if isSamePolicy(np, npObj) { + return true + } } return false @@ -123,7 +103,7 @@ func (npMgr *NetworkPolicyManager) UninitAllNsList() error { func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error { var err error - nsName, nsLabel := util.GetNSNameWithPrefix(nsObj.ObjectMeta.Name), nsObj.ObjectMeta.Labels + nsName, nsLabel := "ns-"+nsObj.ObjectMeta.Name, nsObj.ObjectMeta.Labels log.Logf("NAMESPACE CREATING: [%s/%v]", nsName, nsLabel) ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr @@ -141,14 +121,14 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error { // Add the namespace to its label's ipset list. nsLabels := nsObj.ObjectMeta.Labels for nsLabelKey, nsLabelVal := range nsLabels { - labelKey := util.GetNSNameWithPrefix(nsLabelKey) + labelKey := "ns-" + nsLabelKey log.Logf("Adding namespace %s to ipset list %s", nsName, labelKey) if err = ipsMgr.AddToList(labelKey, nsName); err != nil { log.Errorf("Error: failed to add namespace %s to ipset list %s", nsName, labelKey) return err } - label := util.GetNSNameWithPrefix(nsLabelKey + ":" + nsLabelVal) + label := "ns-" + nsLabelKey + ":" + nsLabelVal log.Logf("Adding namespace %s to ipset list %s", nsName, label) if err = ipsMgr.AddToList(label, nsName); err != nil { log.Errorf("Error: failed to add namespace %s to ipset list %s", nsName, label) @@ -160,7 +140,6 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error { if err != nil { log.Errorf("Error: failed to create namespace %s", nsName) } - setResourceVersion(ns, nsObj.GetObjectMeta().GetResourceVersion()) // Append all labels to the cache NS obj ns.labelsMap = util.AppendMap(ns.labelsMap, nsLabel) @@ -176,8 +155,8 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n } var err error - oldNsNs, oldNsLabel := util.GetNSNameWithPrefix(oldNsObj.ObjectMeta.Name), oldNsObj.ObjectMeta.Labels - newNsNs, newNsLabel := util.GetNSNameWithPrefix(newNsObj.ObjectMeta.Name), newNsObj.ObjectMeta.Labels + oldNsNs, oldNsLabel := "ns-"+oldNsObj.ObjectMeta.Name, oldNsObj.ObjectMeta.Labels + newNsNs, newNsLabel := "ns-"+newNsObj.ObjectMeta.Name, newNsObj.ObjectMeta.Labels log.Logf( "NAMESPACE UPDATING:\n old namespace: [%s/%v]\n new namespace: [%s/%v]", oldNsNs, oldNsLabel, newNsNs, newNsLabel, @@ -210,16 +189,6 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n return nil } - newRv := util.ParseResourceVersion(newNsObj.ObjectMeta.ResourceVersion) - if !util.CompareUintResourceVersions(curNsObj.resourceVersion, newRv) { - log.Logf("Cached NameSpace has larger ResourceVersion number than new Obj. NameSpace: %s Cached RV: %d New RV:\n", - oldNsNs, - curNsObj.resourceVersion, - newRv, - ) - return nil - } - //if no change in labels then return if reflect.DeepEqual(curNsObj.labelsMap, newNsLabel) { log.Logf( @@ -230,32 +199,45 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n } //If the Namespace is not deleted, delete removed labels and create new labels - addToIPSets, deleteFromIPSets := util.GetIPSetListCompareLabels(curNsObj.labelsMap, newNsLabel) + toAddNsLabels, toDeleteNsLabels := util.CompareMapDiff(curNsObj.labelsMap, newNsLabel) // Delete the namespace from its label's ipset list. ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr - for _, nsLabelVal := range deleteFromIPSets { - labelKey := util.GetNSNameWithPrefix(nsLabelVal) + for nsLabelKey, nsLabelVal := range toDeleteNsLabels { + labelKey := "ns-" + nsLabelKey log.Logf("Deleting namespace %s from ipset list %s", oldNsNs, labelKey) if err = ipsMgr.DeleteFromList(labelKey, oldNsNs); err != nil { log.Errorf("Error: failed to delete namespace %s from ipset list %s", oldNsNs, labelKey) return err } + + label := "ns-" + nsLabelKey + ":" + nsLabelVal + log.Logf("Deleting namespace %s from ipset list %s", oldNsNs, label) + if err = ipsMgr.DeleteFromList(label, oldNsNs); err != nil { + log.Errorf("Error: failed to delete namespace %s from ipset list %s", oldNsNs, label) + return err + } } // Add the namespace to its label's ipset list. - for _, nsLabelVal := range addToIPSets { - labelKey := util.GetNSNameWithPrefix(nsLabelVal) + for nsLabelKey, nsLabelVal := range toAddNsLabels { + labelKey := "ns-" + nsLabelKey log.Logf("Adding namespace %s to ipset list %s", oldNsNs, labelKey) if err = ipsMgr.AddToList(labelKey, oldNsNs); err != nil { log.Errorf("Error: failed to add namespace %s to ipset list %s", oldNsNs, labelKey) return err } + + label := "ns-" + nsLabelKey + ":" + nsLabelVal + log.Logf("Adding namespace %s to ipset list %s", oldNsNs, label) + if err = ipsMgr.AddToList(label, oldNsNs); err != nil { + log.Errorf("Error: failed to add namespace %s to ipset list %s", oldNsNs, label) + return err + } } // Append all labels to the cache NS obj curNsObj.labelsMap = util.ClearAndAppendMap(curNsObj.labelsMap, newNsLabel) - setResourceVersion(curNsObj, newNsObj.GetObjectMeta().GetResourceVersion()) npMgr.nsMap[newNsNs] = curNsObj return nil @@ -265,27 +247,26 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n func (npMgr *NetworkPolicyManager) DeleteNamespace(nsObj *corev1.Namespace) error { var err error - nsName, nsLabel := util.GetNSNameWithPrefix(nsObj.ObjectMeta.Name), nsObj.ObjectMeta.Labels + nsName, nsLabel := "ns-"+nsObj.ObjectMeta.Name, nsObj.ObjectMeta.Labels log.Logf("NAMESPACE DELETING: [%s/%v]", nsName, nsLabel) - cachedNsObj, exists := npMgr.nsMap[nsName] + _, exists := npMgr.nsMap[nsName] if !exists { return nil } - log.Logf("NAMESPACE DELETING cached labels: [%s/%v]", nsName, cachedNsObj.labelsMap) // Delete the namespace from its label's ipset list. ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr - nsLabels := cachedNsObj.labelsMap + nsLabels := nsObj.ObjectMeta.Labels for nsLabelKey, nsLabelVal := range nsLabels { - labelKey := util.GetNSNameWithPrefix(nsLabelKey) + labelKey := "ns-" + nsLabelKey log.Logf("Deleting namespace %s from ipset list %s", nsName, labelKey) if err = ipsMgr.DeleteFromList(labelKey, nsName); err != nil { log.Errorf("Error: failed to delete namespace %s from ipset list %s", nsName, labelKey) return err } - label := util.GetNSNameWithPrefix(nsLabelKey + ":" + nsLabelVal) + label := "ns-" + nsLabelKey + ":" + nsLabelVal log.Logf("Deleting namespace %s from ipset list %s", nsName, label) if err = ipsMgr.DeleteFromList(label, nsName); err != nil { log.Errorf("Error: failed to delete namespace %s from ipset list %s", nsName, label) diff --git a/npm/namespace_test.go b/npm/namespace_test.go index 8cfeff4f3b..7f1719369a 100644 --- a/npm/namespace_test.go +++ b/npm/namespace_test.go @@ -16,7 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestNewNs(t *testing.T) { +func TestnewNs(t *testing.T) { if _, err := newNs("test"); err != nil { t.Errorf("TestnewNs failed @ newNs") } @@ -165,7 +165,6 @@ func TestAddNamespaceLabel(t *testing.T) { Labels: map[string]string{ "app": "old-test-namespace", }, - ResourceVersion: "0", }, } @@ -176,8 +175,6 @@ func TestAddNamespaceLabel(t *testing.T) { "app": "old-test-namespace", "update": "true", }, - - ResourceVersion: "1", }, } @@ -228,7 +225,6 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) { "update": "true", "group": "test", }, - ResourceVersion: "0", }, } @@ -239,7 +235,6 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) { "app": "old-test-namespace", "update": "false", }, - ResourceVersion: "1", }, } diff --git a/npm/npm.go b/npm/npm.go index d48523e8f5..15167f986d 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -50,6 +50,7 @@ type NetworkPolicyManager struct { nodeName string nsMap map[string]*namespace + podMap map[string]string // Key: Pod uuid, Value: PodIp isAzureNpmChainCreated bool isSafeToCleanUpAzureNpmChain bool @@ -117,13 +118,14 @@ func (npMgr *NetworkPolicyManager) SendClusterMetrics() { for { <-heartbeat npMgr.Lock() - podCount.Value = 0 + podCount.Value = float64(len(npMgr.podMap)) //Reducing one to remove all-namespaces ns obj nsCount.Value = float64(len(npMgr.nsMap) - 1) + nwPolCount := 0 for _, ns := range npMgr.nsMap { - nwPolicyCount.Value += float64(len(ns.rawNpMap)) - podCount.Value += float64(len(ns.podMap)) + nwPolCount = nwPolCount + len(ns.rawNpMap) } + nwPolicyCount.Value = float64(nwPolCount) npMgr.Unlock() metrics.SendMetric(podCount) @@ -232,6 +234,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in npInformer: npInformer, nodeName: os.Getenv("HOSTNAME"), nsMap: make(map[string]*namespace), + podMap: make(map[string]string), isAzureNpmChainCreated: false, isSafeToCleanUpAzureNpmChain: false, clusterState: telemetry.ClusterState{ @@ -248,7 +251,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs // Create ipset for the namespace. - kubeSystemNs := util.GetNSNameWithPrefix(util.KubeSystemFlag) + kubeSystemNs := "ns-" + util.KubeSystemFlag if err := allNs.ipsMgr.CreateSet(kubeSystemNs, append([]string{util.IpsetNetHashFlag})); err != nil { metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to create ipset for namespace %s.", kubeSystemNs) } @@ -266,14 +269,19 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in npMgr.AddPod(podObj) npMgr.Unlock() }, - UpdateFunc: func(_, new interface{}) { + UpdateFunc: func(old, new interface{}) { + oldPodObj, ok := old.(*corev1.Pod) + if !ok { + metrics.SendErrorLogAndMetric(util.NpmID, "UPDATE Pod: Received unexpected old object type: %v", oldPodObj) + return + } newPodObj, ok := new.(*corev1.Pod) if !ok { metrics.SendErrorLogAndMetric(util.NpmID, "UPDATE Pod: Received unexpected new object type: %v", newPodObj) return } npMgr.Lock() - npMgr.UpdatePod(newPodObj) + npMgr.UpdatePod(oldPodObj, newPodObj) npMgr.Unlock() }, DeleteFunc: func(obj interface{}) { diff --git a/npm/nwpolicy.go b/npm/nwpolicy.go index 161c3a3c3e..3c9a73421c 100644 --- a/npm/nwpolicy.go +++ b/npm/nwpolicy.go @@ -33,7 +33,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP err error ns *namespace exists bool - npNs = util.GetNSNameWithPrefix(npObj.ObjectMeta.Namespace) + npNs = "ns-" + npObj.ObjectMeta.Namespace npName = npObj.ObjectMeta.Name allNs = npMgr.nsMap[util.KubeAllNamespacesFlag] timer = metrics.StartNewTimer() @@ -153,7 +153,7 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo allNs = npMgr.nsMap[util.KubeAllNamespacesFlag] ) - npNs, npName := util.GetNSNameWithPrefix(npObj.ObjectMeta.Namespace), npObj.ObjectMeta.Name + npNs, npName := "ns-"+npObj.ObjectMeta.Namespace, npObj.ObjectMeta.Name log.Logf("NETWORK POLICY DELETING: Namespace: %s, Name:%s", npNs, npName) var exists bool diff --git a/npm/pod.go b/npm/pod.go index abee2c4ca5..fc4ffb121e 100644 --- a/npm/pod.go +++ b/npm/pod.go @@ -7,74 +7,12 @@ import ( "reflect" "github.com/Azure/azure-container-networking/log" - "github.com/Azure/azure-container-networking/npm/ipsm" "github.com/Azure/azure-container-networking/npm/util" corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" ) -type npmPod struct { - name string - namespace string - nodeName string - podUID string - podIP string - isHostNetwork bool - podIPs []v1.PodIP - labels map[string]string - containerPorts []v1.ContainerPort - resourceVersion uint64 // Pod Resource Version - phase corev1.PodPhase -} - -func newNpmPod(podObj *corev1.Pod) (*npmPod, error) { - rv := util.ParseResourceVersion(podObj.GetObjectMeta().GetResourceVersion()) - pod := &npmPod{ - name: podObj.ObjectMeta.Name, - namespace: podObj.ObjectMeta.Namespace, - nodeName: podObj.Spec.NodeName, - podUID: string(podObj.ObjectMeta.UID), - podIP: podObj.Status.PodIP, - podIPs: podObj.Status.PodIPs, - isHostNetwork: podObj.Spec.HostNetwork, - labels: podObj.Labels, - containerPorts: getContainerPortList(podObj), - resourceVersion: rv, - phase: podObj.Status.Phase, - } - - return pod, nil -} - -func getPodObjFromNpmObj(npmPodObj *npmPod) *corev1.Pod { - return &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: npmPodObj.name, - Namespace: npmPodObj.namespace, - Labels: npmPodObj.labels, - UID: types.UID(npmPodObj.podUID), - }, - Status: corev1.PodStatus{ - Phase: npmPodObj.phase, - PodIP: npmPodObj.podIP, - PodIPs: npmPodObj.podIPs, - }, - Spec: corev1.PodSpec{ - HostNetwork: npmPodObj.isHostNetwork, - NodeName: npmPodObj.nodeName, - Containers: []v1.Container{ - v1.Container{ - Ports: npmPodObj.containerPorts, - }, - }, - }, - } - -} - func isValidPod(podObj *corev1.Pod) bool { return len(podObj.Status.PodIP) > 0 } @@ -94,151 +32,110 @@ func isInvalidPodUpdate(oldPodObj, newPodObj *corev1.Pod) (isInvalidUpdate bool) oldPodObj.Status.PodIP == newPodObj.Status.PodIP && newPodObj.ObjectMeta.DeletionTimestamp == nil && newPodObj.ObjectMeta.DeletionGracePeriodSeconds == nil - isInvalidUpdate = isInvalidUpdate && - reflect.DeepEqual(oldPodObj.ObjectMeta.Labels, newPodObj.ObjectMeta.Labels) && - reflect.DeepEqual(oldPodObj.Status.PodIPs, newPodObj.Status.PodIPs) && - reflect.DeepEqual(getContainerPortList(oldPodObj), getContainerPortList(newPodObj)) + isInvalidUpdate = isInvalidUpdate && reflect.DeepEqual(oldPodObj.ObjectMeta.Labels, newPodObj.ObjectMeta.Labels) return } -func getContainerPortList(podObj *corev1.Pod) []v1.ContainerPort { - portList := []v1.ContainerPort{} - for _, container := range podObj.Spec.Containers { - portList = append(portList, container.Ports...) - } - return portList -} - -// appendNamedPortIpsets helps with adding or deleting Pod namedPort IPsets -func appendNamedPortIpsets(ipsMgr *ipsm.IpsetManager, portList []v1.ContainerPort, podUID string, podIP string, delete bool) error { - - for _, port := range portList { - if port.Name == "" { - continue - } - - protocol := "" - - switch port.Protocol { - case v1.ProtocolUDP: - protocol = util.IpsetUDPFlag - case v1.ProtocolSCTP: - protocol = util.IpsetSCTPFlag - case v1.ProtocolTCP: - protocol = util.IpsetTCPFlag - } - - namedPortname := util.NamedPortIPSetPrefix + port.Name - - if delete { - // Delete the pod's named ports from its ipset. - ipsMgr.DeleteFromSet( - namedPortname, - fmt.Sprintf("%s,%s%d", podIP, protocol, port.ContainerPort), - podUID, - ) - continue - } - // Add the pod's named ports to its ipset. - ipsMgr.AddToSet( - namedPortname, - fmt.Sprintf("%s,%s%d", podIP, protocol, port.ContainerPort), - util.IpsetIPPortHashFlag, - podUID, - ) - - } - - return nil -} - // AddPod handles adding pod ip to its label's ipset. func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { if !isValidPod(podObj) { return nil } - // Ignore adding the HostNetwork pod to any ipsets. - if isHostNetworkPod(podObj) { - log.Logf("HostNetwork POD IGNORED: [%s/%s/%s/%+v%s]", podObj.GetObjectMeta().GetUID(), podObj.Namespace, podObj.Name, podObj.Labels, podObj.Status.PodIP) - return nil - } - - npmPodObj, podErr := newNpmPod(podObj) - if podErr != nil { - log.Errorf("Error: failed to create namespace %s, %+v", podObj.ObjectMeta.Name, podObj) - return podErr - } - var ( - err error - podNs = util.GetNSNameWithPrefix(npmPodObj.namespace) - podUID = npmPodObj.podUID - podName = npmPodObj.name - podNodeName = npmPodObj.nodeName - podLabels = npmPodObj.labels - podIP = npmPodObj.podIP - podContainerPorts = npmPodObj.containerPorts - ipsMgr = npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr + err error + podNs = "ns-" + podObj.ObjectMeta.Namespace + podUid = string(podObj.ObjectMeta.UID) + podName = podObj.ObjectMeta.Name + podNodeName = podObj.Spec.NodeName + podLabels = podObj.ObjectMeta.Labels + podIP = podObj.Status.PodIP + podContainers = podObj.Spec.Containers + ipsMgr = npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr ) - log.Logf("POD CREATING: [%s%s/%s/%s%+v%s]", podUID, podNs, podName, podNodeName, podLabels, podIP) + log.Logf("POD CREATING: [%s%s/%s/%s%+v%s]", podUid, podNs, podName, podNodeName, podLabels, podIP) // Add pod namespace if it doesn't exist if _, exists := npMgr.nsMap[podNs]; !exists { - npMgr.nsMap[podNs], err = newNs(podNs) - if err != nil { - log.Errorf("Error: failed to create namespace %s", podNs) - } log.Logf("Creating set: %v, hashedSet: %v", podNs, util.GetHashedName(podNs)) if err = ipsMgr.CreateSet(podNs, append([]string{util.IpsetNetHashFlag})); err != nil { log.Logf("Error creating ipset %s", podNs) } } + // Ignore adding the HostNetwork pod to any ipsets. + if isHostNetworkPod(podObj) { + log.Logf("HostNetwork POD IGNORED: [%s%s/%s/%s%+v%s]", podUid, podNs, podName, podNodeName, podLabels, podIP) + return nil + } + // Add the pod to its namespace's ipset. log.Logf("Adding pod %s to ipset %s", podIP, podNs) - if err = ipsMgr.AddToSet(podNs, podIP, util.IpsetNetHashFlag, podUID); err != nil { + if err = ipsMgr.AddToSet(podNs, podIP, util.IpsetNetHashFlag, podUid); err != nil { log.Errorf("Error: failed to add pod to namespace ipset.") } // Add the pod to its label's ipset. for podLabelKey, podLabelVal := range podLabels { log.Logf("Adding pod %s to ipset %s", podIP, podLabelKey) - if err = ipsMgr.AddToSet(podLabelKey, podIP, util.IpsetNetHashFlag, podUID); err != nil { + if err = ipsMgr.AddToSet(podLabelKey, podIP, util.IpsetNetHashFlag, podUid); err != nil { log.Errorf("Error: failed to add pod to label ipset.") } label := podLabelKey + ":" + podLabelVal log.Logf("Adding pod %s to ipset %s", podIP, label) - if err = ipsMgr.AddToSet(label, podIP, util.IpsetNetHashFlag, podUID); err != nil { + if err = ipsMgr.AddToSet(label, podIP, util.IpsetNetHashFlag, podUid); err != nil { log.Errorf("Error: failed to add pod to label ipset.") } } - // Add pod's named ports from its ipset. - if err = appendNamedPortIpsets(ipsMgr, podContainerPorts, podUID, podIP, false); err != nil { - log.Errorf("Error: failed to add pod to namespace ipset.") + // Add the pod's named ports its ipset. + for _, container := range podContainers { + for _, port := range container.Ports { + if port.Name != "" { + protocol := "" + switch port.Protocol { + case v1.ProtocolUDP: + protocol = util.IpsetUDPFlag + case v1.ProtocolSCTP: + protocol = util.IpsetSCTPFlag + } + namedPortname := util.NamedPortIPSetPrefix + port.Name + ipsMgr.AddToSet( + namedPortname, + fmt.Sprintf("%s,%s%d", podIP, protocol, port.ContainerPort), + util.IpsetIPPortHashFlag, + podUid, + ) + + } + } } // add the Pod info to the podMap - npMgr.nsMap[podNs].podMap[podUID] = npmPodObj + npMgr.podMap[podUid] = podIP return nil } // UpdatePod handles updating pod ip in its label's ipset. -func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { +func (npMgr *NetworkPolicyManager) UpdatePod(oldPodObj, newPodObj *corev1.Pod) error { if !isValidPod(newPodObj) { return nil } + if isInvalidPodUpdate(oldPodObj, newPodObj) { + return nil + } + // today K8s does not allow updating HostNetwork flag for an existing Pod. So NPM can safely // check on the oldPodObj for hostNework value - if isHostNetworkPod(newPodObj) { + if isHostNetworkPod(oldPodObj) { log.Logf( - "POD UPDATING ignored for HostNetwork Pod:\n pod: [%s/%s/%s]", + "POD UPDATING ignored for HostNetwork Pod:\n old pod: [%s/%s/%s]\n new pod: [%s/%s/%s]", + oldPodObj.ObjectMeta.Namespace, oldPodObj.ObjectMeta.Name, oldPodObj.Status.PodIP, newPodObj.ObjectMeta.Namespace, newPodObj.ObjectMeta.Name, newPodObj.Status.PodIP, ) return nil @@ -246,215 +143,111 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { var ( err error - newPodObjNs = util.GetNSNameWithPrefix(newPodObj.ObjectMeta.Namespace) + oldPodObjNs = oldPodObj.ObjectMeta.Namespace + oldPodObjName = oldPodObj.ObjectMeta.Name + oldPodObjLabel = oldPodObj.ObjectMeta.Labels + oldPodObjPhase = oldPodObj.Status.Phase + oldPodObjIP = oldPodObj.Status.PodIP + newPodObjNs = newPodObj.ObjectMeta.Namespace newPodObjName = newPodObj.ObjectMeta.Name newPodObjLabel = newPodObj.ObjectMeta.Labels newPodObjPhase = newPodObj.Status.Phase newPodObjIP = newPodObj.Status.PodIP - ipsMgr = npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr - ) - - // Add pod namespace if it doesn't exist - if _, exists := npMgr.nsMap[newPodObjNs]; !exists { - npMgr.nsMap[newPodObjNs], err = newNs(newPodObjNs) - if err != nil { - log.Errorf("Error: failed to create namespace %s", newPodObjNs) - } - log.Logf("Creating set: %v, hashedSet: %v", newPodObjNs, util.GetHashedName(newPodObjNs)) - if err = ipsMgr.CreateSet(newPodObjNs, append([]string{util.IpsetNetHashFlag})); err != nil { - log.Logf("Error creating ipset %s", newPodObjNs) - } - } - - cachedPodObj, exists := npMgr.nsMap[newPodObjNs].podMap[string(newPodObj.ObjectMeta.UID)] - if !exists { - if addErr := npMgr.AddPod(newPodObj); addErr != nil { - log.Errorf("Error: failed to add pod during update with error %+v", addErr) - } - return nil - } - - if isInvalidPodUpdate(getPodObjFromNpmObj(cachedPodObj), newPodObj) { - return nil - } - - check := util.CompareUintResourceVersions( - cachedPodObj.resourceVersion, - util.ParseResourceVersion(newPodObj.ObjectMeta.ResourceVersion), - ) - if !check { - log.Logf( - "POD UPDATING ignored as resourceVersion of cached pod is greater Pod:\n cached pod: [%s/%s/%s/%d]\n new pod: [%s/%s/%s/%s]", - cachedPodObj.namespace, cachedPodObj.name, cachedPodObj.podIP, cachedPodObj.resourceVersion, - newPodObj.ObjectMeta.Namespace, newPodObj.ObjectMeta.Name, newPodObj.Status.PodIP, newPodObj.ObjectMeta.ResourceVersion, - ) - - return nil - } - - // We are assuming that FAILED to RUNNING pod will send an update - if newPodObj.Status.Phase == v1.PodSucceeded || newPodObj.Status.Phase == v1.PodFailed { - if delErr := npMgr.DeletePod(newPodObj); delErr != nil { - log.Errorf("Error: failed to add pod during update with error %+v", delErr) - } - - return nil - } - - var ( - cachedPodIP = cachedPodObj.podIP - cachedLabels = cachedPodObj.labels ) log.Logf( - "POD UPDATING:\n new pod: [%s/%s/%+v/%s/%s]\n cached pod: [%s/%s/%+v/%s]", + "POD UPDATING:\n old pod: [%s/%s/%+v/%s/%s]\n new pod: [%s/%s/%+v/%s/%s]", + oldPodObjNs, oldPodObjName, oldPodObjLabel, oldPodObjPhase, oldPodObjIP, newPodObjNs, newPodObjName, newPodObjLabel, newPodObjPhase, newPodObjIP, - cachedPodObj.namespace, cachedPodObj.name, cachedPodObj.labels, cachedPodObj.podIP, ) - deleteFromIPSets := []string{} - addToIPSets := []string{} + // Todo: Update if cached ip and podip changed and it is not a delete event - // if the podIp exists, it must match the cachedIp - if cachedPodIP != newPodObjIP { - // TODO Add AI telemetry event - log.Errorf("Error: Unexpected state. Pod (Namespace:%s, Name:%s, uid:%s, has cachedPodIp:%s which is different from PodIp:%s", - newPodObjNs, newPodObjName, cachedPodObj.podUID, cachedPodIP, newPodObjIP) - // cached PodIP needs to be cleaned up from all the cached labels - deleteFromIPSets = util.GetIPSetListFromLabels(cachedLabels) - - // Assume that the pod IP will be released when pod moves to succeeded or failed state. - // If the pod transitions back to an active state, then add operation will re establish the updated pod info. - // new PodIP needs to be added to all newLabels - addToIPSets = util.GetIPSetListFromLabels(newPodObjLabel) - - // Delete the pod from its namespace's ipset. - log.Logf("Deleting pod %s %s from ipset %s and adding pod %s to ipset %s", - cachedPodObj.podUID, - cachedPodIP, - cachedPodObj.namespace, - newPodObjIP, - newPodObjNs, - ) - if err = ipsMgr.DeleteFromSet(cachedPodObj.namespace, cachedPodIP, cachedPodObj.podUID); err != nil { - log.Errorf("Error: failed to delete pod from namespace ipset.") - } - // Add the pod to its namespace's ipset. - if err = ipsMgr.AddToSet(newPodObjNs, newPodObjIP, util.IpsetNetHashFlag, cachedPodObj.podUID); err != nil { - log.Errorf("Error: failed to add pod to namespace ipset.") - } - } else { - //if no change in labels then return - if reflect.DeepEqual(cachedLabels, newPodObjLabel) { - log.Logf( - "POD UPDATING:\n nothing to delete or add. pod: [%s/%s]", - newPodObjNs, newPodObjName, - ) - return nil - } - // delete PodIP from removed labels and add PodIp to new labels - addToIPSets, deleteFromIPSets = util.GetIPSetListCompareLabels(cachedLabels, newPodObjLabel) - } - - // Delete the pod from its label's ipset. - for _, podIPSetName := range deleteFromIPSets { - log.Logf("Deleting pod %s from ipset %s", cachedPodIP, podIPSetName) - if err = ipsMgr.DeleteFromSet(podIPSetName, cachedPodIP, cachedPodObj.podUID); err != nil { - log.Errorf("Error: failed to delete pod from label ipset.") - } - } - - // Add the pod to its label's ipset. - for _, addIPSetName := range addToIPSets { - log.Logf("Adding pod %s to ipset %s", newPodObjIP, addIPSetName) - if err = ipsMgr.AddToSet(addIPSetName, newPodObjIP, util.IpsetNetHashFlag, cachedPodObj.podUID); err != nil { - log.Errorf("Error: failed to add pod to label ipset.") - } + if err = npMgr.DeletePod(oldPodObj); err != nil { + log.Errorf("Error: failed to delete pod during update with error %+v", err) + return err } - // TODO optimize named port addition and deletions. - // named ports are mostly static once configured in todays usage pattern - // so keeping this simple by deleting all and re-adding - newPodPorts := getContainerPortList(newPodObj) - if !reflect.DeepEqual(cachedPodObj.containerPorts, newPodPorts) { - // Delete cached pod's named ports from its ipset. - if err = appendNamedPortIpsets(ipsMgr, cachedPodObj.containerPorts, cachedPodObj.podUID, cachedPodIP, true); err != nil { - log.Errorf("Error: failed to delete pod from namespace ipset.") - } - // Add new pod's named ports from its ipset. - if err = appendNamedPortIpsets(ipsMgr, newPodPorts, cachedPodObj.podUID, newPodObjIP, false); err != nil { - log.Errorf("Error: failed to add pod to namespace ipset.") + // Assume that the pod IP will be released when pod moves to succeeded or failed state. + // If the pod transitions back to an active state, then add operation will re establish the updated pod info. + if newPodObj.ObjectMeta.DeletionTimestamp == nil && newPodObj.ObjectMeta.DeletionGracePeriodSeconds == nil && + newPodObjPhase != v1.PodSucceeded && newPodObjPhase != v1.PodFailed { + if err = npMgr.AddPod(newPodObj); err != nil { + log.Errorf("Error: failed to add pod during update with error %+v", err) } } - // Updating pod cache with new information - npMgr.nsMap[newPodObjNs].podMap[cachedPodObj.podUID], err = newNpmPod(newPodObj) - if err != nil { - return err - } - return nil } // DeletePod handles deleting pod from its label's ipset. func (npMgr *NetworkPolicyManager) DeletePod(podObj *corev1.Pod) error { + var ( + err error + podNs = "ns-" + podObj.ObjectMeta.Namespace + podUid = string(podObj.ObjectMeta.UID) + podName = podObj.ObjectMeta.Name + podNodeName = podObj.Spec.NodeName + podLabels = podObj.ObjectMeta.Labels + podContainers = podObj.Spec.Containers + ipsMgr = npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr + ) - podNs := util.GetNSNameWithPrefix(podObj.Namespace) - - if _, exists := npMgr.nsMap[podNs]; !exists { - return nil - } - - cachedPodObj, exists := npMgr.nsMap[podNs].podMap[string(podObj.ObjectMeta.UID)] + cachedPodIp, exists := npMgr.podMap[podUid] if !exists { return nil } - var ( - err error - podUID = cachedPodObj.podUID - podName = podObj.ObjectMeta.Name - podNodeName = podObj.Spec.NodeName - podLabels = podObj.ObjectMeta.Labels - ipsMgr = npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr - cachedPodIP = cachedPodObj.podIP - cachedPodLabels = cachedPodObj.labels - ) - // if the podIp exists, it must match the cachedIp - if len(podObj.Status.PodIP) > 0 && cachedPodIP != podObj.Status.PodIP { + if len(podObj.Status.PodIP) > 0 && cachedPodIp != podObj.Status.PodIP { // TODO Add AI telemetry event log.Errorf("Error: Unexpected state. Pod (Namespace:%s, Name:%s, uid:%s, has cachedPodIp:%s which is different from PodIp:%s", - podNs, podName, podUID, cachedPodIP, podObj.Status.PodIP) + podNs, podName, podUid, cachedPodIp, podObj.Status.PodIP) } - log.Logf("POD DELETING: [%s/%s%s/%s%+v%s%+v]", podNs, podName, podUID, podNodeName, podLabels, cachedPodIP, cachedPodLabels) + log.Logf("POD DELETING: [%s/%s%s/%s%+v%s]", podNs, podName, podUid, podNodeName, podLabels, cachedPodIp) // Delete the pod from its namespace's ipset. - if err = ipsMgr.DeleteFromSet(podNs, cachedPodIP, podUID); err != nil { + if err = ipsMgr.DeleteFromSet(podNs, cachedPodIp, podUid); err != nil { log.Errorf("Error: failed to delete pod from namespace ipset.") } // Delete the pod from its label's ipset. - for podLabelKey, podLabelVal := range cachedPodLabels { - log.Logf("Deleting pod %s from ipset %s", cachedPodIP, podLabelKey) - if err = ipsMgr.DeleteFromSet(podLabelKey, cachedPodIP, podUID); err != nil { + for podLabelKey, podLabelVal := range podLabels { + log.Logf("Deleting pod %s from ipset %s", cachedPodIp, podLabelKey) + if err = ipsMgr.DeleteFromSet(podLabelKey, cachedPodIp, podUid); err != nil { log.Errorf("Error: failed to delete pod from label ipset.") } label := podLabelKey + ":" + podLabelVal - log.Logf("Deleting pod %s from ipset %s", cachedPodIP, label) - if err = ipsMgr.DeleteFromSet(label, cachedPodIP, podUID); err != nil { + log.Logf("Deleting pod %s from ipset %s", cachedPodIp, label) + if err = ipsMgr.DeleteFromSet(label, cachedPodIp, podUid); err != nil { log.Errorf("Error: failed to delete pod from label ipset.") } } - // Delete pod's named ports from its ipset. Delete is TRUE - if err = appendNamedPortIpsets(ipsMgr, cachedPodObj.containerPorts, podUID, cachedPodIP, true); err != nil { - log.Errorf("Error: failed to delete pod from namespace ipset.") + // Delete pod's named ports from its ipset. + for _, container := range podContainers { + for _, port := range container.Ports { + if port.Name != "" { + protocol := "" + switch port.Protocol { + case v1.ProtocolUDP: + protocol = util.IpsetUDPFlag + case v1.ProtocolSCTP: + protocol = util.IpsetSCTPFlag + } + namedPortname := util.NamedPortIPSetPrefix + port.Name + ipsMgr.DeleteFromSet( + namedPortname, + fmt.Sprintf("%s,%s%d", cachedPodIp, protocol, port.ContainerPort), + podUid, + ) + } + } } - delete(npMgr.nsMap[podNs].podMap, podUID) + delete(npMgr.podMap, podUid) return nil } diff --git a/npm/pod_test.go b/npm/pod_test.go index f3209fe49c..5ffe957974 100644 --- a/npm/pod_test.go +++ b/npm/pod_test.go @@ -3,7 +3,6 @@ package npm import ( - "reflect" "testing" "github.com/Azure/azure-container-networking/npm/ipsm" @@ -12,7 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestIsValidPod(t *testing.T) { +func TestisValidPod(t *testing.T) { podObj := &corev1.Pod{ Status: corev1.PodStatus{ Phase: "Running", @@ -24,7 +23,7 @@ func TestIsValidPod(t *testing.T) { } } -func TestIsSystemPod(t *testing.T) { +func TestisSystemPod(t *testing.T) { podObj := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: util.KubeSystemFlag, @@ -38,6 +37,7 @@ func TestIsSystemPod(t *testing.T) { func TestAddPod(t *testing.T) { npMgr := &NetworkPolicyManager{ nsMap: make(map[string]*namespace), + podMap: make(map[string]string), TelemetryEnabled: false, } @@ -94,6 +94,7 @@ func TestAddPod(t *testing.T) { func TestUpdatePod(t *testing.T) { npMgr := &NetworkPolicyManager{ nsMap: make(map[string]*namespace), + podMap: make(map[string]string), TelemetryEnabled: false, } @@ -135,7 +136,6 @@ func TestUpdatePod(t *testing.T) { Labels: map[string]string{ "app": "new-test-pod", }, - ResourceVersion: "1", }, Status: corev1.PodStatus{ Phase: "Running", @@ -148,102 +148,16 @@ func TestUpdatePod(t *testing.T) { t.Errorf("TestUpdatePod failed @ AddPod") } - if err := npMgr.UpdatePod(newPodObj); err != nil { + if err := npMgr.UpdatePod(oldPodObj, newPodObj); err != nil { t.Errorf("TestUpdatePod failed @ UpdatePod") } - - cachedPodObj, exists := npMgr.nsMap["ns-"+newPodObj.Namespace].podMap[string(newPodObj.ObjectMeta.UID)] - if !exists { - t.Errorf("TestUpdatePod failed @ pod exists check") - } - - if !reflect.DeepEqual(cachedPodObj.labels, newPodObj.Labels) { - t.Errorf("TestUpdatePod failed @ labels check") - } - npMgr.Unlock() -} - -func TestOldRVUpdatePod(t *testing.T) { - npMgr := &NetworkPolicyManager{ - nsMap: make(map[string]*namespace), - TelemetryEnabled: false, - } - - allNs, err := newNs(util.KubeAllNamespacesFlag) - if err != nil { - panic(err.Error) - } - npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs - - ipsMgr := ipsm.NewIpsetManager() - if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { - t.Errorf("TestOldRVUpdatePod failed @ ipsMgr.Save") - } - - defer func() { - if err := ipsMgr.Restore(util.IpsetTestConfigFile); err != nil { - t.Errorf("TestOldRVUpdatePod failed @ ipsMgr.Restore") - } - }() - - oldPodObj := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "old-test-pod", - Namespace: "test-namespace", - Labels: map[string]string{ - "app": "old-test-pod", - }, - ResourceVersion: "1", - }, - Status: corev1.PodStatus{ - Phase: "Running", - PodIP: "1.2.3.4", - }, - } - - newPodObj := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "new-test-pod", - Namespace: "test-namespace", - Labels: map[string]string{ - "app": "new-test-pod", - }, - ResourceVersion: "0", - }, - Status: corev1.PodStatus{ - Phase: "Running", - PodIP: "4.3.2.1", - }, - } - - npMgr.Lock() - if err := npMgr.AddPod(oldPodObj); err != nil { - t.Errorf("TestOldRVUpdatePod failed @ AddPod") - } - - if err := npMgr.UpdatePod(newPodObj); err != nil { - t.Errorf("TestOldRVUpdatePod failed @ UpdatePod") - } - - cachedPodObj, exists := npMgr.nsMap["ns-"+newPodObj.Namespace].podMap[string(newPodObj.ObjectMeta.UID)] - if !exists { - t.Errorf("TestOldRVUpdatePod failed @ pod exists check") - } - - if cachedPodObj.resourceVersion != 1 { - t.Errorf("TestOldRVUpdatePod failed @ resourceVersion check") - } - - if !reflect.DeepEqual(cachedPodObj.labels, oldPodObj.Labels) { - t.Errorf("TestOldRVUpdatePod failed @ labels check") - } - npMgr.Unlock() } func TestDeletePod(t *testing.T) { npMgr := &NetworkPolicyManager{ nsMap: make(map[string]*namespace), + podMap: make(map[string]string), TelemetryEnabled: false, } @@ -286,16 +200,13 @@ func TestDeletePod(t *testing.T) { if err := npMgr.DeletePod(podObj); err != nil { t.Errorf("TestDeletePod failed @ DeletePod") } - - if len(npMgr.nsMap["ns-"+podObj.Namespace].podMap) > 1 { - t.Errorf("TestDeletePod failed @ podMap length check") - } npMgr.Unlock() } func TestAddHostNetworkPod(t *testing.T) { npMgr := &NetworkPolicyManager{ nsMap: make(map[string]*namespace), + podMap: make(map[string]string), TelemetryEnabled: false, } @@ -338,8 +249,8 @@ func TestAddHostNetworkPod(t *testing.T) { t.Errorf("TestAddHostNetworkPod failed @ AddPod") } - if len(npMgr.nsMap) > 1 { - t.Errorf("TestAddHostNetworkPod failed @ nsMap length check") + if len(npMgr.podMap) >= 1 { + t.Errorf("TestAddHostNetworkPod failed @ podMap length check") } npMgr.Unlock() } @@ -347,6 +258,7 @@ func TestAddHostNetworkPod(t *testing.T) { func TestUpdateHostNetworkPod(t *testing.T) { npMgr := &NetworkPolicyManager{ nsMap: make(map[string]*namespace), + podMap: make(map[string]string), TelemetryEnabled: false, } @@ -398,9 +310,6 @@ func TestUpdateHostNetworkPod(t *testing.T) { Phase: "Running", PodIP: "4.3.2.1", }, - Spec: corev1.PodSpec{ - HostNetwork: true, - }, } npMgr.Lock() @@ -408,11 +317,11 @@ func TestUpdateHostNetworkPod(t *testing.T) { t.Errorf("TestUpdateHostNetworkPod failed @ AddPod") } - if err := npMgr.UpdatePod(newPodObj); err != nil { + if err := npMgr.UpdatePod(oldPodObj, newPodObj); err != nil { t.Errorf("TestUpdateHostNetworkPod failed @ UpdatePod") } - if len(npMgr.nsMap) > 1 { + if len(npMgr.podMap) >= 1 { t.Errorf("TestUpdateHostNetworkPod failed @ podMap length check") } npMgr.Unlock() @@ -421,6 +330,7 @@ func TestUpdateHostNetworkPod(t *testing.T) { func TestDeleteHostNetworkPod(t *testing.T) { npMgr := &NetworkPolicyManager{ nsMap: make(map[string]*namespace), + podMap: make(map[string]string), TelemetryEnabled: false, } @@ -463,7 +373,7 @@ func TestDeleteHostNetworkPod(t *testing.T) { t.Errorf("TestDeleteHostNetworkPod failed @ AddPod") } - if len(npMgr.nsMap) > 1 { + if len(npMgr.podMap) >= 1 { t.Errorf("TestDeleteHostNetworkPod failed @ podMap length check") } diff --git a/npm/util/const.go b/npm/util/const.go index 14146f9dfe..48b4cef362 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -121,7 +121,6 @@ const ( IpsetUDPFlag string = "udp:" IpsetSCTPFlag string = "sctp:" - IpsetTCPFlag string = "tcp:" AzureNpmFlag string = "azure-npm" AzureNpmPrefix string = "azure-npm-" @@ -133,8 +132,6 @@ const ( //Prefixes for ipsets NamedPortIPSetPrefix string = "namedport:" - - NamespacePrefix string = "ns-" ) //NPM telemetry constants. diff --git a/npm/util/util.go b/npm/util/util.go index 6d7cd2d7e0..b23b2e9ddd 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -8,10 +8,8 @@ import ( "os" "regexp" "sort" - "strconv" "strings" - "github.com/Azure/azure-container-networking/log" "github.com/Masterminds/semver" "k8s.io/apimachinery/pkg/version" ) @@ -71,38 +69,26 @@ func SortMap(m *map[string]string) ([]string, []string) { return sortedKeys, sortedVals } -// GetIPSetListFromLabels combine Labels into a single slice -func GetIPSetListFromLabels(labels map[string]string) []string { - var ( - ipsetList = []string{} - ) - for labelKey, labelVal := range labels { - ipsetList = append(ipsetList, labelKey, labelKey+":"+labelVal) - - } - return ipsetList -} - -// GetIPSetListCompareLabels compares Labels and -// returns a delete ipset list and add ipset list -func GetIPSetListCompareLabels(orig map[string]string, new map[string]string) ([]string, []string) { - notInOrig := []string{} - notInNew := []string{} +// CompareMapDiff will compare two maps string[string] and returns +// missing values in both +func CompareMapDiff(orig map[string]string, new map[string]string) (map[string]string, map[string]string) { + notInOrig := make(map[string]string) + notInNew := make(map[string]string) for keyOrig, valOrig := range orig { if valNew, ok := new[keyOrig]; ok { if valNew != valOrig { - notInNew = append(notInNew, keyOrig+":"+valOrig) - notInOrig = append(notInOrig, keyOrig+":"+valNew) + notInNew[keyOrig] = valOrig + notInOrig[keyOrig] = valNew } } else { - notInNew = append(notInNew, keyOrig, keyOrig+":"+valOrig) + notInNew[keyOrig] = valOrig } } for keyNew, valNew := range new { if _, ok := orig[keyNew]; !ok { - notInOrig = append(notInOrig, keyNew, keyNew+":"+valNew) + notInOrig[keyNew] = valNew } } @@ -252,42 +238,3 @@ func DropEmptyFields(s []string) []string { return s } - -// GetNSNameWithPrefix returns Namespace name with ipset prefix -func GetNSNameWithPrefix(nsName string) string { - return NamespacePrefix + nsName -} - -// CompareResourceVersions take in two resource versions and returns true if new is greater than old -func CompareResourceVersions(rvOld string, rvNew string) bool { - // Ignore oldRV error as we care about new RV - tempRvOld := ParseResourceVersion(rvOld) - tempRvnew := ParseResourceVersion(rvNew) - if tempRvnew > tempRvOld { - return true - } - - return false -} - -// CompareUintResourceVersions take in two resource versions as uint and returns true if new is greater than old -func CompareUintResourceVersions(rvOld uint64, rvNew uint64) bool { - if rvNew > rvOld { - return true - } - - return false -} - -// ParseResourceVersion get uint64 version of ResourceVersion -func ParseResourceVersion(rv string) uint64 { - if rv == "" { - return 0 - } - rvInt, err := strconv.ParseUint(rv, 10, 64) - if err != nil { - log.Logf("Error: while parsing resource version to uint64 %s", rv) - } - - return rvInt -} diff --git a/npm/util/util_test.go b/npm/util/util_test.go index 0452d2b976..f388ee030e 100644 --- a/npm/util/util_test.go +++ b/npm/util/util_test.go @@ -1,8 +1,8 @@ package util import ( - "reflect" "testing" + "reflect" "k8s.io/apimachinery/pkg/version" ) @@ -15,7 +15,7 @@ func TestSortMap(t *testing.T) { } sortedKeys, sortedVals := SortMap(m) - + expectedKeys := []string{ "a", "c", @@ -102,12 +102,12 @@ func TestCompareK8sVer(t *testing.T) { Major: "1", Minor: "14.8-hotfix.20191113", } - + secondVer = &version.Info{ Major: "1", Minor: "11", } - + if res := CompareK8sVer(firstVer, secondVer); res != 1 { t.Errorf("TestCompareK8sVer failed @ firstVer > secondVer w/ hotfix tag/pre-release") } @@ -116,12 +116,12 @@ func TestCompareK8sVer(t *testing.T) { Major: "1", Minor: "14+", } - + secondVer = &version.Info{ Major: "1", Minor: "11", } - + if res := CompareK8sVer(firstVer, secondVer); res != 1 { t.Errorf("TestCompareK8sVer failed @ firstVer > secondVer w/ minor+ release") } @@ -227,46 +227,4 @@ func TestDropEmptyFields(t *testing.T) { if !reflect.DeepEqual(resultSlice, expectedSlice) { t.Errorf("TestDropEmptyFields failed @ slice comparison") } -} - -func TestCompareResourceVersions(t *testing.T) { - oldRv := "12345" - newRV := "23456" - - check := CompareResourceVersions(oldRv, newRV) - if !check { - t.Errorf("TestCompareResourceVersions failed @ compare RVs with error returned wrong result ") - } - -} - -func TestInValidOldResourceVersions(t *testing.T) { - oldRv := "sssss" - newRV := "23456" - - check := CompareResourceVersions(oldRv, newRV) - if !check { - t.Errorf("TestInValidOldResourceVersions failed @ compare RVs with error returned wrong result ") - } - -} - -func TestInValidNewResourceVersions(t *testing.T) { - oldRv := "12345" - newRV := "sssss" - - check := CompareResourceVersions(oldRv, newRV) - if check { - t.Errorf("TestInValidNewResourceVersions failed @ compare RVs with error returned wrong result ") - } - -} - -func TestParseResourceVersion(t *testing.T) { - testRv := "string" - - check := ParseResourceVersion(testRv) - if check > 0 { - t.Errorf("TestParseResourceVersion failed @ inavlid RV gave no error") - } -} +} \ No newline at end of file