Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CR scale subresources update request fails when using non-storage version's endpoint #68035

Closed
roycaihw opened this issue Aug 29, 2018 · 11 comments · Fixed by #70087
Closed

CR scale subresources update request fails when using non-storage version's endpoint #68035

roycaihw opened this issue Aug 29, 2018 · 11 comments · Fixed by #70087
Assignees
Labels
area/custom-resources kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@roycaihw
Copy link
Member

roycaihw commented Aug 29, 2018

Is this a BUG REPORT or FEATURE REQUEST?:
/kind bug

What happened:
When trying to update a CR's scale subresource, the update request fails if the CRD has multiple versions and the request is sent to a non-storage versions's endpoint.

What you expected to happen:
The update request succeed.

How to reproduce it (as minimally and precisely as possible):
roycaihw@71b7f5d In CRD integration subresources_test, add a second version (v1) to the CRD and mark it as the storage version. The test TestScaleSubresource still exercises on the top level version (v1beta1) and fails with:

FAIL: TestScaleSubresource (1.59s)
	subresources_test.go:291: WishIHadChosenNoxu.mygroup.example.com "foo" is invalid: apiVersion: Invalid value: "mygroup.example.com/v1": must be mygroup.example.com/v1beta1

Anything else we need to know?:

Environment:

  • Kubernetes version (use kubectl version): from master branch HEAD
  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):
  • Kernel (e.g. uname -a):
  • Install tools:
  • Others:

/assign @mbohlool
cc @sttts

@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. kind/bug Categorizes issue or PR as related to a bug. labels Aug 29, 2018
@roycaihw
Copy link
Member Author

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Aug 29, 2018
@sttts
Copy link
Contributor

sttts commented Aug 30, 2018

This is much deeper in apimachinery than #68038 suggests. It's again a case of machinery conversion code which thinks that in=Unstructured, out=Unstructured implies same GVK. Obviously this is wrong. But the code (versioning decoder in this case) skips another CR conversion step.

@liggitt
Copy link
Member

liggitt commented Aug 30, 2018

I attempted to recreate this against master and was unable to... PUT requests to the /status subresource for both versions worked as expected for me

@sttts
Copy link
Contributor

sttts commented Aug 30, 2018

I attempted to recreate this against master and was unable to... PUT requests to the /status subresource for both versions worked as expected for me

Only scale has a problem as far as I see.

@roycaihw roycaihw changed the title CR subresources update request fails when using non-storage version's endpoint CR scale subresources update request fails when using non-storage version's endpoint Aug 30, 2018
@roycaihw
Copy link
Member Author

Only scale has a problem as far as I see.

You're right. I had status integration test fail for different reason. Updated the issue topic and comment

@roycaihw
Copy link
Member Author

roycaihw commented Aug 30, 2018

/area custom-resources
/milestone v1.12

@k8s-ci-robot
Copy link
Contributor

@roycaihw: You must be a member of the kubernetes/kubernetes-milestone-maintainers github team to set the milestone.

In response to this:

/area custom-resource
/milestone v1.12

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.

@sttts sttts added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Aug 31, 2018
@sttts sttts added this to the v1.12 milestone Aug 31, 2018
@guineveresaenger
Copy link
Contributor

@sttts @roycaihw this issue needs to be priority critical-urgent for it to remain in 1.12 milestone. Please check and triage.

cc @lavalamp @deads2k

@deads2k
Copy link
Contributor

deads2k commented Sep 4, 2018

/priority critical-urgent

critical to CRD conversion

@k8s-ci-robot k8s-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Sep 4, 2018
@guineveresaenger
Copy link
Contributor

/remove-priority important-soon

@guineveresaenger
Copy link
Contributor

/milestone clear
/priority important-soon
/remove-priority critical-urgent

This is related to CRD webhook versioning feature which has been pushed back to v1.13.

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Sep 11, 2018
@k8s-ci-robot k8s-ci-robot removed this from the v1.12 milestone Sep 11, 2018
@k8s-ci-robot k8s-ci-robot removed the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/custom-resources kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
7 participants