Create pkg/scheduling/apis/v1beta1 and move priorityClass to beta #63100

ravisantoshgudimetla · 2018-04-24T18:51:13Z

What this PR does / why we need it:
This is for creating pkg/apis/scheduling/v1beta1 so that priorityClasses could be moved to beta.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of #57471

Special notes for your reviewer:
/cc @bsalamat @aveshagarwal

Release note:

The `PriorityClass` API is promoted to `scheduling.k8s.io/v1beta1`

aveshagarwal · 2018-04-25T13:02:24Z

hack/.golint_failures

@@ -66,6 +66,7 @@ pkg/apis/rbac/v1beta1
 pkg/apis/rbac/validation
 pkg/apis/scheduling
 pkg/apis/scheduling/v1alpha1


why are you keeping v1alpha1?

I think as I understand, it might be needed, if some apis are alpha and some are beta?

That is correct. I think we don't have a beta group for scheduling yet. So, we need it say if new api like priorityClass comes up in future.

aveshagarwal · 2018-04-25T13:05:02Z

pkg/apis/scheduling/install/install.go

@@ -37,12 +38,13 @@ func Install(groupFactoryRegistry announced.APIGroupFactoryRegistry, registry *r
 	if err := announced.NewGroupMetaFactory(
 		&announced.GroupMetaFactoryArgs{
 			GroupName:                  scheduling.GroupName,
-			VersionPreferenceOrder:     []string{v1alpha1.SchemeGroupVersion.Version},
+			VersionPreferenceOrder:     []string{v1beta1.SchemeGroupVersion.Version, v1alpha1.SchemeGroupVersion.Version},
 			RootScopedKinds:            sets.NewString("PriorityClass"),


Here it says that rootscopekind is PriorityClass? So wondering why to keep both v1alpha1 and v1beta1 if its only for priorityclass?

I am not a 100% sure. I believe it is because of backward compatibility. I have looked at existing API which have graduated to beta and all of them have this pattern of beta and alpha.

In my understanding, there is no guarantee provided by Kubernetes of backward compatibility for alpha apis. See the alpha api section: https://kubernetes.io/docs/concepts/overview/kubernetes-api/

bsalamat

Looks generally good, but I am not so sure about keeping v1alpha1. API folks can help us review that part.
Please make sure verifications and tests pass.

aveshagarwal · 2018-04-25T18:18:40Z

@liggitt

ravisantoshgudimetla · 2018-04-25T22:48:13Z

Looks generally good, but I am not so sure about keeping v1alpha1. API folks can help us review that part.

You mean priorityClass in v1alpha1 or remove v1alpha1 in general?

Please make sure verifications and tests pass.

Yes. There was an issue with go 1.9 version on my machine for gofmt. Changing to 1.10.1 solved it.

ravisantoshgudimetla · 2018-04-25T23:10:39Z

@kubernetes/sig-scheduling-pr-reviews @kubernetes/api-reviewers

bsalamat · 2018-04-26T01:16:31Z

You mean priorityClass in v1alpha1 or remove v1alpha1 in general?

Removing priorityClass from v1alpha1 and since it is the only item there, maybe removing v1alpha1 as well. Basically, I would like to know what the recommended workflow is when we graduate an API to beta.

k82cn · 2018-04-26T09:14:51Z

AFAIK, we should remove v1alpha1 in graduation :)

ravisantoshgudimetla · 2018-04-26T14:53:17Z

Removing priorityClass from v1alpha1 and since it is the only item there, maybe removing v1alpha1 as well. Basically, I would like to know what the recommended workflow is when we graduate an API to beta.

I am not 100% sure of removing entire v1alpha1. If in future, we want to have api like priorityClass as alpha we have create v1alpha1 again and having it also provides assurance that a particular api is in alpha for sometime before moving to beta.
I was following this guide, https://github.com/kubernetes/community/blob/master/contributors/devel/api_changes.md#making-a-new-api-version, here it tells to copy the existing folder instead of renaming or moving it. While I am not 100% sure why this is needed and this doesn't answer the exact question, I looked at the existing file structure in the code, For example, https://github.com/kubernetes/kubernetes/tree/master/pkg/apis/rbac has both v1beta,v1alpha,v1. It could be because some features within that group may be v1alpha1 and some may be v1beta1 that being said, I will wait for api folks to comment as well.

aveshagarwal · 2018-04-30T18:00:34Z

@bsalamat can you check why it was removed from 1.11 milestone?

ravisantoshgudimetla · 2018-05-11T14:51:14Z

/retest

ravisantoshgudimetla · 2018-05-11T20:00:53Z

/retest

ravisantoshgudimetla · 2018-05-11T23:40:02Z

/test pull-kubernetes-e2e-kops-aws

liggitt · 2018-05-12T04:09:29Z

pkg/features/kube_features.go

@@ -302,7 +302,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
 	HugePages:                                   {Default: true, PreRelease: utilfeature.Beta},
 	DebugContainers:                             {Default: false, PreRelease: utilfeature.Alpha},
 	PodShareProcessNamespace:                    {Default: false, PreRelease: utilfeature.Alpha},
-	PodPriority:                                 {Default: false, PreRelease: utilfeature.Alpha},


@aveshagarwal are the quota controls in place to allow this to be enabled by default? (flipping the feature gate isn't required to add v1beta1 priority classes)

@liggitt I believe you are talking about #57963, this got an lgtm but waiting on approval from @bsalamat

yes, that. until that is available, we should not enable the feature by default.

Ok, I will make this alpha so that this PR can go ahead.

liggitt · 2018-05-12T04:14:15Z

/lgtm

liggitt · 2018-05-12T06:19:58Z

/lgtm

ravisantoshgudimetla · 2018-05-12T15:13:05Z

/retest

ravisantoshgudimetla · 2018-05-12T15:40:45Z

@liggitt Thanks for the review.

/assign @smarterclayton

k8s-github-robot · 2018-05-12T15:41:32Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@liggitt @ravisantoshgudimetla @smarterclayton

Pull Request Labels

sig/scheduling: Pull Request will be escalated to these SIGs if needed.
priority/important-soon: Escalate to the pull request owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
kind/feature: New functionality.

Help

smarterclayton · 2018-05-14T18:29:12Z

/approve

based on api approver comments and sig approval

k8s-ci-robot · 2018-05-14T18:29:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, ravisantoshgudimetla, smarterclayton

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:

~~api/OWNERS~~ [liggitt,smarterclayton]
~~cmd/kube-apiserver/OWNERS~~ [liggitt,smarterclayton]
~~docs/api-reference/OWNERS~~ [liggitt,smarterclayton]
~~hack/OWNERS~~ [liggitt,smarterclayton]
~~pkg/OWNERS~~ [smarterclayton]
~~staging/OWNERS~~ [smarterclayton]
~~test/OWNERS~~ [liggitt,smarterclayton]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ravisantoshgudimetla · 2018-05-14T18:32:27Z

Thanks @smarterclayton

k8s-github-robot · 2018-05-14T21:35:18Z

Automatic merge from submit-queue (batch tested with PRs 55511, 63372, 63400, 63100, 63769). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Apr 24, 2018

k8s-ci-robot requested review from aveshagarwal and bsalamat April 24, 2018 18:51

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 24, 2018

k8s-github-robot added the kind/new-api label Apr 24, 2018

ravisantoshgudimetla force-pushed the priority-beta-api branch 2 times, most recently from 302c5a1 to 30f7f25 Compare April 24, 2018 21:24

aveshagarwal reviewed Apr 25, 2018

View reviewed changes

ravisantoshgudimetla force-pushed the priority-beta-api branch from 30f7f25 to 255bfc7 Compare April 25, 2018 16:01

bsalamat reviewed Apr 25, 2018

View reviewed changes

ravisantoshgudimetla force-pushed the priority-beta-api branch 2 times, most recently from 538bd90 to 2542a83 Compare April 25, 2018 21:37

k8s-ci-robot added the sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. label Apr 25, 2018

bsalamat added this to the v1.11 milestone Apr 26, 2018

bsalamat added the status/approved-for-milestone label Apr 26, 2018

k8s-github-robot added the milestone/incomplete-labels label Apr 26, 2018

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 26, 2018

bsalamat added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Apr 26, 2018

k8s-github-robot added milestone/removed and removed milestone/incomplete-labels labels Apr 30, 2018

k8s-github-robot removed this from the v1.11 milestone Apr 30, 2018

ravisantoshgudimetla force-pushed the priority-beta-api branch from 8cb6702 to 3099271 Compare May 11, 2018 13:38

ravisantoshgudimetla force-pushed the priority-beta-api branch from 3099271 to 47e44fa Compare May 11, 2018 18:27

ravisantoshgudimetla mentioned this pull request May 11, 2018

Modify e2e tests to use priorityClass beta client version & switch priorityClass to beta #63724

Merged

ravisantoshgudimetla force-pushed the priority-beta-api branch from 47e44fa to 1f530ec Compare May 11, 2018 18:51

ravisantoshgudimetla force-pushed the priority-beta-api branch from 1f530ec to aab8378 Compare May 11, 2018 20:39

ravisantoshgudimetla force-pushed the priority-beta-api branch from aab8378 to 99a3484 Compare May 12, 2018 01:16

liggitt reviewed May 12, 2018

View reviewed changes

k8s-ci-robot assigned liggitt May 12, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 12, 2018

ravisantoshgudimetla added 2 commits May 12, 2018 02:01

Move scheduling api to beta

1db5990

Generated

f20bd00

ravisantoshgudimetla force-pushed the priority-beta-api branch from 99a3484 to f20bd00 Compare May 12, 2018 06:01

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 12, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 12, 2018

k8s-ci-robot assigned smarterclayton May 12, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 14, 2018

k8s-github-robot merged commit a1b54f3 into kubernetes:master May 14, 2018

ravisantoshgudimetla deleted the priority-beta-api branch May 14, 2018 21:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create pkg/scheduling/apis/v1beta1 and move priorityClass to beta #63100

Create pkg/scheduling/apis/v1beta1 and move priorityClass to beta #63100

ravisantoshgudimetla commented Apr 24, 2018 •

edited by liggitt

aveshagarwal Apr 25, 2018

aveshagarwal Apr 25, 2018

ravisantoshgudimetla Apr 25, 2018

aveshagarwal Apr 25, 2018

ravisantoshgudimetla Apr 25, 2018

aveshagarwal Apr 26, 2018

bsalamat left a comment

aveshagarwal commented Apr 25, 2018

ravisantoshgudimetla commented Apr 25, 2018

ravisantoshgudimetla commented Apr 25, 2018

bsalamat commented Apr 26, 2018

k82cn commented Apr 26, 2018

ravisantoshgudimetla commented Apr 26, 2018 •

edited

aveshagarwal commented Apr 30, 2018

ravisantoshgudimetla commented May 11, 2018

ravisantoshgudimetla commented May 11, 2018

ravisantoshgudimetla commented May 11, 2018

liggitt May 12, 2018 •

edited

ravisantoshgudimetla May 12, 2018

liggitt May 12, 2018 •

edited

ravisantoshgudimetla May 12, 2018 •

edited

ravisantoshgudimetla May 12, 2018

liggitt commented May 12, 2018

liggitt commented May 12, 2018

ravisantoshgudimetla commented May 12, 2018

ravisantoshgudimetla commented May 12, 2018 •

edited

k8s-github-robot commented May 12, 2018

smarterclayton commented May 14, 2018

k8s-ci-robot commented May 14, 2018

ravisantoshgudimetla commented May 14, 2018

k8s-github-robot commented May 14, 2018

Create pkg/scheduling/apis/v1beta1 and move priorityClass to beta #63100

Create pkg/scheduling/apis/v1beta1 and move priorityClass to beta #63100

Conversation

ravisantoshgudimetla commented Apr 24, 2018 • edited by liggitt

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsalamat left a comment

Choose a reason for hiding this comment

aveshagarwal commented Apr 25, 2018

ravisantoshgudimetla commented Apr 25, 2018

ravisantoshgudimetla commented Apr 25, 2018

bsalamat commented Apr 26, 2018

k82cn commented Apr 26, 2018

ravisantoshgudimetla commented Apr 26, 2018 • edited

aveshagarwal commented Apr 30, 2018

ravisantoshgudimetla commented May 11, 2018

ravisantoshgudimetla commented May 11, 2018

ravisantoshgudimetla commented May 11, 2018

liggitt May 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt May 12, 2018 • edited

Choose a reason for hiding this comment

ravisantoshgudimetla May 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented May 12, 2018

liggitt commented May 12, 2018

ravisantoshgudimetla commented May 12, 2018

ravisantoshgudimetla commented May 12, 2018 • edited

k8s-github-robot commented May 12, 2018

smarterclayton commented May 14, 2018

k8s-ci-robot commented May 14, 2018

ravisantoshgudimetla commented May 14, 2018

k8s-github-robot commented May 14, 2018

ravisantoshgudimetla commented Apr 24, 2018 •

edited by liggitt

ravisantoshgudimetla commented Apr 26, 2018 •

edited

liggitt May 12, 2018 •

edited

liggitt May 12, 2018 •

edited

ravisantoshgudimetla May 12, 2018 •

edited

ravisantoshgudimetla commented May 12, 2018 •

edited