diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index bda2e825598..d677c2c371f 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1076,7 +1076,7 @@ func NewMainKubelet(ctx context.Context, klet.version = v klet.nodeDeclaredFeaturesFramework = framework klet.nodeDeclaredFeatures = klet.discoverNodeDeclaredFeatures() - klet.nodeDeclaredFeaturesSet = ndf.NewFeatureSet(klet.nodeDeclaredFeatures...) + klet.nodeDeclaredFeaturesSet = framework.MustMapSorted(klet.nodeDeclaredFeatures) } // Safe, allowed sysctls can always be used as unsafe sysctls in the spec. @@ -2927,8 +2927,8 @@ func (kl *Kubelet) HandlePodUpdates(ctx context.Context, pods []*v1.Pod) { if err != nil { logger.Error(err, "Failed to infer required features for pod update", "pod", klog.KObj(pod)) } - if reqs.Len() != 0 { - matchResult, err := ndf.MatchNodeFeatureSet(reqs, kl.nodeDeclaredFeaturesSet) + if !reqs.IsEmpty() { + matchResult, err := kl.nodeDeclaredFeaturesFramework.MatchNodeFeatureSet(reqs, kl.nodeDeclaredFeaturesSet) if err != nil { logger.Error(err, "Failed to match pod features with the node", "pod", klog.KObj(pod)) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index f81ae75f462..82b06c6b4af 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -5002,7 +5002,7 @@ func TestSyncPodNodeDeclaredFeaturesUpdate(t *testing.T) { oldPod: oldPod, newPod: newPod, nodeFeatures: []string{"FeatureB"}, - registeredFeatures: []ndf.Feature{createMockFeature(t, "FeatureA", true, "")}, + registeredFeatures: []ndf.Feature{createMockFeature(t, "FeatureA", true, ""), createMockFeature(t, "FeatureB", false, "")}, expectEvent: true, expectedEventMsg: "Pod requires node features that are not available: FeatureA", }, @@ -5012,7 +5012,7 @@ func TestSyncPodNodeDeclaredFeaturesUpdate(t *testing.T) { componentVersion: "1.35.0", oldPod: oldPod, newPod: newPod, - nodeFeatures: []string{""}, + nodeFeatures: []string{}, registeredFeatures: []ndf.Feature{createMockFeature(t, "FeatureA", true, "1.34.0")}, expectEvent: false, }, @@ -5023,7 +5023,7 @@ func TestSyncPodNodeDeclaredFeaturesUpdate(t *testing.T) { oldPod: nil, newPod: newPod, nodeFeatures: []string{"FeatureB"}, - registeredFeatures: []ndf.Feature{createMockFeature(t, "FeatureA", true, "")}, + registeredFeatures: []ndf.Feature{createMockFeature(t, "FeatureA", true, ""), createMockFeature(t, "FeatureB", false, "")}, expectEvent: false, }, } @@ -5044,7 +5044,7 @@ func TestSyncPodNodeDeclaredFeaturesUpdate(t *testing.T) { framework := ndf.New(tc.registeredFeatures) kubelet.nodeDeclaredFeaturesFramework = framework kubelet.nodeDeclaredFeatures = tc.nodeFeatures - kubelet.nodeDeclaredFeaturesSet = ndf.NewFeatureSet(kubelet.nodeDeclaredFeatures...) + kubelet.nodeDeclaredFeaturesSet = framework.MustMapSorted(kubelet.nodeDeclaredFeatures) kubelet.version = utilversion.MustParse("1.35.0") if tc.oldPod != nil { diff --git a/pkg/kubelet/lifecycle/handlers.go b/pkg/kubelet/lifecycle/handlers.go index 4c5f3f7c28b..c2de1e8c030 100644 --- a/pkg/kubelet/lifecycle/handlers.go +++ b/pkg/kubelet/lifecycle/handlers.go @@ -271,11 +271,11 @@ func (c *declaredFeaturesAdmitHandler) Admit(attrs *PodAdmitAttributes) PodAdmit } } - if reqs.Len() == 0 { + if reqs.IsEmpty() { return PodAdmitResult{Admit: true} } - matchResult, err := ndf.MatchNodeFeatureSet(reqs, c.ndfSet) + matchResult, err := c.ndfFramework.MatchNodeFeatureSet(reqs, c.ndfSet) if err != nil { return PodAdmitResult{ Admit: false, diff --git a/pkg/kubelet/lifecycle/handlers_test.go b/pkg/kubelet/lifecycle/handlers_test.go index 274f664e44c..25f3dce2b9f 100644 --- a/pkg/kubelet/lifecycle/handlers_test.go +++ b/pkg/kubelet/lifecycle/handlers_test.go @@ -910,7 +910,8 @@ func TestDeclaredFeaturesAdmitHandler(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { framework := ndf.New(tc.registeredFeatures) - handler := NewDeclaredFeaturesAdmitHandler(framework, ndf.NewFeatureSet(tc.nodeDeclaredFeatures...), tc.version) + fs := framework.MustMapSorted(tc.nodeDeclaredFeatures) + handler := NewDeclaredFeaturesAdmitHandler(framework, fs, tc.version) attrs := &PodAdmitAttributes{Pod: pod} result := handler.Admit(attrs) diff --git a/pkg/scheduler/framework/plugins/nodedeclaredfeatures/nodedeclaredfeatures.go b/pkg/scheduler/framework/plugins/nodedeclaredfeatures/nodedeclaredfeatures.go index 6b602c157ba..f72c8cf4942 100644 --- a/pkg/scheduler/framework/plugins/nodedeclaredfeatures/nodedeclaredfeatures.go +++ b/pkg/scheduler/framework/plugins/nodedeclaredfeatures/nodedeclaredfeatures.go @@ -24,7 +24,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/sets" versionutil "k8s.io/apimachinery/pkg/util/version" "k8s.io/component-base/version" ndf "k8s.io/component-helpers/nodedeclaredfeatures" @@ -94,7 +93,7 @@ func (pl *NodeDeclaredFeatures) PreFilter(ctx context.Context, cycleState fwk.Cy if err != nil { return nil, fwk.AsStatus(err) } - if reqs.Len() == 0 { + if reqs.IsEmpty() { return nil, fwk.NewStatus(fwk.Skip) } cycleState.Write(preFilterStateKey, &preFilterState{reqs: reqs}) @@ -115,7 +114,7 @@ func (pl *NodeDeclaredFeatures) Filter(ctx context.Context, cycleState fwk.Cycle if err != nil { return fwk.AsStatus(err) } - result, err := ndf.MatchNodeFeatureSet(s.reqs, nodeInfo.GetNodeDeclaredFeatures()) + result, err := ndf.DefaultFramework.MatchNodeFeatureSet(s.reqs, nodeInfo.GetNodeDeclaredFeatures()) if err != nil { return fwk.AsStatus(err) } @@ -131,9 +130,8 @@ func (pl *NodeDeclaredFeatures) SignPod(ctx context.Context, pod *v1.Pod) ([]fwk if err != nil { return nil, fwk.AsStatus(err) } - featuresList := sets.List(fs.Set) return []fwk.SignFragment{ - {Key: fwk.FeaturesSignerName, Value: featuresList}, + {Key: fwk.FeaturesSignerName, Value: fs.String()}, }, nil } diff --git a/pkg/scheduler/framework/plugins/nodedeclaredfeatures/nodedeclaredfeatures_test.go b/pkg/scheduler/framework/plugins/nodedeclaredfeatures/nodedeclaredfeatures_test.go index 66dc0781b83..65b1a413d55 100644 --- a/pkg/scheduler/framework/plugins/nodedeclaredfeatures/nodedeclaredfeatures_test.go +++ b/pkg/scheduler/framework/plugins/nodedeclaredfeatures/nodedeclaredfeatures_test.go @@ -51,6 +51,14 @@ func createMockFeature(t *testing.T, name string, infer bool, maxVersionStr stri } func TestPreFilter(t *testing.T) { + const ( + feature1 = "TestFeature1" + feature2 = "TestFeature2" + ) + mapper := ndf.NewFeatureMapper([]string{feature1, feature2}) + newFS := func(features ...string) ndf.FeatureSet { + return mapper.MustMapSorted(features) + } _, ctx := ktesting.NewTestContext(t) testCases := []struct { name string @@ -67,7 +75,7 @@ func TestPreFilter(t *testing.T) { pod: st.MakePod().Name("test-pod").Obj(), componenetVersion: "1.35.0", nodeFeatures: []ndf.Feature{ - createMockFeature(t, "TestFeature", true, ""), + createMockFeature(t, feature1, true, ""), }, expectedStatus: fwk.NewStatus(fwk.Skip), expectedState: nil, @@ -78,10 +86,10 @@ func TestPreFilter(t *testing.T) { pod: st.MakePod().Name("test-pod").Obj(), componenetVersion: "1.35.0", nodeFeatures: []ndf.Feature{ - createMockFeature(t, "TestFeature", true, ""), + createMockFeature(t, feature1, true, ""), }, expectedStatus: fwk.NewStatus(fwk.Success), - expectedState: &preFilterState{reqs: ndf.NewFeatureSet("TestFeature")}, + expectedState: &preFilterState{reqs: newFS(feature1)}, }, { name: "Pod with multiple feature requirements", @@ -89,11 +97,11 @@ func TestPreFilter(t *testing.T) { pod: st.MakePod().Name("test-pod").Obj(), componenetVersion: "1.35.0", nodeFeatures: []ndf.Feature{ - createMockFeature(t, "TestFeature1", true, "1.38.0"), - createMockFeature(t, "TestFeature2", true, "1.38.0"), + createMockFeature(t, feature1, true, "1.38.0"), + createMockFeature(t, feature2, true, "1.38.0"), }, expectedStatus: fwk.NewStatus(fwk.Success), - expectedState: &preFilterState{reqs: ndf.NewFeatureSet("TestFeature1", "TestFeature2")}, + expectedState: &preFilterState{reqs: newFS(feature1, feature2)}, }, { name: "Pod with no requirements", @@ -101,7 +109,7 @@ func TestPreFilter(t *testing.T) { pod: st.MakePod().Name("test-pod").Obj(), componenetVersion: "1.35.0", nodeFeatures: []ndf.Feature{ - createMockFeature(t, "TestFeature", false, ""), + createMockFeature(t, feature1, false, ""), }, expectedStatus: fwk.NewStatus(fwk.Skip), expectedState: nil, @@ -112,7 +120,7 @@ func TestPreFilter(t *testing.T) { pod: st.MakePod().Name("test-pod").Obj(), componenetVersion: "1.34.0", nodeFeatures: []ndf.Feature{ - createMockFeature(t, "TestFeature", true, "1.33.0"), + createMockFeature(t, feature1, true, "1.33.0"), }, expectedStatus: fwk.NewStatus(fwk.Skip), expectedState: nil, @@ -159,6 +167,16 @@ func TestPreFilter(t *testing.T) { func TestFilter(t *testing.T) { _, ctx := ktesting.NewTestContext(t) + const ( + featureA = "FeatureA" + featureB = "FeatureB" + featureC = "FeatureC" + ) + f, _ := ndftesting.NewMockFramework(t, featureA, featureB, featureC) + ndftesting.SetFrameworkDuringTest(t, f) + newFS := func(features ...string) ndf.FeatureSet { + return f.MustMapSorted(features) + } testCases := []struct { name string @@ -172,7 +190,7 @@ func TestFilter(t *testing.T) { name: "plugin disabled", pluginEnabled: false, pod: st.MakePod().Name("test-pod").Obj(), - node: st.MakeNode().Name("node-1").DeclaredFeatures([]string{"FeatureA", "FeatureB"}).Obj(), + node: st.MakeNode().Name("node-1").DeclaredFeatures([]string{featureA, featureB}).Obj(), preFilterReqs: nil, expectedStatus: nil, }, @@ -180,23 +198,23 @@ func TestFilter(t *testing.T) { name: "Node matches requirements", pluginEnabled: true, pod: st.MakePod().Name("test-pod").Obj(), - node: st.MakeNode().Name("node-1").DeclaredFeatures([]string{"FeatureA", "FeatureB"}).Obj(), - preFilterReqs: []string{"FeatureA"}, + node: st.MakeNode().Name("node-1").DeclaredFeatures([]string{featureA, featureB}).Obj(), + preFilterReqs: []string{featureA}, expectedStatus: fwk.NewStatus(fwk.Success), }, { name: "Node does not match requirements", pluginEnabled: true, pod: st.MakePod().Name("test-pod").Obj(), - node: st.MakeNode().Name("node-1").DeclaredFeatures([]string{"FeatureB"}).Obj(), - preFilterReqs: []string{"FeatureA"}, + node: st.MakeNode().Name("node-1").DeclaredFeatures([]string{featureB}).Obj(), + preFilterReqs: []string{featureA}, expectedStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node declared features check failed - unsatisfied requirements: FeatureA"), }, { name: "Node with multiple features, pod requires subset", pod: st.MakePod().Name("test-pod").Obj(), - node: st.MakeNode().Name("node-multi").DeclaredFeatures([]string{"FeatureA", "FeatureB", "FeatureC"}).Obj(), - preFilterReqs: []string{"FeatureA", "FeatureC"}, + node: st.MakeNode().Name("node-multi").DeclaredFeatures([]string{featureA, featureB, featureC}).Obj(), + preFilterReqs: []string{featureA, featureC}, expectedStatus: fwk.NewStatus(fwk.Success), }, { @@ -204,15 +222,15 @@ func TestFilter(t *testing.T) { pluginEnabled: true, pod: st.MakePod().Name("test-pod").Obj(), node: st.MakeNode().Name("node-1").Obj(), - preFilterReqs: []string{"FeatureA"}, + preFilterReqs: []string{featureA}, expectedStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node declared features check failed - unsatisfied requirements: FeatureA"), }, { name: "Node with some but not all required features", pluginEnabled: true, pod: st.MakePod().Name("test-pod").Obj(), - node: st.MakeNode().Name("node-1").DeclaredFeatures([]string{"FeatureA"}).Obj(), - preFilterReqs: []string{"FeatureA", "FeatureB"}, + node: st.MakeNode().Name("node-1").DeclaredFeatures([]string{featureA}).Obj(), + preFilterReqs: []string{featureA, featureB}, expectedStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node declared features check failed - unsatisfied requirements: FeatureB"), }, { @@ -239,7 +257,7 @@ func TestFilter(t *testing.T) { } cycleState := framework.NewCycleState() if tc.preFilterReqs != nil { - cycleState.Write(preFilterStateKey, &preFilterState{reqs: ndf.NewFeatureSet(tc.preFilterReqs...)}) + cycleState.Write(preFilterStateKey, &preFilterState{reqs: newFS(tc.preFilterReqs...)}) } status := plugin.Filter(ctx, cycleState, tc.pod, nodeInfo) diff --git a/pkg/scheduler/framework/types.go b/pkg/scheduler/framework/types.go index fe98008e301..d7e216ab648 100644 --- a/pkg/scheduler/framework/types.go +++ b/pkg/scheduler/framework/types.go @@ -478,7 +478,10 @@ func (n *NodeInfo) SetNode(node *v1.Node) { n.node = node n.Allocatable = NewResource(node.Status.Allocatable) if utilfeature.DefaultFeatureGate.Enabled(features.NodeDeclaredFeatures) { - n.DeclaredFeatures = ndf.NewFeatureSet(node.Status.DeclaredFeatures...) + // Use TryMap rather than Map here in case the node has unknown features. Since we're only + // concerned with matching known features to the node, there is no risk to discarding + // unknown features. + n.DeclaredFeatures = ndf.DefaultFramework.TryMap(node.Status.DeclaredFeatures) } n.Generation = nextGeneration() } diff --git a/pkg/scheduler/framework/types_test.go b/pkg/scheduler/framework/types_test.go index fe910a19341..97c74d9c883 100644 --- a/pkg/scheduler/framework/types_test.go +++ b/pkg/scheduler/framework/types_test.go @@ -30,6 +30,7 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" featuregatetesting "k8s.io/component-base/featuregate/testing" ndf "k8s.io/component-helpers/nodedeclaredfeatures" + ndftesting "k8s.io/component-helpers/nodedeclaredfeatures/testing" "k8s.io/klog/v2" fwk "k8s.io/kube-scheduler/framework" "k8s.io/kubernetes/pkg/features" @@ -362,6 +363,8 @@ func TestNewNodeInfo(t *testing.T) { func TestNodeInfoClone(t *testing.T) { nodeName := "test-node" + declaredFeatureSet := ndf.NewFeatureMapper([]string{"A", "B", "C"}).MustMapSorted([]string{"A", "C"}) + tests := []struct { nodeInfo *NodeInfo expected *NodeInfo @@ -557,7 +560,7 @@ func TestNodeInfoClone(t *testing.T) { UsedPorts: fwk.HostPortInfo{}, ImageStates: map[string]*fwk.ImageStateSummary{}, PVCRefCounts: map[string]int{}, - DeclaredFeatures: ndf.NewFeatureSet("FeatureA", "FeatureB"), + DeclaredFeatures: declaredFeatureSet.Clone(), }, expected: &NodeInfo{ Requested: &Resource{}, @@ -567,7 +570,7 @@ func TestNodeInfoClone(t *testing.T) { UsedPorts: fwk.HostPortInfo{}, ImageStates: map[string]*fwk.ImageStateSummary{}, PVCRefCounts: map[string]int{}, - DeclaredFeatures: ndf.NewFeatureSet("FeatureA", "FeatureB"), + DeclaredFeatures: declaredFeatureSet, }, }, } @@ -1240,11 +1243,13 @@ func TestNodeInfoRemovePod(t *testing.T) { } func TestSetNodeDeclaredFeatures(t *testing.T) { + ndfFramework, _ := ndftesting.NewMockFramework(t, "FeatureA", "FeatureB") + ndftesting.SetFrameworkDuringTest(t, ndfFramework) tests := []struct { name string featureGateEnabled bool nodeStatus v1.NodeStatus - expectedFeatures ndf.FeatureSet + expectedFeatures []string }{ { name: "Feature gate disabled", @@ -1252,7 +1257,7 @@ func TestSetNodeDeclaredFeatures(t *testing.T) { nodeStatus: v1.NodeStatus{ DeclaredFeatures: []string{"FeatureA", "FeatureB"}, }, - expectedFeatures: ndf.NewFeatureSet(), + expectedFeatures: nil, }, { name: "Feature gate enabled, node has features", @@ -1260,7 +1265,7 @@ func TestSetNodeDeclaredFeatures(t *testing.T) { nodeStatus: v1.NodeStatus{ DeclaredFeatures: []string{"FeatureA", "FeatureB"}, }, - expectedFeatures: ndf.NewFeatureSet("FeatureA", "FeatureB"), + expectedFeatures: []string{"FeatureA", "FeatureB"}, }, { name: "Feature gate enabled, node has no features", @@ -1268,7 +1273,15 @@ func TestSetNodeDeclaredFeatures(t *testing.T) { nodeStatus: v1.NodeStatus{ DeclaredFeatures: []string{}, }, - expectedFeatures: ndf.NewFeatureSet(), + expectedFeatures: nil, + }, + { + name: "Feature gate enabled, node has an unknown feature", + featureGateEnabled: true, + nodeStatus: v1.NodeStatus{ + DeclaredFeatures: []string{"FeatureA", "OtherFeature"}, + }, + expectedFeatures: []string{"FeatureA"}, }, { name: "Feature gate enabled, node status has nil features", @@ -1276,7 +1289,7 @@ func TestSetNodeDeclaredFeatures(t *testing.T) { nodeStatus: v1.NodeStatus{ DeclaredFeatures: nil, }, - expectedFeatures: ndf.NewFeatureSet(), + expectedFeatures: nil, }, } @@ -1290,8 +1303,23 @@ func TestSetNodeDeclaredFeatures(t *testing.T) { } ni.SetNode(node) gotFeatures := ni.GetNodeDeclaredFeatures() - if !gotFeatures.Equal(tt.expectedFeatures) { - t.Errorf("SetNode() or GetNodeDeclaredFeatures() unexpected result, got: %v, want: %v", gotFeatures, tt.expectedFeatures) + if !tt.featureGateEnabled { + if !gotFeatures.IsEmpty() { + got, err := ndfFramework.Unmap(gotFeatures) + if err != nil { + t.Fatalf("Failed to unmap features: %v", err) + } + t.Errorf("Expected GetNodeDeclaredFeatures() to return nil; got %v", got) + } + return + } + expected := ndfFramework.MustMapSorted(tt.expectedFeatures) + if !gotFeatures.Equal(expected) { + got, err := ndfFramework.Unmap(gotFeatures) + if err != nil { + t.Fatalf("Failed to unmap features: %v", err) + } + t.Errorf("SetNode() or GetNodeDeclaredFeatures() unexpected result, got: %v, want: %v", got, tt.expectedFeatures) } }) } diff --git a/plugin/pkg/admission/nodedeclaredfeatures/admission.go b/plugin/pkg/admission/nodedeclaredfeatures/admission.go index 93a4519c259..884307bab10 100644 --- a/plugin/pkg/admission/nodedeclaredfeatures/admission.go +++ b/plugin/pkg/admission/nodedeclaredfeatures/admission.go @@ -163,7 +163,7 @@ func (p *Plugin) validatePodUpdate(pod, oldPod *core.Pod, a admission.Attributes return admission.NewForbidden(a, fmt.Errorf("failed to infer pod capability requirements: %w", err)) } // If there are no specific feature requirements for this update, we're done. - if reqs.Len() == 0 { + if reqs.IsEmpty() { return nil } node, err := p.nodeLister.Get(pod.Spec.NodeName) @@ -173,7 +173,7 @@ func (p *Plugin) validatePodUpdate(pod, oldPod *core.Pod, a admission.Attributes } return admission.NewForbidden(a, fmt.Errorf("failed to get node %q: %w", pod.Spec.NodeName, err)) } - result, err := ndf.MatchNode(reqs, node) + result, err := p.nodeDeclaredFeatureFramework.MatchNode(reqs, node) if err != nil { return admission.NewForbidden(a, fmt.Errorf("failed to match pod requirements against node %q: %w", node.Name, err)) } diff --git a/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/bitmap.go b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/bitmap.go new file mode 100644 index 00000000000..d6d88adb6f4 --- /dev/null +++ b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/bitmap.go @@ -0,0 +1,121 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nodedeclaredfeatures + +import ( + "encoding/binary" + "encoding/hex" + "fmt" + "slices" +) + +type bitmap []uint64 + +// newBitmap creates a new bitmap that can store the number of bits specified by size. +func newBitmap(size int) bitmap { + return make(bitmap, (size+63)/64) +} + +// Set sets the bit at the specified index. +func (b bitmap) Set(i int) { + if i < 0 || i/64+1 > len(b) { + panic(fmt.Sprintf("bitmap index out of range: %d", i)) + } + + b[i/64] |= 1 << (i % 64) +} + +// Get returns the bit value at the specified index. +func (b bitmap) Get(i int) bool { + if i < 0 || i/64+1 > len(b) { + panic(fmt.Sprintf("bitmap index out of range: %d", i)) + } + + return b[i/64]&(1<<(i%64)) != 0 +} + +// Difference returns a new bitmap containing bits set in b but not in other. +func (b bitmap) Difference(other bitmap) (bitmap, error) { + if len(b) != len(other) { + return nil, fmt.Errorf("bitmap size mismatch: %d != %d", len(b), len(other)) + } + + diff := make(bitmap, len(b)) + for i := range b { + diff[i] = b[i] &^ other[i] + } + return diff, nil +} + +// Contains checks whether b is a subset of other. +func (b bitmap) IsSubset(other bitmap) (bool, error) { + if len(b) != len(other) { + return false, fmt.Errorf("bitmap size mismatch; %d != %d", len(b), len(other)) + } + for i := range b { + if b[i]&^other[i] != 0 { + return false, nil + } + } + return true, nil +} + +// Equal returns true if b and other have the same bits set. +func (b bitmap) Equal(other bitmap) bool { + if len(b) != len(other) { + return false + } + + for i := range b { + if b[i] != other[i] { + return false + } + } + return true +} + +// IsEmpty returns true if no bits are set in the bitmap. +func (b bitmap) IsEmpty() bool { + for i := range b { + if b[i] != 0 { + return false + } + } + return true +} + +// Clone returns a deep copy of the bitmap. +func (b bitmap) Clone() bitmap { + return slices.Clone(b) +} + +// String returns a binary representation, with leftmost character representing +// the first (0th) bit. +func (b bitmap) String() string { + // Pre-allocate buffer (16 chars per uint64) + buf := make([]byte, len(b)*16) + + var scratch [8]byte // Holds byte representation + for i, n := range b { + // Convert uint64 to bytes + binary.BigEndian.PutUint64(scratch[:], n) + // Encode bytes to 16 hex characters. + hex.Encode(buf[i*16:], scratch[:]) + } + + return string(buf) +} diff --git a/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/bitmap_test.go b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/bitmap_test.go new file mode 100644 index 00000000000..afb1ed0b8a4 --- /dev/null +++ b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/bitmap_test.go @@ -0,0 +1,319 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nodedeclaredfeatures + +import ( + "fmt" + "testing" +) + +func TestNewBitmap(t *testing.T) { + tests := []struct { + name string + size int + expected int // number of uint64 elements + }{ + {"Size 0", 0, 0}, + {"Size 1", 1, 1}, + {"Size 63", 63, 1}, + {"Size 64", 64, 1}, // (64+63)/64 = 1 + {"Size 65", 65, 2}, + {"Size 128", 128, 2}, // (128+63)/64 = 2 + {"Size 129", 129, 3}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := newBitmap(tt.size) + if len(b) != tt.expected { + t.Errorf("len(b) = %d, want %d", len(b), tt.expected) + } + }) + } +} + +func TestBitmap_GetSet(t *testing.T) { + b := newBitmap(128) + indices := []int{0, 63, 64, 127} + + // Initially all false + for _, i := range indices { + if b.Get(i) { + t.Errorf("expected bit %d to be false initially", i) + } + } + + // Set bits + for _, i := range indices { + b.Set(i) + } + + for _, i := range indices { + if !b.Get(i) { + t.Errorf("expected bit %d to be true after Set", i) + } + } + + // Verify other bits are still false + for _, i := range []int{1, 62, 65} { + if b.Get(i) { + t.Errorf("expected bit %d to be false", i) + } + } + + // Error bounds. + for _, i := range []int{-1, 128} { + msg := fmt.Sprintf("bitmap index out of range: %d", i) + t.Run(fmt.Sprintf("Panic_Set_%d", i), func(t *testing.T) { + defer expectPanic(t, msg) + b.Set(i) + }) + t.Run(fmt.Sprintf("Panic_Get_%d", i), func(t *testing.T) { + defer expectPanic(t, msg) + b.Get(i) + }) + } +} + +func TestBitmap_DifferenceSubset(t *testing.T) { + tests := []struct { + name string + bSize int + otherSize int + bSets []int + otherSets []int + expectedErr bool + expectedDiff []int // expect other.Contains IFF expectedDiff is empty. + }{ + { + name: "Mismatch size", + bSize: 64, + otherSize: 128, + expectedErr: true, + }, + { + name: "Disjoint sets", + bSize: 64, + otherSize: 64, + bSets: []int{1, 2}, + otherSets: []int{3, 4}, + expectedDiff: []int{1, 2}, + }, + { + name: "Overlap", + bSize: 64, + otherSize: 64, + bSets: []int{1, 2, 3}, + otherSets: []int{2, 3, 4}, + expectedDiff: []int{1}, // 2 and 3 removed + }, + { + name: "Identical", + bSize: 64, + otherSize: 64, + bSets: []int{1, 2}, + otherSets: []int{1, 2}, + expectedDiff: []int{}, + }, + { + name: "Subset", + bSize: 64, + otherSize: 64, + bSets: []int{1, 2}, + otherSets: []int{1, 2, 3}, + expectedDiff: []int{}, + }, + { + name: "Superset", + bSize: 64, + otherSize: 64, + bSets: []int{1, 2, 3}, + otherSets: []int{1, 2}, + expectedDiff: []int{3}, + }, + { + name: "Empty contains Empty", + bSize: 64, + otherSize: 64, + expectedDiff: []int{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := newBitmap(tt.bSize) + for _, i := range tt.bSets { + b.Set(i) + } + other := newBitmap(tt.otherSize) + for _, i := range tt.otherSets { + other.Set(i) + } + + // Test Difference + diff, err := b.Difference(other) + if tt.expectedErr { + if err == nil { + t.Error("expected error, got nil") + } + if diff != nil { + t.Errorf("expected nil diff, got %v", diff) + } + } else { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + for _, i := range tt.expectedDiff { + if !diff.Get(i) { + t.Errorf("Difference: Index %d should be set", i) + } + } + // Verify count of set bits for Difference + count := 0 + for i := 0; i < tt.bSize; i++ { + if diff.Get(i) { + count++ + } + } + if count != len(tt.expectedDiff) { + t.Errorf("Difference: Unexpected number of bits set, got %d, want %d", count, len(tt.expectedDiff)) + } + } + + // Test IsSubset + result, err := b.IsSubset(other) + if tt.expectedErr { + if err == nil { + t.Error("expected error, got nil") + } + if result { + t.Error("expected result to be false on error") + } + } else { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + expectedResult := len(tt.expectedDiff) == 0 + if result != expectedResult { + t.Errorf("IsSubset result mismatch: got %v, want %v", result, expectedResult) + } + } + }) + } +} + +func TestBitmap_Equal(t *testing.T) { + b1 := newBitmap(64) + b1.Set(1) + + b2 := newBitmap(64) + b2.Set(1) + + b3 := newBitmap(64) + b3.Set(2) + + b4 := newBitmap(128) // Different size + b4.Set(1) + + if !b1.Equal(b2) { + t.Error("expected b1 to equal b2") + } + if b1.Equal(b3) { + t.Error("expected b1 not to equal b3") + } + if b1.Equal(b4) { + t.Error("expected b1 not to equal b4") + } +} + +func TestBitmap_IsEmpty(t *testing.T) { + b := newBitmap(64) + if !b.IsEmpty() { + t.Error("expected b to be empty") + } + + b.Set(10) + if b.IsEmpty() { + t.Error("expected b not to be empty") + } +} + +func TestBitmap_Clone(t *testing.T) { + b := newBitmap(64) + b.Set(1) + + clone := b.Clone() + if !b.Equal(clone) { + t.Error("expected clone to equal original") + } + + // Modify clone, original should not change + clone.Set(2) + if clone.Equal(b) { + t.Error("expected modified clone not to equal original") + } + if b.Get(2) { + t.Error("expected original not to have bit 2 set") + } + if !clone.Get(2) { + t.Error("expected clone to have bit 2 set") + } +} + +func TestBitmap_String(t *testing.T) { + // Size 64 -> 1 uint64 + b := newBitmap(64) + // 0x0000000000000001 (bit 0 set) + b.Set(0) + // Output is hex encoded BigEndian of the uint64 + // uint64(1) -> bytes: [0 0 0 0 0 0 0 1] + // hex: "0000000000000001" + if got, want := b.String(), "0000000000000001"; got != want { + t.Errorf("got %q, want %q", got, want) + } + + b2 := newBitmap(64) + b2.Set(63) + // 1 << 63 is the highest bit. + // 0x8000000000000000 + // bytes: [128 0 0 0 0 0 0 0] -> hex "8000000000000000" + if got, want := b2.String(), "8000000000000000"; got != want { + t.Errorf("got %q, want %q", got, want) + } + + // Multi-word + b3 := newBitmap(128) + b3.Set(0) + b3.Set(64) + // Array of 2 uint64s. + // b3[0] has bit 0 set -> "0000000000000001" + // b3[1] has bit 0 (global 64) set -> "0000000000000001" + // Output order: iterates slice. + // "0000000000000001" + "0000000000000001" + if got, want := b3.String(), "00000000000000010000000000000001"; got != want { + t.Errorf("got %q, want %q", got, want) + } +} + +func expectPanic(t *testing.T, expected interface{}) { + r := recover() + if r == nil { + t.Errorf("expected panic %v, but did not panic", expected) + } else if r != expected { + t.Errorf("expected panic %v, but got %v", expected, r) + } +} diff --git a/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/featureset.go b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/featureset.go new file mode 100644 index 00000000000..5b2f38bb5c3 --- /dev/null +++ b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/featureset.go @@ -0,0 +1,106 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nodedeclaredfeatures + +import ( + "fmt" + "slices" +) + +// FeatureSet is a set of node features. +type FeatureSet = bitmap + +// FeatureMapper maps feature names to bit positions in a FeatureSet. +type FeatureMapper struct { + registeredFeatures []string +} + +// NewFeatureMapper creates a FeatureMapper from a list of known features. +func NewFeatureMapper(features []string) *FeatureMapper { + sortedFeatures := slices.Clone(features) + slices.Sort(sortedFeatures) + return &FeatureMapper{sortedFeatures} +} + +// NewFeatureSet returns an empty FeatureSet sized to the registered features. +func (m *FeatureMapper) NewFeatureSet() FeatureSet { + return FeatureSet(newBitmap(len(m.registeredFeatures))) +} + +// MapSorted creates a FeatureSet from a sorted slice of feature names. +func (m *FeatureMapper) MapSorted(sortedFeatures []string) (FeatureSet, error) { + return m.mapSorted(sortedFeatures, false) +} + +// MustMapSorted is a convenience wrapper around MapSorted that panics on errors. +func (m *FeatureMapper) MustMapSorted(sortedFeatures []string) FeatureSet { + s, err := m.MapSorted(sortedFeatures) + if err != nil { + panic(err) + } + return s +} + +// TryMap creates a FeatureSet from a sorted slice, ignoring unknown features. +func (m *FeatureMapper) TryMap(sortedFeatures []string) FeatureSet { + fs, _ := m.mapSorted(sortedFeatures, true) + return fs +} + +func (m *FeatureMapper) mapSorted(sortedFeatures []string, ignoreUnknown bool) (FeatureSet, error) { + s := m.NewFeatureSet() + + if len(sortedFeatures) == 0 { + return s, nil + } + + i, j := 0, 0 + for i < len(sortedFeatures) && j < len(m.registeredFeatures) { + if sortedFeatures[i] == m.registeredFeatures[j] { + s.Set(j) + i++ + j++ + } else if sortedFeatures[i] < m.registeredFeatures[j] { + if !ignoreUnknown { + return s, fmt.Errorf("unknown feature %s", sortedFeatures[i]) + } + i++ + } else { + j++ + } + } + + if !ignoreUnknown && i < len(sortedFeatures) { + return s, fmt.Errorf("unknown feature %s", sortedFeatures[i]) + } + return s, nil +} + +// Unmap returns the names of the features set in the FeatureSet (sorted). +func (m *FeatureMapper) Unmap(s FeatureSet) []string { + if s.IsEmpty() { + return nil + } + + var keys []string + for i, k := range m.registeredFeatures { + if s.Get(i) { + keys = append(keys, k) + } + } + return keys +} diff --git a/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/featureset_test.go b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/featureset_test.go new file mode 100644 index 00000000000..b82d9e68757 --- /dev/null +++ b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/featureset_test.go @@ -0,0 +1,141 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nodedeclaredfeatures + +import ( + "slices" + "testing" + + "k8s.io/apimachinery/pkg/util/sets" +) + +func TestFeatureMapper(t *testing.T) { + features := []string{"a", "b", "c", "d"} + mapper := NewFeatureMapper(features) + + tests := []struct { + name string + input []string + expectError bool + expectedSet sets.Set[int] // indices in features + }{ + { + name: "Empty input", + input: nil, + expectedSet: nil, + }, + { + name: "Single valid feature", + input: []string{"a"}, + expectedSet: sets.New(0), + }, + { + name: "Multiple valid features", + input: []string{"a", "c"}, + expectedSet: sets.New(0, 2), + }, + { + name: "All features", + input: []string{"a", "b", "c", "d"}, + expectedSet: sets.New(0, 1, 2, 3), + }, + { + name: "Unknown feature (start)", + input: []string{"0", "a"}, + expectError: true, + expectedSet: sets.New(0), + }, + { + name: "Unknown feature (middle)", + input: []string{"a", "ab", "b"}, + expectError: true, + expectedSet: sets.New(0, 1), + }, + { + name: "Unknown feature (end)", + input: []string{"d", "e"}, + expectError: true, + expectedSet: sets.New(3), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test TryMap + fs := mapper.TryMap(tt.input) + for i, known := range features { + expected := tt.expectedSet.Has(i) + got := fs.Get(i) + if got != expected { + t.Errorf("Feature %s (index %d) state mismatch: got %v, want %v", known, i, got, expected) + } + } + + // Test MapSorted + fs2, err := mapper.MapSorted(tt.input) + if tt.expectError { + if err == nil { + t.Error("expected error from MapSorted, got nil") + } + } else { + if err != nil { + t.Fatalf("unexpected error from MapSorted: %v", err) + } + if !fs.Equal(fs2) { + t.Errorf("fs != fs2: got %v, want %v", fs2, fs) + } + } + + // Test MustMapSorted + if tt.expectError { + func() { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic from MustMapSorted, but did not panic") + } + }() + mapper.MustMapSorted(tt.input) + }() + } else { + got := mapper.MustMapSorted(tt.input) + if !fs.Equal(got) { + t.Errorf("MustMapSorted result mismatch: got %v, want %v", got, fs) + } + } + + // Test Unmap + unmapped := mapper.Unmap(fs) + if len(unmapped) != tt.expectedSet.Len() { + t.Errorf("len(unmapped) = %d, want %d", len(unmapped), tt.expectedSet.Len()) + } + if tt.expectError { + // If there's an error, just verify that the unmapped results of TryMap are + // contained within the input. + inputSets := sets.New(tt.input...) + for _, u := range unmapped { + if !inputSets.Has(u) { + t.Errorf("unmapped result %q not found in input %v", u, tt.input) + } + } + } else { + if !slices.Equal(tt.input, unmapped) { + t.Errorf("Unmap result mismatch: got %v, want %v", unmapped, tt.input) + } + } + }) + } +} diff --git a/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/framework.go b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/framework.go index 1a223e2cafc..a2c0e50f6be 100644 --- a/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/framework.go +++ b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/framework.go @@ -18,9 +18,10 @@ package nodedeclaredfeatures import ( "fmt" + "slices" + "strings" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/version" "k8s.io/component-helpers/nodedeclaredfeatures/features" "k8s.io/component-helpers/nodedeclaredfeatures/types" @@ -29,38 +30,25 @@ import ( // Framework provides functions for discovering node features and inferring pod feature requirements. // It is stateful and holds the feature registry. type Framework struct { + *FeatureMapper registry []types.Feature } -// FeatureSet is a set of node features. -type FeatureSet struct { - sets.Set[string] -} - -// NewFeatureSet creates a FeatureSet from a list of feature names. -func NewFeatureSet(features ...string) FeatureSet { - return FeatureSet{Set: sets.New(features...)} -} - -// Equal returns true if both the sets have the same features. -func (s *FeatureSet) Equal(other FeatureSet) bool { - return s.Set.Equal(other.Set) -} - -// Clone returns a copy of the FeatureSet. -func (s *FeatureSet) Clone() FeatureSet { - if s.Set == nil { - return FeatureSet{Set: nil} - } - return FeatureSet{Set: s.Set.Clone()} -} - var DefaultFramework = New(features.AllFeatures) // New creates a new instance of the Framework. func New(registry []types.Feature) *Framework { + // Ensure the features are sorted. + slices.SortFunc(registry, func(a, b types.Feature) int { + return strings.Compare(a.Name(), b.Name()) + }) + featureNames := make([]string, len(registry)) + for i, f := range registry { + featureNames[i] = f.Name() + } return &Framework{ - registry: registry, + registry: registry, + FeatureMapper: NewFeatureMapper(featureNames), } } @@ -84,14 +72,14 @@ func (f *Framework) InferForPodScheduling(podInfo *types.PodInfo, targetVersion if targetVersion == nil { return FeatureSet{}, fmt.Errorf("target version cannot be nil") } - reqs := NewFeatureSet() - for _, f := range f.registry { + reqs := f.NewFeatureSet() + for i, f := range f.registry { if f.MaxVersion() != nil && targetVersion.GreaterThan(f.MaxVersion()) { // If target version is greater than the feature's max version, no need to require the feature continue } if f.InferForScheduling(podInfo) { - reqs.Insert(f.Name()) + reqs.Set(i) } } return reqs, nil @@ -102,14 +90,14 @@ func (f *Framework) InferForPodUpdate(oldPodInfo, newPodInfo *types.PodInfo, tar if targetVersion == nil { return FeatureSet{}, fmt.Errorf("target version cannot be nil") } - reqs := NewFeatureSet() - for _, f := range f.registry { + reqs := f.NewFeatureSet() + for i, f := range f.registry { if f.MaxVersion() != nil && targetVersion.GreaterThan(f.MaxVersion()) { // If target version is greater than the feature's max version, no need to require the feature continue } if f.InferForUpdate(oldPodInfo, newPodInfo) { - reqs.Insert(f.Name()) + reqs.Set(i) } } return reqs, nil @@ -128,32 +116,34 @@ type MatchResult struct { // It returns a MatchResult: // - IsMatch is true if all requiredFeatures are present in node.status.declaredFeatures. // - UnsatisfiedRequirements lists features in requiredFeatures but not in node.status.declaredFeatures. -func MatchNode(requiredFeatures FeatureSet, node *v1.Node) (*MatchResult, error) { +func (f *Framework) MatchNode(requiredFeatures FeatureSet, node *v1.Node) (*MatchResult, error) { if node == nil { return nil, fmt.Errorf("node cannot be nil") } - return MatchNodeFeatureSet(requiredFeatures, NewFeatureSet(node.Status.DeclaredFeatures...)) + fs, err := f.MapSorted(node.Status.DeclaredFeatures) + if err != nil { + return nil, err + } + return f.MatchNodeFeatureSet(requiredFeatures, fs) } // MatchNodeFeatureSet compares a set of required features against a set of features present on a node. // It returns a MatchResult: // - IsMatch is true if all requiredFeatures are present in nodeFeatures. // - UnsatisfiedRequirements lists features in requiredFeatures but not in nodeFeatures. -func MatchNodeFeatureSet(requiredFeatures FeatureSet, nodeFeatures FeatureSet) (*MatchResult, error) { - if requiredFeatures.Len() == 0 { +func (f *Framework) MatchNodeFeatureSet(requiredFeatures FeatureSet, nodeFeatures FeatureSet) (*MatchResult, error) { + if requiredFeatures.IsEmpty() { return &MatchResult{IsMatch: true}, nil // No requirements to match. } - var mismatched []string - for req := range requiredFeatures.Set { - if !nodeFeatures.Has(req) { - mismatched = append(mismatched, req) - continue - } + diff, err := requiredFeatures.Difference(nodeFeatures) + if err != nil { + return nil, err } - if len(mismatched) > 0 { - return &MatchResult{IsMatch: false, UnsatisfiedRequirements: mismatched}, nil + if diff.IsEmpty() { + return &MatchResult{IsMatch: true}, nil } - return &MatchResult{IsMatch: true}, nil + unsatisfiedRequirements := f.Unmap(diff) + return &MatchResult{IsMatch: false, UnsatisfiedRequirements: unsatisfiedRequirements}, nil } // GetFeatureRequirements returns the feature gates that a feature depends on. diff --git a/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/framework_test.go b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/framework_test.go index 9a14bb0d89c..9ba0c6dabd2 100644 --- a/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/framework_test.go +++ b/staging/src/k8s.io/component-helpers/nodedeclaredfeatures/framework_test.go @@ -40,13 +40,24 @@ type mockFeature struct { requirements func() *FeatureRequirements } -func (f *mockFeature) Name() string { return f.name } -func (f *mockFeature) Discover(cfg *types.NodeConfiguration) bool { return f.discover(cfg) } +func (f *mockFeature) Name() string { return f.name } +func (f *mockFeature) Discover(cfg *types.NodeConfiguration) bool { + if f.discover != nil { + return f.discover(cfg) + } + return true +} func (f *mockFeature) InferForScheduling(podInfo *types.PodInfo) bool { - return f.inferForScheduling(podInfo) + if f.inferForScheduling != nil { + return f.inferForScheduling(podInfo) + } + return false } func (f *mockFeature) InferForUpdate(oldPodInfo, newPodInfo *types.PodInfo) bool { - return f.inferForUpdate(oldPodInfo, newPodInfo) + if f.inferForUpdate != nil { + return f.inferForUpdate(oldPodInfo, newPodInfo) + } + return false } func (f *mockFeature) MaxVersion() *version.Version { return f.maxVersion } func (f *mockFeature) Requirements() *FeatureRequirements { return f.requirements() } @@ -66,6 +77,17 @@ func newMockFeatureGate(features map[string]bool) *mockFeatureGate { return &mockFeatureGate{features: features} } +func newTestFramework(features ...string) *Framework { + slices.Sort(features) + + allFeatures := make([]Feature, len(features)) + for i, name := range features { + allFeatures[i] = &mockFeature{name: name} + } + + return New(allFeatures) +} + func TestDiscoverNodeFeatures(t *testing.T) { featureMaxVersion := version.MustParse("1.38.0") registry := []types.Feature{ @@ -188,7 +210,7 @@ func TestInferForPodScheduling(t *testing.T) { registry []types.Feature newPod *v1.Pod targetVersion *version.Version - expectedReqs FeatureSet + expectedReqs []string expectErr bool errContains string }{ @@ -203,7 +225,7 @@ func TestInferForPodScheduling(t *testing.T) { }, newPod: podWithPodLevelResources, targetVersion: version.MustParse("1.30.0"), - expectedReqs: NewFeatureSet("PodLevelResources"), + expectedReqs: []string{"PodLevelResources"}, expectErr: false, }, { @@ -217,7 +239,7 @@ func TestInferForPodScheduling(t *testing.T) { }, newPod: podWithoutPodLevelResources, targetVersion: version.MustParse("1.30.0"), - expectedReqs: NewFeatureSet(), + expectedReqs: nil, expectErr: false, }, { @@ -232,7 +254,7 @@ func TestInferForPodScheduling(t *testing.T) { }, newPod: podWithPodLevelResources, targetVersion: version.MustParse("1.31.0"), - expectedReqs: NewFeatureSet(), + expectedReqs: nil, expectErr: false, }, { @@ -247,7 +269,21 @@ func TestInferForPodScheduling(t *testing.T) { }, newPod: podWithPodLevelResources, targetVersion: version.MustParse("0.0.0-alpha.2.39+049eafd34dfbd2"), - expectedReqs: NewFeatureSet("PodLevelResources"), + expectedReqs: []string{"PodLevelResources"}, + expectErr: false, + }, + { + name: "exceeds max version", + registry: []types.Feature{ + &mockFeature{ + name: "PodLevelResources", + inferForScheduling: inferPodlevelResources, + maxVersion: version.MustParse("1.30.0"), + }, + }, + newPod: podWithPodLevelResources, + targetVersion: version.MustParse("1.40.0"), + expectedReqs: nil, expectErr: false, }, { @@ -261,7 +297,7 @@ func TestInferForPodScheduling(t *testing.T) { }, newPod: podWithPodLevelResources, targetVersion: nil, - expectedReqs: NewFeatureSet(), + expectedReqs: nil, expectErr: true, errContains: "target version cannot be nil", }, @@ -280,9 +316,11 @@ func TestInferForPodScheduling(t *testing.T) { } } else { if err != nil { - t.Errorf("unexpected error %v", err) - } else if !reflect.DeepEqual(tc.expectedReqs, reqs) { - t.Errorf("expected %#v, got %#v", tc.expectedReqs, reqs) + t.Fatalf("unexpected error %v", err) + } + unmappedReqs := framework.Unmap(reqs) + if !reflect.DeepEqual(tc.expectedReqs, unmappedReqs) { + t.Errorf("expected %#v, got %#v", tc.expectedReqs, unmappedReqs) } } }) @@ -331,7 +369,7 @@ func TestInferForPodUpdate(t *testing.T) { oldPod *v1.Pod newPod *v1.Pod targetVersion *version.Version - expectedReqs FeatureSet + expectedReqs []string expectErr bool errContains string }{ @@ -347,7 +385,7 @@ func TestInferForPodUpdate(t *testing.T) { oldPod: podWith1CPU, newPod: podWith2CPU, targetVersion: version.MustParse("1.30.0"), - expectedReqs: NewFeatureSet("InPlacePodResize"), + expectedReqs: []string{"InPlacePodResize"}, expectErr: false, }, { @@ -362,7 +400,7 @@ func TestInferForPodUpdate(t *testing.T) { oldPod: podWith1CPU, newPod: podWith2CPU, targetVersion: version.MustParse("1.35.0-alpha.2.39+049eafd34dfbd2"), - expectedReqs: NewFeatureSet("InPlacePodResize"), + expectedReqs: []string{"InPlacePodResize"}, expectErr: false, }, { @@ -377,7 +415,7 @@ func TestInferForPodUpdate(t *testing.T) { oldPod: podWith1CPU, newPod: podWith1CPU, targetVersion: version.MustParse("1.30.0"), - expectedReqs: NewFeatureSet(), + expectedReqs: nil, expectErr: false, }, { @@ -393,7 +431,7 @@ func TestInferForPodUpdate(t *testing.T) { oldPod: podWith1CPU, newPod: podWith2CPU, targetVersion: version.MustParse("1.31.0"), - expectedReqs: NewFeatureSet(), + expectedReqs: nil, expectErr: false, }, { @@ -408,7 +446,7 @@ func TestInferForPodUpdate(t *testing.T) { oldPod: podWith1CPU, newPod: podWith2CPU, targetVersion: nil, - expectedReqs: NewFeatureSet(), + expectedReqs: nil, expectErr: true, errContains: "target version cannot be nil", }, @@ -427,9 +465,11 @@ func TestInferForPodUpdate(t *testing.T) { } } else { if err != nil { - t.Errorf("unexpected error %v", err) - } else if !reflect.DeepEqual(tc.expectedReqs, reqs) { - t.Errorf("expected %#v, got %#v", tc.expectedReqs, reqs) + t.Fatalf("unexpected error %v", err) + } + unmappedReqs := framework.Unmap(reqs) + if !reflect.DeepEqual(tc.expectedReqs, unmappedReqs) { + t.Errorf("expected %#v, got %#v", tc.expectedReqs, unmappedReqs) } } }) @@ -437,44 +477,45 @@ func TestInferForPodUpdate(t *testing.T) { } func TestMatchNode(t *testing.T) { + framework := newTestFramework("feature-a", "feature-b", "feature-c") testCases := []struct { name string - podFeatureRequirements FeatureSet + podFeatureRequirements []string nodeFeatures []string expectedMatch bool expectedUnsatisfied []string }{ { name: "all features match", - podFeatureRequirements: NewFeatureSet("feature-a", "feature-b"), + podFeatureRequirements: []string{"feature-a", "feature-b"}, nodeFeatures: []string{"feature-a", "feature-b", "feature-c"}, expectedMatch: true, expectedUnsatisfied: nil, }, { name: "some features missing", - podFeatureRequirements: NewFeatureSet("feature-a", "feature-b"), + podFeatureRequirements: []string{"feature-a", "feature-b"}, nodeFeatures: []string{"feature-a", "feature-c"}, expectedMatch: false, expectedUnsatisfied: []string{"feature-b"}, }, { name: "all features missing", - podFeatureRequirements: NewFeatureSet("feature-a", "feature-b"), + podFeatureRequirements: []string{"feature-a", "feature-b"}, nodeFeatures: []string{"feature-c"}, expectedMatch: false, expectedUnsatisfied: []string{"feature-a", "feature-b"}, }, { name: "no node features", - podFeatureRequirements: NewFeatureSet("feature-a", "feature-b"), + podFeatureRequirements: []string{"feature-a", "feature-b"}, nodeFeatures: []string{}, expectedMatch: false, expectedUnsatisfied: []string{"feature-a", "feature-b"}, }, { name: "no requirements", - podFeatureRequirements: NewFeatureSet(), + podFeatureRequirements: []string{}, nodeFeatures: []string{"feature-a", "feature-b", "feature-c"}, expectedMatch: true, expectedUnsatisfied: nil, @@ -492,9 +533,9 @@ func TestMatchNode(t *testing.T) { switch variationName { case "MatchNode": node := &v1.Node{Status: v1.NodeStatus{DeclaredFeatures: tc.nodeFeatures}} - result, err = MatchNode(tc.podFeatureRequirements, node) + result, err = framework.MatchNode(framework.MustMapSorted(tc.podFeatureRequirements), node) case "MatchNodeFeatureSet": - result, err = MatchNodeFeatureSet(tc.podFeatureRequirements, NewFeatureSet(tc.nodeFeatures...)) + result, err = framework.MatchNodeFeatureSet(framework.MustMapSorted(tc.podFeatureRequirements), framework.MustMapSorted(tc.nodeFeatures)) default: t.Fatalf("unknown match variation: %s", variationName) } @@ -518,7 +559,7 @@ func TestMatchNode(t *testing.T) { } // Test nil node - _, err := MatchNode(NewFeatureSet("feature-a"), nil) + _, err := framework.MatchNode(framework.MustMapSorted([]string{"feature-a"}), nil) if err == nil { t.Fatalf("MatchNode should return an error for a nil node") }