Skip to content

Commit

Permalink
refine(fmt): use gofumpt to refine shared_cores_numa_binding codes
Browse files Browse the repository at this point in the history
  • Loading branch information
csfldf committed Jun 6, 2024
1 parent 9ff003e commit e3847e8
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ import (
)

func (p *DynamicPolicy) sharedCoresAllocationHandler(ctx context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceAllocationResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceAllocationResponse, error) {
if req == nil {
return nil, fmt.Errorf("sharedCoresAllocationHandler got nil req")
}
Expand All @@ -54,7 +55,8 @@ func (p *DynamicPolicy) sharedCoresAllocationHandler(ctx context.Context,
}

func (p *DynamicPolicy) sharedCoresWithoutNUMABindingAllocationHandler(_ context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceAllocationResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceAllocationResponse, error) {
if req == nil {
return nil, fmt.Errorf("sharedCoresAllocationHandler got nil request")
}
Expand Down Expand Up @@ -130,7 +132,6 @@ func (p *DynamicPolicy) sharedCoresWithoutNUMABindingAllocationHandler(_ context
} else {
p.state.SetAllocationInfo(allocationInfo.PodUid, allocationInfo.ContainerName, allocationInfo)
_, err = p.doAndCheckPutAllocationInfo(allocationInfo, false)

if err != nil {
return nil, err
}
Expand All @@ -148,7 +149,6 @@ func (p *DynamicPolicy) sharedCoresWithoutNUMABindingAllocationHandler(_ context
allocationInfo.OriginalTopologyAwareAssignments = machine.DeepcopyCPUAssignment(pooledCPUsTopologyAwareAssignments)
} else {
_, err := p.doAndCheckPutAllocationInfo(allocationInfo, true)

if err != nil {
return nil, err
}
Expand Down Expand Up @@ -182,7 +182,8 @@ func (p *DynamicPolicy) sharedCoresWithoutNUMABindingAllocationHandler(_ context
}

func (p *DynamicPolicy) reclaimedCoresAllocationHandler(_ context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceAllocationResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceAllocationResponse, error) {
if req == nil {
return nil, fmt.Errorf("reclaimedCoresAllocationHandler got nil request")
}
Expand Down Expand Up @@ -268,8 +269,8 @@ func (p *DynamicPolicy) reclaimedCoresAllocationHandler(_ context.Context,
}

func (p *DynamicPolicy) dedicatedCoresAllocationHandler(ctx context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceAllocationResponse, error) {

req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceAllocationResponse, error) {
if req == nil {
return nil, fmt.Errorf("dedicatedCoresAllocationHandler got nil req")
}
Expand All @@ -283,13 +284,15 @@ func (p *DynamicPolicy) dedicatedCoresAllocationHandler(ctx context.Context,
}

func (p *DynamicPolicy) dedicatedCoresWithoutNUMABindingAllocationHandler(_ context.Context,
_ *pluginapi.ResourceRequest) (*pluginapi.ResourceAllocationResponse, error) {
_ *pluginapi.ResourceRequest,
) (*pluginapi.ResourceAllocationResponse, error) {
// todo: support dedicated_cores without NUMA binding
return nil, fmt.Errorf("not support dedicated_cores without NUMA binding")
}

func (p *DynamicPolicy) dedicatedCoresWithNUMABindingAllocationHandler(ctx context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceAllocationResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceAllocationResponse, error) {
if req.ContainerType == pluginapi.ContainerType_SIDECAR {
return p.allocationSidecarHandler(ctx, req, apiconsts.PodAnnotationQoSLevelDedicatedCores)
}
Expand Down Expand Up @@ -400,7 +403,8 @@ func (p *DynamicPolicy) dedicatedCoresWithNUMABindingAllocationHandler(ctx conte

// allocationSidecarHandler currently we set cpuset of sidecar to the cpuset of its main container
func (p *DynamicPolicy) allocationSidecarHandler(_ context.Context,
req *pluginapi.ResourceRequest, qosLevel string) (*pluginapi.ResourceAllocationResponse, error) {
req *pluginapi.ResourceRequest, qosLevel string,
) (*pluginapi.ResourceAllocationResponse, error) {
_, reqFloat64, err := util.GetQuantityFromResourceReq(req)
if err != nil {
return nil, fmt.Errorf("getReqQuantityFromResourceReq failed with error: %v", err)
Expand Down Expand Up @@ -466,7 +470,8 @@ func (p *DynamicPolicy) allocationSidecarHandler(_ context.Context,
}

func (p *DynamicPolicy) sharedCoresWithNUMABindingAllocationHandler(ctx context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceAllocationResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceAllocationResponse, error) {
if req.ContainerType == pluginapi.ContainerType_SIDECAR {
return p.allocationSidecarHandler(ctx, req, apiconsts.PodAnnotationQoSLevelSharedCores)
}
Expand Down Expand Up @@ -503,7 +508,8 @@ func (p *DynamicPolicy) sharedCoresWithNUMABindingAllocationHandler(ctx context.
}

func (p *DynamicPolicy) allocateNumaBindingCPUs(numCPUs int, hint *pluginapi.TopologyHint,
machineState state.NUMANodeMap, reqAnnotations map[string]string) (machine.CPUSet, error) {
machineState state.NUMANodeMap, reqAnnotations map[string]string,
) (machine.CPUSet, error) {
if hint == nil {
return machine.NewCPUSet(), fmt.Errorf("hint is nil")
} else if len(hint.Nodes) == 0 {
Expand All @@ -529,7 +535,6 @@ func (p *DynamicPolicy) allocateNumaBindingCPUs(numCPUs int, hint *pluginapi.Top
} else {
var err error
alignedCPUs, err = calculator.TakeByTopology(p.machineInfo, alignedAvailableCPUs, numCPUs)

if err != nil {
general.ErrorS(err, "take cpu for NUMA not exclusive binding container failed",
"hints", hint.Nodes,
Expand Down Expand Up @@ -559,7 +564,8 @@ func (p *DynamicPolicy) allocateNumaBindingCPUs(numCPUs int, hint *pluginapi.Top
}

func (p *DynamicPolicy) allocateSharedNumaBindingCPUs(req *pluginapi.ResourceRequest,
hint *pluginapi.TopologyHint) (*state.AllocationInfo, error) {
hint *pluginapi.TopologyHint,
) (*state.AllocationInfo, error) {
if req == nil {
return nil, fmt.Errorf("nil req")
} else if hint == nil {
Expand Down Expand Up @@ -603,7 +609,6 @@ func (p *DynamicPolicy) allocateSharedNumaBindingCPUs(req *pluginapi.ResourceReq

p.state.SetAllocationInfo(allocationInfo.PodUid, allocationInfo.ContainerName, allocationInfo)
checkedAllocationInfo, err := p.doAndCheckPutAllocationInfo(allocationInfo, true)

if err != nil {
return nil, fmt.Errorf("doAndCheckPutAllocationInfo failed with error: %v", err)
}
Expand Down Expand Up @@ -710,7 +715,8 @@ func (p *DynamicPolicy) adjustAllocationEntries() error {
// 3. apply them to local state
// 4. clean pools
func (p *DynamicPolicy) adjustPoolsAndIsolatedEntries(poolsQuantityMap map[string]map[int]int,
isolatedQuantityMap map[string]map[string]int, entries state.PodEntries, machineState state.NUMANodeMap) error {
isolatedQuantityMap map[string]map[string]int, entries state.PodEntries, machineState state.NUMANodeMap,
) error {
availableCPUs := machineState.GetFilteredAvailableCPUSet(p.reservedCPUs, nil, state.CheckDedicatedNUMABinding)

poolsCPUSet, isolatedCPUSet, err := p.generatePoolsAndIsolation(poolsQuantityMap, isolatedQuantityMap, availableCPUs)
Expand Down Expand Up @@ -785,8 +791,8 @@ func (p *DynamicPolicy) reclaimOverlapNUMABinding(poolsCPUSet map[string]machine
// 3. construct entries for shared_cores, reclaimed_cores, numa_binding dedicated_cores containers
func (p *DynamicPolicy) applyPoolsAndIsolatedInfo(poolsCPUSet map[string]machine.CPUSet,
isolatedCPUSet map[string]map[string]machine.CPUSet, curEntries state.PodEntries,
machineState state.NUMANodeMap, sharedBindingNUMAs sets.Int) error {

machineState state.NUMANodeMap, sharedBindingNUMAs sets.Int,
) error {
newPodEntries := make(state.PodEntries)
unionDedicatedIsolatedCPUSet := machine.NewCPUSet()

Expand Down Expand Up @@ -944,7 +950,6 @@ func (p *DynamicPolicy) applyPoolsAndIsolatedInfo(poolsCPUSet map[string]machine
// it's because we reply on GetSpecifiedPoolName (in GetPoolName) when calling CheckNUMABindingSharedCoresAntiAffinity,
// At that time, NUMA hint for the candicate container isn't confirmed, so we can't implement NUMA hint aware logic in GetSpecifiedPoolName.
ownerPoolName, err = allocationInfo.GetSpecifiedNUMABindingPoolName()

if err != nil {
return fmt.Errorf("pod: %s/%s, container: %s is shared_cores with numa_binding, "+
"GetSpecifiedNUMABindingPoolName failed with error: %v",
Expand Down Expand Up @@ -1016,8 +1021,8 @@ func (p *DynamicPolicy) applyPoolsAndIsolatedInfo(poolsCPUSet map[string]machine
}

func (p *DynamicPolicy) generateNUMABindingPoolsCPUSetInPlace(poolsCPUSet map[string]machine.CPUSet,
poolsQuantityMap map[string]map[int]int, availableCPUs machine.CPUSet) (machine.CPUSet, error) {

poolsQuantityMap map[string]map[int]int, availableCPUs machine.CPUSet,
) (machine.CPUSet, error) {
numaToPoolQuantityMap := make(map[int]map[string]int)
originalAvailableCPUSet := availableCPUs.Clone()
enableReclaim := p.dynamicConfig.GetDynamicConfiguration().EnableReclaim
Expand Down Expand Up @@ -1077,8 +1082,8 @@ func (p *DynamicPolicy) generateNUMABindingPoolsCPUSetInPlace(poolsCPUSet map[st
// 3. apportion to other pools if reclaimed is disabled
func (p *DynamicPolicy) generatePoolsAndIsolation(poolsQuantityMap map[string]map[int]int,
isolatedQuantityMap map[string]map[string]int, availableCPUs machine.CPUSet) (poolsCPUSet map[string]machine.CPUSet,
isolatedCPUSet map[string]map[string]machine.CPUSet, err error) {

isolatedCPUSet map[string]map[string]machine.CPUSet, err error,
) {
poolsBindingNUMAs := sets.NewInt()
poolsToSkip := make([]string, 0, len(poolsQuantityMap))
nonBindingPoolsQuantityMap := make(map[string]int)
Expand Down Expand Up @@ -1221,7 +1226,8 @@ func (p *DynamicPolicy) generatePoolsAndIsolation(poolsQuantityMap map[string]ma
}

func (p *DynamicPolicy) generateProportionalPoolsCPUSetInPlace(poolsQuantityMap map[string]int,
poolsCPUSet map[string]machine.CPUSet, availableCPUs machine.CPUSet) (machine.CPUSet, error) {
poolsCPUSet map[string]machine.CPUSet, availableCPUs machine.CPUSet,
) (machine.CPUSet, error) {
availableSize := availableCPUs.Size()

proportionalPoolsQuantityMap, totalProportionalPoolsQuantity := getProportionalPoolsQuantityMap(poolsQuantityMap, availableSize)
Expand All @@ -1243,7 +1249,6 @@ func (p *DynamicPolicy) generateProportionalPoolsCPUSetInPlace(poolsQuantityMap
} else {
var err error
availableCPUs, err = p.takeCPUsForPoolsInPlace(proportionalPoolsQuantityMap, poolsCPUSet, availableCPUs)

if err != nil {
return availableCPUs, err
}
Expand Down Expand Up @@ -1353,8 +1358,8 @@ func (p *DynamicPolicy) apportionReclaimedPool(poolsCPUSet map[string]machine.CP

func (p *DynamicPolicy) takeCPUsForPoolsInPlace(poolsQuantityMap map[string]int,
poolsCPUSet map[string]machine.CPUSet,
availableCPUs machine.CPUSet) (machine.CPUSet, error) {

availableCPUs machine.CPUSet,
) (machine.CPUSet, error) {
originalAvailableCPUSet := availableCPUs.Clone()
var poolsCPUSetToAdd map[string]machine.CPUSet
var tErr error
Expand All @@ -1378,7 +1383,8 @@ func (p *DynamicPolicy) takeCPUsForPoolsInPlace(poolsQuantityMap map[string]int,
// and it will consider the total available cpuset during calculation.
// the returned value includes cpuset pool map and remaining available cpuset.
func (p *DynamicPolicy) takeCPUsForPools(poolsQuantityMap map[string]int,
availableCPUs machine.CPUSet) (map[string]machine.CPUSet, machine.CPUSet, error) {
availableCPUs machine.CPUSet,
) (map[string]machine.CPUSet, machine.CPUSet, error) {
poolsCPUSet := make(map[string]machine.CPUSet)
clonedAvailableCPUs := availableCPUs.Clone()

Expand All @@ -1404,7 +1410,8 @@ func (p *DynamicPolicy) takeCPUsForPools(poolsQuantityMap map[string]int,
// and it will consider the total available cpuset during calculation.
// the returned value includes cpuset map for pod/container combinations and remaining available cpuset.
func (p *DynamicPolicy) takeCPUsForContainers(containersQuantityMap map[string]map[string]int,
availableCPUs machine.CPUSet) (map[string]map[string]machine.CPUSet, machine.CPUSet, error) {
availableCPUs machine.CPUSet,
) (map[string]map[string]machine.CPUSet, machine.CPUSet, error) {
containersCPUSet := make(map[string]map[string]machine.CPUSet)
clonedAvailableCPUs := availableCPUs.Clone()

Expand Down
33 changes: 20 additions & 13 deletions pkg/agent/qrm-plugins/cpu/dynamicpolicy/policy_hint_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ import (
)

func (p *DynamicPolicy) sharedCoresHintHandler(ctx context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceHintsResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceHintsResponse, error) {
if req == nil {
return nil, fmt.Errorf("got nil request")
}
Expand All @@ -53,12 +54,14 @@ func (p *DynamicPolicy) sharedCoresHintHandler(ctx context.Context,
}

func (p *DynamicPolicy) reclaimedCoresHintHandler(ctx context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceHintsResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceHintsResponse, error) {
return p.sharedCoresHintHandler(ctx, req)
}

func (p *DynamicPolicy) dedicatedCoresHintHandler(ctx context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceHintsResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceHintsResponse, error) {
if req == nil {
return nil, fmt.Errorf("dedicatedCoresHintHandler got nil req")
}
Expand All @@ -72,7 +75,8 @@ func (p *DynamicPolicy) dedicatedCoresHintHandler(ctx context.Context,
}

func (p *DynamicPolicy) dedicatedCoresWithNUMABindingHintHandler(_ context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceHintsResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceHintsResponse, error) {
// currently, we set cpuset of sidecar to the cpuset of its main container,
// so there is no numa preference here.
if req.ContainerType == pluginapi.ContainerType_SIDECAR {
Expand Down Expand Up @@ -138,15 +142,17 @@ func (p *DynamicPolicy) dedicatedCoresWithNUMABindingHintHandler(_ context.Conte
}

func (p *DynamicPolicy) dedicatedCoresWithoutNUMABindingHintHandler(_ context.Context,
_ *pluginapi.ResourceRequest) (*pluginapi.ResourceHintsResponse, error) {
_ *pluginapi.ResourceRequest,
) (*pluginapi.ResourceHintsResponse, error) {
// todo: support dedicated_cores without NUMA binding
return nil, fmt.Errorf("not support dedicated_cores without NUMA binding")
}

// calculateHints is a helper function to calculate the topology hints
// with the given container requests.
func (p *DynamicPolicy) calculateHints(reqInt int, machineState state.NUMANodeMap,
reqAnnotations map[string]string) (map[string]*pluginapi.ListOfTopologyHints, error) {
reqAnnotations map[string]string,
) (map[string]*pluginapi.ListOfTopologyHints, error) {
numaNodes := make([]int, 0, len(machineState))
for numaNode := range machineState {
numaNodes = append(numaNodes, numaNode)
Expand Down Expand Up @@ -232,7 +238,8 @@ func (p *DynamicPolicy) calculateHints(reqInt int, machineState state.NUMANodeMa
}

func (p *DynamicPolicy) sharedCoresWithNUMABindingHintHandler(_ context.Context,
req *pluginapi.ResourceRequest) (*pluginapi.ResourceHintsResponse, error) {
req *pluginapi.ResourceRequest,
) (*pluginapi.ResourceHintsResponse, error) {
// currently, we set cpuset of sidecar to the cpuset of its main container,
// so there is no numa preference here.
if req.ContainerType == pluginapi.ContainerType_SIDECAR {
Expand Down Expand Up @@ -285,8 +292,8 @@ func (p *DynamicPolicy) sharedCoresWithNUMABindingHintHandler(_ context.Context,
}

func (p *DynamicPolicy) populateHintsByPreferPolicy(numaNodes []int, preferPolicy string,
hints map[string]*pluginapi.ListOfTopologyHints, machineState state.NUMANodeMap, reqInt int) {

hints map[string]*pluginapi.ListOfTopologyHints, machineState state.NUMANodeMap, reqInt int,
) {
preferIndexes, maxLeft, minLeft := []int{}, -1, math.MaxInt

for _, nodeID := range numaNodes {
Expand Down Expand Up @@ -329,8 +336,8 @@ func (p *DynamicPolicy) populateHintsByPreferPolicy(numaNodes []int, preferPolic
}

func (p *DynamicPolicy) filterNUMANodesByHintPreferLowThreshold(reqInt int,
machineState state.NUMANodeMap, numaNodes []int) []int {

machineState state.NUMANodeMap, numaNodes []int,
) []int {
filteredNUMANodes := make([]int, 0, len(numaNodes))

for _, nodeID := range numaNodes {
Expand All @@ -351,8 +358,8 @@ func (p *DynamicPolicy) filterNUMANodesByHintPreferLowThreshold(reqInt int,
}

func (p *DynamicPolicy) calculateHintsForNUMABindingSharedCores(reqInt int, machineState state.NUMANodeMap,
reqAnnotations map[string]string) (map[string]*pluginapi.ListOfTopologyHints, error) {

reqAnnotations map[string]string,
) (map[string]*pluginapi.ListOfTopologyHints, error) {
numaNodes := machineState.GetFilteredNUMASetWithAnnotations(
state.CheckNUMABindingSharedCoresAntiAffinity, reqAnnotations).ToSliceInt()

Expand Down
7 changes: 4 additions & 3 deletions pkg/agent/qrm-plugins/cpu/dynamicpolicy/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,8 +578,8 @@ func (ns *NUMANodeState) ExistMatchedAllocationInfo(f func(ai *AllocationInfo) b
// holds true for some pods of this numa else it returns false.
func (ns *NUMANodeState) ExistMatchedAllocationInfoWithAnnotations(
f func(ai *AllocationInfo, annotations map[string]string) bool,
annotations map[string]string) bool {

annotations map[string]string,
) bool {
for _, containerEntries := range ns.PodEntries {
for _, allocationInfo := range containerEntries {
if f(allocationInfo, annotations) {
Expand Down Expand Up @@ -652,7 +652,8 @@ func (nm NUMANodeMap) GetFilteredNUMASet(excludeNUMAPredicate func(ai *Allocatio
// which are excluded by the predicate accepting AllocationInfo in the target NUMA and input annotations of candidate.
func (nm NUMANodeMap) GetFilteredNUMASetWithAnnotations(
excludeNUMAPredicate func(ai *AllocationInfo, annotations map[string]string) bool,
annotations map[string]string) machine.CPUSet {
annotations map[string]string,
) machine.CPUSet {
res := machine.NewCPUSet()
for numaID, numaNodeState := range nm {
if numaNodeState.ExistMatchedAllocationInfoWithAnnotations(excludeNUMAPredicate, annotations) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2543,6 +2543,7 @@ func TestGetSocketTopology(t *testing.T) {
as.Equalf(tc.expectedSocketTopology, actualSocketToplogy, "failed in test case: %s", tc.description)
}
}

func TestAllocationInfo_GetSpecifiedNUMABindingPoolName(t *testing.T) {
t.Parallel()
testName := "test"
Expand Down
5 changes: 2 additions & 3 deletions pkg/agent/qrm-plugins/cpu/dynamicpolicy/state/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ func GetSharedQuantityMapFromPodEntries(podEntries PodEntries, ignoreAllocationI
}

err := CountAllocationInfosToPoolsQuantityMap(allocationInfosToCount, poolsQuantityMap)

if err != nil {
return nil, fmt.Errorf("CountAllocationInfosToPoolsQuantityMap faild with error: %v", err)
}
Expand Down Expand Up @@ -296,8 +295,8 @@ func GetSharedBindingNUMAsFromQuantityMap(poolsQuantityMap map[string]map[int]in
}

func CountAllocationInfosToPoolsQuantityMap(allocationInfos []*AllocationInfo,
poolsQuantityMap map[string]map[int]int) error {

poolsQuantityMap map[string]map[int]int,
) error {
if poolsQuantityMap == nil {
return fmt.Errorf("nil poolsQuantityMap in CountAllocationInfosToPoolsQuantityMap")
}
Expand Down
Loading

0 comments on commit e3847e8

Please sign in to comment.