Change the API priority #102943

CaoDonghui123 · 2021-06-17T09:34:36Z

What type of PR is this?

/kind api-change

What this PR does / why we need it:

TODO (mortent): priority should change after 1.21. See #95718 (comment)

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

CaoDonghui123 · 2021-06-17T10:26:20Z

/retest

fejta-bot · 2021-06-17T10:33:06Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

pacoxu · 2021-06-17T11:07:53Z

/cc @mortent

You may follow 8574e3e to fix the test failures.

test/integration/etcd/data.go need updated as well. https://github.com/kubernetes/kubernetes/blob/master/test/integration/etcd/data.go#L266-L282

CaoDonghui123 · 2021-06-18T02:46:02Z

/hold

pacoxu · 2021-06-18T02:52:05Z

To stop CI from running tests, you can convert PR to Draft. And /test pull-kubernetes-unit to run a specified test to check the current commit.

CaoDonghui123 · 2021-06-18T05:54:15Z

/test pull-kubernetes-verify

fedebongio · 2021-06-23T01:04:54Z

assign @kubernetes/api-reviewers
/remove-sig api-machinery

liggitt · 2021-06-23T03:02:59Z

pkg/registry/policy/podsecuritypolicy/storage/storage_test.go

@@ -72,40 +71,6 @@ func validNewPodSecurityPolicy() *policy.PodSecurityPolicy {
 	}
 }

