Add check to skip PodTopologySpread PreFilter if no constraints are specified
This commit adds a check in the PodTopologySpread PreFilter function to return a Skip status if there are no topology spread constraints specified This prevents unnecessary processing and filtering for pods that don't have any topology spread constraints. This change is a part of the work for issue #114399. Signed-off-by: utam0k <k0ma@utam0k.jp>
This commit is contained in:
		@@ -152,7 +152,10 @@ func (pl *PodTopologySpread) PreFilter(ctx context.Context, cycleState *framewor
 | 
				
			|||||||
	s, err := pl.calPreFilterState(ctx, pod)
 | 
						s, err := pl.calPreFilterState(ctx, pod)
 | 
				
			||||||
	if err != nil {
 | 
						if err != nil {
 | 
				
			||||||
		return nil, framework.AsStatus(err)
 | 
							return nil, framework.AsStatus(err)
 | 
				
			||||||
 | 
						} else if s != nil && len(s.Constraints) == 0 {
 | 
				
			||||||
 | 
							return nil, framework.NewStatus(framework.Skip)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	cycleState.Write(preFilterStateKey, s)
 | 
						cycleState.Write(preFilterStateKey, s)
 | 
				
			||||||
	return nil, nil
 | 
						return nil, nil
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
@@ -236,11 +239,8 @@ func getPreFilterState(cycleState *framework.CycleState) (*preFilterState, error
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
// calPreFilterState computes preFilterState describing how pods are spread on topologies.
 | 
					// calPreFilterState computes preFilterState describing how pods are spread on topologies.
 | 
				
			||||||
func (pl *PodTopologySpread) calPreFilterState(ctx context.Context, pod *v1.Pod) (*preFilterState, error) {
 | 
					func (pl *PodTopologySpread) calPreFilterState(ctx context.Context, pod *v1.Pod) (*preFilterState, error) {
 | 
				
			||||||
	allNodes, err := pl.sharedLister.NodeInfos().List()
 | 
					 | 
				
			||||||
	if err != nil {
 | 
					 | 
				
			||||||
		return nil, fmt.Errorf("listing NodeInfos: %w", err)
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
	var constraints []topologySpreadConstraint
 | 
						var constraints []topologySpreadConstraint
 | 
				
			||||||
 | 
						var err error
 | 
				
			||||||
	if len(pod.Spec.TopologySpreadConstraints) > 0 {
 | 
						if len(pod.Spec.TopologySpreadConstraints) > 0 {
 | 
				
			||||||
		// We have feature gating in APIServer to strip the spec
 | 
							// We have feature gating in APIServer to strip the spec
 | 
				
			||||||
		// so don't need to re-check feature gate, just check length of Constraints.
 | 
							// so don't need to re-check feature gate, just check length of Constraints.
 | 
				
			||||||
@@ -262,6 +262,11 @@ func (pl *PodTopologySpread) calPreFilterState(ctx context.Context, pod *v1.Pod)
 | 
				
			|||||||
		return &preFilterState{}, nil
 | 
							return &preFilterState{}, nil
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						allNodes, err := pl.sharedLister.NodeInfos().List()
 | 
				
			||||||
 | 
						if err != nil {
 | 
				
			||||||
 | 
							return nil, fmt.Errorf("listing NodeInfos: %w", err)
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	s := preFilterState{
 | 
						s := preFilterState{
 | 
				
			||||||
		Constraints:          constraints,
 | 
							Constraints:          constraints,
 | 
				
			||||||
		TpKeyToCriticalPaths: make(map[string]*criticalPaths, len(constraints)),
 | 
							TpKeyToCriticalPaths: make(map[string]*criticalPaths, len(constraints)),
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -76,6 +76,7 @@ func TestPreFilterState(t *testing.T) {
 | 
				
			|||||||
		objs                      []runtime.Object
 | 
							objs                      []runtime.Object
 | 
				
			||||||
		defaultConstraints        []v1.TopologySpreadConstraint
 | 
							defaultConstraints        []v1.TopologySpreadConstraint
 | 
				
			||||||
		want                      *preFilterState
 | 
							want                      *preFilterState
 | 
				
			||||||
 | 
							wantPrefilterStatus       *framework.Status
 | 
				
			||||||
		enableMinDomains          bool
 | 
							enableMinDomains          bool
 | 
				
			||||||
		enableNodeInclusionPolicy bool
 | 
							enableNodeInclusionPolicy bool
 | 
				
			||||||
		enableMatchLabelKeys      bool
 | 
							enableMatchLabelKeys      bool
 | 
				
			||||||
@@ -573,7 +574,7 @@ func TestPreFilterState(t *testing.T) {
 | 
				
			|||||||
			objs: []runtime.Object{
 | 
								objs: []runtime.Object{
 | 
				
			||||||
				&v1.Service{Spec: v1.ServiceSpec{Selector: map[string]string{"baz": "kep"}}},
 | 
									&v1.Service{Spec: v1.ServiceSpec{Selector: map[string]string{"baz": "kep"}}},
 | 
				
			||||||
			},
 | 
								},
 | 
				
			||||||
			want: &preFilterState{},
 | 
								wantPrefilterStatus: framework.NewStatus(framework.Skip),
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name: "default constraints and a service, but pod has constraints",
 | 
								name: "default constraints and a service, but pod has constraints",
 | 
				
			||||||
@@ -613,7 +614,7 @@ func TestPreFilterState(t *testing.T) {
 | 
				
			|||||||
			objs: []runtime.Object{
 | 
								objs: []runtime.Object{
 | 
				
			||||||
				&v1.Service{Spec: v1.ServiceSpec{Selector: map[string]string{"foo": "bar"}}},
 | 
									&v1.Service{Spec: v1.ServiceSpec{Selector: map[string]string{"foo": "bar"}}},
 | 
				
			||||||
			},
 | 
								},
 | 
				
			||||||
			want: &preFilterState{},
 | 
								wantPrefilterStatus: framework.NewStatus(framework.Skip),
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name: "TpKeyToDomainsNum is calculated when MinDomains is enabled",
 | 
								name: "TpKeyToDomainsNum is calculated when MinDomains is enabled",
 | 
				
			||||||
@@ -1453,6 +1454,14 @@ func TestPreFilterState(t *testing.T) {
 | 
				
			|||||||
			},
 | 
								},
 | 
				
			||||||
			enableMatchLabelKeys: true,
 | 
								enableMatchLabelKeys: true,
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
 | 
							{
 | 
				
			||||||
 | 
								name: "skip if not specified",
 | 
				
			||||||
 | 
								pod:  st.MakePod().Name("p").Label("foo", "").Obj(),
 | 
				
			||||||
 | 
								nodes: []*v1.Node{
 | 
				
			||||||
 | 
									st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(),
 | 
				
			||||||
 | 
								},
 | 
				
			||||||
 | 
								wantPrefilterStatus: framework.NewStatus(framework.Skip),
 | 
				
			||||||
 | 
							},
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	for _, tt := range tests {
 | 
						for _, tt := range tests {
 | 
				
			||||||
		t.Run(tt.name, func(t *testing.T) {
 | 
							t.Run(tt.name, func(t *testing.T) {
 | 
				
			||||||
@@ -1469,15 +1478,21 @@ func TestPreFilterState(t *testing.T) {
 | 
				
			|||||||
			p.(*PodTopologySpread).enableMatchLabelKeysInPodTopologySpread = tt.enableMatchLabelKeys
 | 
								p.(*PodTopologySpread).enableMatchLabelKeysInPodTopologySpread = tt.enableMatchLabelKeys
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			cs := framework.NewCycleState()
 | 
								cs := framework.NewCycleState()
 | 
				
			||||||
			if _, s := p.(*PodTopologySpread).PreFilter(ctx, cs, tt.pod); !s.IsSuccess() {
 | 
								_, s := p.(*PodTopologySpread).PreFilter(ctx, cs, tt.pod)
 | 
				
			||||||
				t.Fatal(s.AsError())
 | 
								if !tt.wantPrefilterStatus.Equal(s) {
 | 
				
			||||||
 | 
									t.Errorf("PodTopologySpread#PreFilter() returned unexpected status. got: %v, want: %v", s, tt.wantPrefilterStatus)
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
								if !s.IsSuccess() {
 | 
				
			||||||
 | 
									return
 | 
				
			||||||
 | 
								}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			got, err := getPreFilterState(cs)
 | 
								got, err := getPreFilterState(cs)
 | 
				
			||||||
			if err != nil {
 | 
								if err != nil {
 | 
				
			||||||
				t.Fatal(err)
 | 
									t.Fatalf("failed to get PreFilterState from cyclestate: %v", err)
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
			if diff := cmp.Diff(tt.want, got, cmpOpts...); diff != "" {
 | 
								if diff := cmp.Diff(tt.want, got, cmpOpts...); diff != "" {
 | 
				
			||||||
				t.Errorf("PodTopologySpread#PreFilter() returned diff (-want,+got):\n%s", diff)
 | 
									t.Errorf("PodTopologySpread#PreFilter() returned unexpected prefilter status: diff (-want,+got):\n%s", diff)
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
		})
 | 
							})
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user