fix: handle PickFixed CRP in eviction controller #998
Conversation
| // we don't know the desired bindings for PickAll. | ||
| } | ||
|
|
||
| var disruptionsAllowed int |
There was a problem hiding this comment.
I can't seem to find where "For PickAll CRPs, MaxUnavailable won't be specified in DB" is enforced?
There was a problem hiding this comment.
There was a problem hiding this comment.
so we mark any eviction associated with the bad PDB as not executed instead of rejecting the PBD itself
There was a problem hiding this comment.
We use PDB as an information source as of now, even if the CRP targeted by PDB is missing we allow the PDB to exist. So in the case of,
- PickAll CRP where the PDB specifies MaxUnavailable or MinAvailable as percentage we say the eviction cannot be executed due to a misconfigured PDB
- In the case for PickFixed CRP, the eviction fails on validation, and we mark eviction as invalid
Arvindthiru
force-pushed
the
evictionPickFixed
branch
from
81821b1 to
c706b93
Compare
| // * if the linked Placement object is of the PickFixed placement type, | ||
| // the percentage is against the number of clusters specified in the placement (i.e., the | ||
| // length of ClusterNames field in the placement policy); | ||
| // we don't perform any calculation because eviction is not allowed for PickFixed CRP. |
There was a problem hiding this comment.
PickAll API comments
For MaxUnavailable: https://github.com/Azure/fleet/blob/main/apis/placement/v1alpha1/disruptionbudget_types.go#L49-L50
For MinAvailable:
https://github.com/Azure/fleet/blob/main/apis/placement/v1alpha1/disruptionbudget_types.go#L79-L80
2 participants
Description of your changes
Fixes #
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer