Add a parameter to include soft topology spread constraints #474
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/cc @seanmalloy |
k8s-ci-robot
added
the
kind/feature
k8s-ci-robot
added
the
needs-rebase
lixiang233
force-pushed
the
Ft_include_soft_constraints
branch
from
0168fab
to
00ebb30
Compare
k8s-ci-robot
added
size/S
|
Rebased, PTAL @damemi @seanmalloy @ingvagabund |
| @@ -76,7 +76,7 @@ func RemovePodsViolatingTopologySpreadConstraint( | |||
| nodes []*v1.Node, | |||
| podEvictor *evictions.PodEvictor, | |||
| ) { | |||
| thresholdPriority, includedNamespaces, excludedNamespaces, err := validateAndParseTopologySpreadParams(ctx, client, strategy.Params) | |||
| includeSoftConstraints, thresholdPriority, includedNamespaces, excludedNamespaces, err := validateAndParseTopologySpreadParams(ctx, client, strategy.Params) | |||
There was a problem hiding this comment.
I don't think includeSoftConstraints needs to be validated by this function, as it just complicates the function signature (and isn't actually validated, since it's just a bool).
We can just use params.IncludeSoftConstraints on L129 and add a nil check in that if statement for params so we don't get a dereference panic. wdyt?
There was a problem hiding this comment.
I originally wanted to check params only once so I put it in validateAndParseTopologySpreadParams.
It's more concise to do the nil check inside that if statement, I'll update soon.
lixiang233
force-pushed
the
Ft_include_soft_constraints
branch
from
00ebb30
to
8ba9cb1
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damemi, lixiang233 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/lgtm |
k8s-ci-robot
added
the
lgtm
…t_constraints Add a parameter to include soft topology spread constraints
Fixes: #461