Descheduler profiles #548
Descheduler profiles #548
Conversation
|
/cc @soltysh @ingvagabund |
openshift-ci-robot
requested review from
ingvagabund,
soltysh,
danehans and
knobunc
| last-updated: 2020-11-23 | ||
| status: provisional | ||
| see-also: | ||
| - "https://github.com/openshift/enhancements/pull/541/" |
There was a problem hiding this comment.
I don't expect the other PR not merging, so I'd just link to the file like below.
|
|
||
| ## Release Signoff Checklist | ||
|
|
||
| - [ ] Enhancement is `implementable` |
There was a problem hiding this comment.
Check this, we are implementing this right away.
| type KubeDeschedulerSpec struct { | ||
| Profiles []string `json:"profiles"` | ||
| ... | ||
| } |
There was a problem hiding this comment.
Another alternative is a list of enums.
There was a problem hiding this comment.
are there any opinions on preference for either one? I'm not attached either way but curious if anyone else had any insight
There was a problem hiding this comment.
let's go with a list of enums, we can then re-use the kubebuilder validation for enums for that type. The only thing we'll have to ensure is to de-duplication or call out what happens and validation if contradicting profiles are applied.
| Ideally, this field will immediately replace the existing `strategies` config as the initial GA option for | ||
| configuring the descheduler. | ||
|
|
||
| However, since the current API is version `v1beta1` we may be required to continue supporting the |
There was a problem hiding this comment.
I'd clearly express the deprecation timeline, which is Beta: 9 months or 3 releases (whichever is longer), for beta-level APIs. For conversions you'll need to have both (strategies and profile) added to both APIs, but v1beta1 version should be deprecated right away, same with strategies field in v1, and point to using the new profiles.
There was a problem hiding this comment.
ok, so for the v1 API we should still provide strategies with a note that it's deprecated and will be removed? Or just provide the v1 API with only profiles and still support the v1beta1 api with strategies until that API is removed according to that timeline?
There was a problem hiding this comment.
@damemi yeah, for v1 provide deprecated strategies. We should also announce deprecation of v1beta1 too.
damemi
force-pushed
the
descheduler-profiles
branch
2 times, most recently
from
2f6b9c0
to
27735e8
Compare
| type KubeDeschedulerSpec struct { | ||
| Profiles []string `json:"profiles"` | ||
| ... | ||
| } |
There was a problem hiding this comment.
are there any opinions on preference for either one? I'm not attached either way but curious if anyone else had any insight
| Ideally, this field will immediately replace the existing `strategies` config as the initial GA option for | ||
| configuring the descheduler. | ||
|
|
||
| However, since the current API is version `v1beta1` we may be required to continue supporting the |
There was a problem hiding this comment.
ok, so for the v1 API we should still provide strategies with a note that it's deprecated and will be removed? Or just provide the v1 API with only profiles and still support the v1beta1 api with strategies until that API is removed according to that timeline?
damemi
force-pushed
the
descheduler-profiles
branch
from
27735e8
to
6eccef1
Compare
|
To clarify on what we decided for this, we will add both fields to v1beta1 and remove functionality for the old field (only providing it for transition support). When v1beta1 is removed in favor of v1, those fields will be removed and not present in v1. I have updated the enhancement to include this. |
damemi
force-pushed
the
descheduler-profiles
branch
from
6eccef1
to
6003e36
Compare
There was a problem hiding this comment.
This was discussed earlier today during our call and this was agreed between @damemi, @ingvagabund, @deads2k and myself
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damemi, soltysh 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 |
openshift-ci-robot
added
the
approved
No description provided.