Merge pull request #10182 from iholder101/bugfix/revert-tsc-master-nodes

Stop considering nodes without `kubevirt.io/schedulable` label when finding lowest TSC frequency on the cluster

pkg/virt-controller/watch/topology/filter.go

@@ -3,6 +3,8 @@ package topology
 import (
 	"math"
 
+	"k8s.io/client-go/tools/cache"
+
 	v1 "k8s.io/api/core/v1"
 
 	virtv1 "kubevirt.io/api/core/v1"
@@ -21,19 +23,7 @@ func IsSchedulable(node *v1.Node) bool {
 		return false
 	}
 
-	if node.Labels[virtv1.NodeSchedulable] != "true" || node.Spec.Unschedulable {
-		return false
-	}
-
-	if node.Spec.Taints != nil {
-		for _, taint := range node.Spec.Taints {
-			if taint.Effect == v1.TaintEffectNoSchedule {
-				return false
-			}
-		}
-	}
-
-	return true
+	return node.Labels[virtv1.NodeSchedulable] == "true"
 }
 
 func HasInvTSCFrequency(node *v1.Node) bool {
@@ -87,6 +77,17 @@ func Not(f FilterPredicateFunc) FilterPredicateFunc {
 	}
 }
 
+func Or(predicates ...FilterPredicateFunc) FilterPredicateFunc {
+	return func(node *v1.Node) bool {
+		for _, p := range predicates {
+			if p(node) {
+				return true
+			}
+		}
+		return false
+	}
+}
+
 func FilterNodesFromCache(objs []interface{}, predicates ...FilterPredicateFunc) []*v1.Node {
 	match := []*v1.Node{}
 	for _, obj := range objs {
@@ -105,6 +106,22 @@ func FilterNodesFromCache(objs []interface{}, predicates ...FilterPredicateFunc)
 	return match
 }
 
+func IsNodeRunningVmis(vmiStore cache.Store) FilterPredicateFunc {
+	return func(node *v1.Node) bool {
+		if node == nil {
+			return false
+		}
+
+		for _, vmi := range vmiStore.List() {
+			vmi := vmi.(*virtv1.VirtualMachineInstance)
+			if vmi.Status.NodeName == node.Name {
+				return true
+			}
+		}
+		return false
+	}
+}
+
 // ToleranceForFrequency returns TSCTolerancePPM parts per million of freq, rounded down to the nearest Hz
 func ToleranceForFrequency(freq int64) int64 {
 	return int64(math.Floor(float64(freq) * (TSCTolerancePPM / 1000000)))

pkg/virt-controller/watch/topology/filter_test.go

@@ -7,6 +7,7 @@ import (
 	. "github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/tools/cache"
 
 	virtv1 "kubevirt.io/api/core/v1"
 
@@ -30,40 +31,6 @@ var _ = Describe("Filter", func() {
 		))
 	})
 
-	It("should filter out nodes with a noschedule taint", func() {
-		nodeWithUnschedulableTaint := node("node0", true)
-		nodeWithUnschedulableTaint.Spec.Taints = []v1.Taint{{Key: "noschedule", Effect: v1.TaintEffectNoSchedule}}
-
-		nodes := topology.NodesToObjects(
-			nodeWithUnschedulableTaint,
-			node("node1", false),
-			node("node2", true),
-			nil,
-		)
-		Expect(topology.FilterNodesFromCache(nodes,
-			topology.IsSchedulable,
-		)).To(ConsistOf(
-			nodes[2],
-		))
-	})
-
-	It("should filter out nodes with spec.unschedulable == true", func() {
-		nodeWithUnschedulableField := node("node0", true)
-		nodeWithUnschedulableField.Spec.Unschedulable = true
-
-		nodes := topology.NodesToObjects(
-			nodeWithUnschedulableField,
-			node("node1", false),
-			node("node2", true),
-			nil,
-		)
-		Expect(topology.FilterNodesFromCache(nodes,
-			topology.IsSchedulable,
-		)).To(ConsistOf(
-			nodes[2],
-		))
-	})
-
 	It("should inverse the search result", func() {
 		nodes := topology.NodesToObjects(
 			node("node0", true),
@@ -147,6 +114,48 @@ var _ = Describe("Filter", func() {
 			topology.NodeOfVMI(vmiOnNode("myvmi", "")),
 		)).To(BeEmpty())
 	})
+
+	It("should filter nodes running vmis", func() {
+		vmiStore := cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)
+		vmi := vmiOnNode("myvmi", "node1")
+		err := vmiStore.Add(vmi)
+		Expect(err).ToNot(HaveOccurred())
+
+		nodes := topology.NodesToObjects(
+			node("node0", false),
+			node("node1", false),
+			node("node2", false),
+			nil,
+		)
+		Expect(topology.FilterNodesFromCache(nodes,
+			topology.IsNodeRunningVmis(vmiStore),
+		)).To(ConsistOf(
+			nodes[1],
+		))
+	})
+
+	It("should filter nodes running vmis and schedulable nodes", func() {
+		vmiStore := cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)
+		vmi := vmiOnNode("myvmi", "node1")
+		err := vmiStore.Add(vmi)
+		Expect(err).ToNot(HaveOccurred())
+
+		nodes := topology.NodesToObjects(
+			node("node0", false),
+			node("node1", false),
+			node("node2", true),
+			nil,
+		)
+		Expect(topology.FilterNodesFromCache(nodes,
+			topology.Or(
+				topology.IsNodeRunningVmis(vmiStore),
+				topology.IsSchedulable,
+			),
+		)).To(ConsistOf(
+			nodes[1],
+			nodes[2],
+		))
+	})
 })
 
 func node(name string, schedulable bool) *v1.Node {

pkg/virt-controller/watch/topology/hinter.go

@@ -56,7 +56,10 @@ func (t *topologyHinter) LowestTSCFrequencyOnCluster() (int64, error) {
 	}
 	nodes := FilterNodesFromCache(t.nodeStore.List(),
 		HasInvTSCFrequency,
-		IsSchedulable,
+		Or(
+			IsSchedulable,
+			IsNodeRunningVmis(t.vmiStore),
+		),
 	)
 	freq := LowestTSCFrequency(nodes)
 	return freq, nil