-func TestCreate(t *testing.T) {


restore these?

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/102943/pull-kubernetes-unit/1405733896372359168
I tried to fix these failed by removing TestCreate() and TestUpdate()

oh... the failing tests mean that this change completely breaks the podsecuritypolicy API. That resource doesn't have a v1 type, so switching the default for policy to v1 means that that API fails to encode.

You need to do something like the following to tell the server to keep storing PodSecurityPolicy objects in v1beta1:

diff --git a/pkg/kubeapiserver/default_storage_factory_builder.go b/pkg/kubeapiserver/default_storage_factory_builder.go index 31479d77646..4802f4cf07d 100644 --- a/pkg/kubeapiserver/default_storage_factory_builder.go +++ b/pkg/kubeapiserver/default_storage_factory_builder.go @@ -62,6 +62,7 @@ func NewStorageFactoryConfig() *StorageFactoryConfig { resources := []schema.GroupVersionResource{ apisstorage.Resource("csistoragecapacities").WithVersion("v1beta1"), + policy.Resource("podsecuritypolicies").WithVersion("v1beta1"), } return &StorageFactoryConfig{

thank you for your help
I will fix it

look like the test failed still there

liggitt

~~lgtm other than restoring the dropped tests~~

edit: this breaks the podsecuritypolicy API. see #102943 (comment)

k8s-ci-robot · 2021-06-23T06:16:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: CaoDonghui123
To complete the pull request process, please assign liggitt after the PR has been reviewed.
You can assign the PR to them by writing /assign @liggitt in a comment when ready.

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

Needs approval from an approver in each of these files:

pkg/apis/OWNERS
pkg/controlplane/storageversionhashdata/OWNERS
pkg/kubeapiserver/OWNERS
test/integration/etcd/OWNERS

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

k8s-ci-robot · 2021-06-23T07:21:29Z

@CaoDonghui123: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-unit	`ed680b4`	link	`/test pull-kubernetes-unit`
pull-kubernetes-e2e-gce-ubuntu-containerd	`ed680b4`	link	`/test pull-kubernetes-e2e-gce-ubuntu-containerd`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

caesarxuchao · 2021-06-24T20:04:37Z

/triage accepted

k8s-ci-robot · 2021-08-10T23:05:01Z

@CaoDonghui123: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ritazh · 2021-11-01T16:22:17Z

@CaoDonghui123 can you pls rebase?

liggitt · 2021-11-01T16:28:07Z

pkg/controlplane/storageversionhashdata/data.go

-	"policy/v1beta1/podsecuritypolicies":                                   "khBLobUXkqA=",
+	"policy/v1/poddisruptionbudgets":                                       "EVWiDmWqyJw=",
+	"policy/v1beta1/poddisruptionbudgets":                                  "EVWiDmWqyJw=",
+	"policy/v1beta1/podsecuritypolicies":                                   "bAz9Pn7x8wg=",


podsecuritypolicies must not change, there's no v1 version to encode to (I would not expect this test to pass)

k8s-ci-robot requested review from alculquicondor and lavalamp June 17, 2021 09:35

k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 17, 2021

k8s-ci-robot requested a review from mortent June 17, 2021 11:07

CaoDonghui123 force-pushed the fixtodo1 branch from fd546eb to 8cc675f Compare June 18, 2021 02:12

k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jun 18, 2021

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 18, 2021

CaoDonghui123 marked this pull request as draft June 18, 2021 03:00

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 18, 2021

CaoDonghui123 force-pushed the fixtodo1 branch from 8cc675f to 1866c15 Compare June 18, 2021 03:46

CaoDonghui123 marked this pull request as ready for review June 18, 2021 03:47

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 18, 2021

CaoDonghui123 force-pushed the fixtodo1 branch from 1866c15 to 2582b7a Compare June 21, 2021 06:01

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 21, 2021

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 21, 2021

k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 23, 2021

liggitt reviewed Jun 23, 2021

View reviewed changes

Change the API priority

ed680b4

CaoDonghui123 force-pushed the fixtodo1 branch from 2582b7a to ed680b4 Compare June 23, 2021 06:15

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 23, 2021

k8s-ci-robot added area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Jun 23, 2021

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 24, 2021

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

liggitt reviewed Nov 1, 2021

View reviewed changes

CaoDonghui123 closed this Nov 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change the API priority #102943

Change the API priority #102943

CaoDonghui123 commented Jun 17, 2021

CaoDonghui123 commented Jun 17, 2021

fejta-bot commented Jun 17, 2021

pacoxu commented Jun 17, 2021 •

edited

Loading

CaoDonghui123 commented Jun 18, 2021

pacoxu commented Jun 18, 2021

CaoDonghui123 commented Jun 18, 2021

fedebongio commented Jun 23, 2021

liggitt Jun 23, 2021

CaoDonghui123 Jun 23, 2021

liggitt Jun 23, 2021

CaoDonghui123 Jun 23, 2021

CaoDonghui123 Jun 24, 2021

liggitt left a comment •

edited

Loading

k8s-ci-robot commented Jun 23, 2021

k8s-ci-robot commented Jun 23, 2021

caesarxuchao commented Jun 24, 2021

k8s-ci-robot commented Aug 10, 2021

ritazh commented Nov 1, 2021

liggitt Nov 1, 2021

Change the API priority #102943

Change the API priority #102943

Conversation

CaoDonghui123 commented Jun 17, 2021

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

CaoDonghui123 commented Jun 17, 2021

fejta-bot commented Jun 17, 2021

pacoxu commented Jun 17, 2021 • edited Loading

CaoDonghui123 commented Jun 18, 2021

pacoxu commented Jun 18, 2021

CaoDonghui123 commented Jun 18, 2021

fedebongio commented Jun 23, 2021

liggitt Jun 23, 2021

Choose a reason for hiding this comment

CaoDonghui123 Jun 23, 2021

Choose a reason for hiding this comment

liggitt Jun 23, 2021

Choose a reason for hiding this comment

CaoDonghui123 Jun 23, 2021

Choose a reason for hiding this comment

CaoDonghui123 Jun 24, 2021

Choose a reason for hiding this comment

liggitt left a comment • edited Loading

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 23, 2021

k8s-ci-robot commented Jun 23, 2021

caesarxuchao commented Jun 24, 2021

k8s-ci-robot commented Aug 10, 2021

ritazh commented Nov 1, 2021

liggitt Nov 1, 2021

Choose a reason for hiding this comment

pacoxu commented Jun 17, 2021 •

edited

Loading

liggitt left a comment •

edited

Loading