helm: Update default `descheduler/v1alpha1` to `descheduler/v1alpha2`

[dongjiang1989]

This PR setting default deschedulerPolicyAPIVersion from descheduler/v1alpha1 to descheduler/v1alpha2

[k8s-ci-robot]

Hi @dongjiang1989. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[a7i]

Thank you for your PR. Duplicate of https://github.com/kubernetes-sigs/descheduler/pull/1222 and #1139

It was discussed in the community meeting that we should finish #1187 before making this change. Please confirm @knelasevero

[a7i]

/hold

[knelasevero]

the other way around, getting rid of v1alpha1 will make it easier to wrap the framework up, and get rid of some of the complex conversions

I say we go with v1alph2 as default already. Quite a few releases with that up

[knelasevero]

/lgtm

[knelasevero]

/cc @damemi /cc @ingvagabund

[damemi]

1187 seems pretty open ended, looks like there is some architectural changes in the first comment. But other things like nodeFit updates or other features can be added later. Could we get an update on that issue of what is really in scope of "wrap up"? Having a list of what's left to be done would be helpful

If most of the wrap up work that's left is there to support v1alpha1, then I agree that switching to default here will help nudge users so we can drop v1alpha1.

[a7i]

@dongjiang1989 thoughts on making it match what's in https://github.com/kubernetes-sigs/descheduler/pull/1139/files? That matches closer to what we have for kustomize w.r.t naming

[knelasevero]

If most of the wrap up work that's left is there to support v1alpha1, then I agree that switching to default here will help nudge users so we can drop v1alpha1.

It is not that most of the wrap up work that's left is there to support v1alpha1. We have to go through that list from Jan and that other list from me anyways. However, some of those items are way harder to work on with the v1alpha1 still in our repo (and the conversions we have here). Simplifying the register or even having less arguments on the register function all get easier to do when we dont have the conversion logic being carried into the registry (which I had to do since using the universal decoder directly was hard with versions so different [and all that thing about having plugin types indepedently in each plugin and not in the general API]). New versions that we might have (v1alpha3 or v1beta1 or w/e) will all be easier to handle since we probably will be able to just use the universal decoder since their structure will probably be way more similar (I think).

Having a list of what's left to be done would be helpful

Isn't it already listed there?

[damemi]

@knelasevero that issue lists a lot of tasks, and it's not clear which are done or in progress, or the priority that they need to be done in. So, that's why I was asking for an update on it.

If the goal is to make v1alpha2 the default so we can drop v1alpha1, then I agree with @a7i that this seems like something that would be done at the end of the wrap up. That doesn't mean it can't be done now, but we need to clarify what the scope of the wrap up really is.

If there are goals for v1alpha2 that aren't reasonable to support with v1alpha1 (such as simplifying the register), then we can identify those and break off v1alpha1 here. But I would like to see a final status update so we know where everything stands before making that call.

That said, I don't see a reason to block this PR if it's just updating the default value in the Helm chart. I actually think we should merge this, just not with the goal of immediately following up with dropping v1alpha1 (if that makes sense?)

[knelasevero]

@knelasevero that issue lists a lot of tasks, and it's not clear which are done or in progress, or the priority that they need to be done in. So, that's why I was asking for an update on it.

Sorry, I misread it :sweat_smile: Having an update on each item and pointing out if they are finished or how far we are on each makes a lot of sense.

If the goal is to make v1alpha2 the default so we can drop v1alpha1, then I agree with @a7i that this seems like something that would be done at the end of the wrap up. That doesn't mean it can't be done now, but we need to clarify what the scope of the wrap up really is.

I agree, I think there are conflicting understandings of what the wrap up would be. I would have to think a little bit about that one, and I guess we can go over this in the community call or the async call we wanted to have eventually.

If there are goals for v1alpha2 that aren't reasonable to support with v1alpha1 (such as simplifying the register), then we can identify those and break off v1alpha1 here. But I would like to see a final status update so we know where everything stands before making that call.

What do you mean by break off? (do we want to move this conversation to somewhere else? I am ok keeping it here, just asking since this is going a bit offtopic for the PR)

That said, I don't see a reason to block this PR if it's just updating the default value in the Helm chart. I actually think we should merge this, just not with the goal of immediately following up with dropping v1alpha1 (if that makes sense?)

Agree, I should have been more clear in my previous comment.

the other way around, getting rid of v1alpha1 will make it easier to wrap the framework up, and get rid of some of the complex conversions

I say we go with v1alpha2 as default already. Quite a few releases with that up

When I said this :point_up: I meant not in this PR, and not necessarily in something that would be an immediate follow up, more like we should start making more efforts in that direction (soon), and this PR is one of the steps.

[damemi]

@knelasevero sounds good, sorry to derail this PR. I'm +1 to unblocking it then. By "break off" I meant make it clear where our support/feature set for v1alpha1 is ending (specifically in the context of the v1alpha2 work and how it affects that). But yes, we should move the discussion to a different place too :)

[dongjiang1989]

@dongjiang1989 thoughts on making it match what's in https://github.com/kubernetes-sigs/descheduler/pull/1139/files? That matches closer to what we have for kustomize w.r.t naming

Thanks for your review. I got it. This PR follow it

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from knelasevero. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/kubernetes-sigs/descheduler/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

[ingvagabund]

v1alpha2 has been around for some time. +1 making it "officially closed for any other change" and focusing on v1alpha3 instead.

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[a7i]

Already addressed in #1139 Thank you for your contribution 🏆 /close