Merge pull request #117683 from utam0k/skip-topologyspread-empty

Add check to skip PodTopologySpread PreFilter if no constraints are specified
This commit is contained in:
Kubernetes Prow Robot 2023-05-03 06:48:24 -07:00 committed by GitHub
commit 0d67dd689b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 10 deletions

View File

@ -152,7 +152,10 @@ func (pl *PodTopologySpread) PreFilter(ctx context.Context, cycleState *framewor
s, err := pl.calPreFilterState(ctx, pod)
if err != nil {
return nil, framework.AsStatus(err)
} else if s != nil && len(s.Constraints) == 0 {
return nil, framework.NewStatus(framework.Skip)
}
cycleState.Write(preFilterStateKey, s)
return nil, nil
}
@ -236,11 +239,8 @@ func getPreFilterState(cycleState *framework.CycleState) (*preFilterState, error
// calPreFilterState computes preFilterState describing how pods are spread on topologies.
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 err error
if len(pod.Spec.TopologySpreadConstraints) > 0 {
// We have feature gating in APIServer to strip the spec
// 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
}
allNodes, err := pl.sharedLister.NodeInfos().List()
if err != nil {
return nil, fmt.Errorf("listing NodeInfos: %w", err)
}
s := preFilterState{
Constraints: constraints,
TpKeyToCriticalPaths: make(map[string]*criticalPaths, len(constraints)),

View File

@ -77,6 +77,7 @@ func TestPreFilterState(t *testing.T) {
objs []runtime.Object
defaultConstraints []v1.TopologySpreadConstraint
want *preFilterState
wantPrefilterStatus *framework.Status
enableMinDomains bool
enableNodeInclusionPolicy bool
enableMatchLabelKeys bool
@ -574,7 +575,7 @@ func TestPreFilterState(t *testing.T) {
objs: []runtime.Object{
&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",
@ -614,7 +615,7 @@ func TestPreFilterState(t *testing.T) {
objs: []runtime.Object{
&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",
@ -1454,6 +1455,14 @@ func TestPreFilterState(t *testing.T) {
},
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 {
t.Run(tt.name, func(t *testing.T) {
@ -1471,15 +1480,21 @@ func TestPreFilterState(t *testing.T) {
p.(*PodTopologySpread).enableMatchLabelKeysInPodTopologySpread = tt.enableMatchLabelKeys
cs := framework.NewCycleState()
if _, s := p.(*PodTopologySpread).PreFilter(ctx, cs, tt.pod); !s.IsSuccess() {
t.Fatal(s.AsError())
_, s := p.(*PodTopologySpread).PreFilter(ctx, cs, tt.pod)
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)
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 != "" {
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)
}
})
}