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.

Sign up for GitHub

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

Jump to bottom

Update knative CRD status with "UpdateStatus" instead of "Update" #2274

Closed
lichuqiang opened this issue Oct 20, 2018 · 4 comments
Closed

Update knative CRD status with "UpdateStatus" instead of "Update" #2274

lichuqiang opened this issue Oct 20, 2018 · 4 comments
Assignees
@lichuqiang
Labels
area/API API objects and controllers kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/spec Discussion of how a feature should be exposed to customers.

Comments

@lichuqiang
Copy link
Contributor

Expected Behavior

Updating knative CRD status with kube client with "UpdateStatus" .

Actual Behavior

We are calling "Update" interface for status update.

Additional Info

As described in #2189 (comment), currently we are calling "Update" interface for CRD resources status update. And related codes are commented with "for CRD there's no updatestatus, so use normal update".

Is it just cruft due to historic reasons? or currently there's still bug (that I'm not aware of) in UpdateStatus for CRDs that block us from moving forward?
Can we just get rid of this and call UpdateStatus for status update directly?

/cc @vaikas-google @mattmoor @tcnghia for thoughts
@knative-prow-robot knative-prow-robot added area/API API objects and controllers kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/spec Discussion of how a feature should be exposed to customers. labels Oct 20, 2018
@lichuqiang
Copy link
Contributor Author

/assign

@vaikas
Copy link
Contributor

vaikas commented Oct 23, 2018

Yes, it's still sort of kind of historic cruft, CRDs before didn't support the /status subresource. They recently (1.12?) started supporting this. So, all those codes should get changed when enough of the hosted k8s clusters support it. We should discuss what the bar is for determining when we can start relying on a behaviour given a cluster.
This also has some ramifications for existing clusters since unless they get upgraded to the min bar for supporting the subresources for CRDs, things will break.
Note that in 1.12 they are still in BETA:
https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#subresources

@mattmoor
Copy link
Member

This is supported in 1.11. @dprotaso (with @bsnchan ) has an ancient change to add support for this.

There may be another issue for this, let me dig a bit

@mattmoor
Copy link
Member

Let's use: #643

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lichuqiang lichuqiang

Labels
area/API API objects and controllers kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/spec Discussion of how a feature should be exposed to customers.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mattmoor @vaikas @lichuqiang @knative-prow-robot