Clarify remark about object names wrt CRD #46263
Conversation
k8s-ci-robot
added
sig/docs
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -161,7 +161,7 @@ The [CustomResourceDefinition](/docs/tasks/extend-kubernetes/custom-resources/cu | |||
| API resource allows you to define custom resources. | |||
| Defining a CRD object creates a new custom resource with a name and schema that you specify. | |||
| The Kubernetes API serves and handles the storage of your custom resource. | |||
| The name of a CRD object must be a valid | |||
| The name of an object whose kind/resource is defined by a CRD must be a valid | |||
There was a problem hiding this comment.
Sounds to me that this paragraph focuses more on the CRD rather than the CRs created from the CRDs.
There was a problem hiding this comment.
I agree. I recommend:
- revert this line
- add some more text at the end of the paragraph that talks about restrictions on names of CRs, with a bit of a more verbose explanation about the difference between a CRD and the CRs you can create once a CRD is defined.
There was a problem hiding this comment.
@tengqm, @sftim: thanks for the comments. I set out to do as suggested, but am having trouble finding "the paragraph that talks about restrictions on names of CRs". Can someone please provide a pointer?
BTW, regarding the persistent slippage between "resource" and "object", this file has a clear statement that comports with most (but sadly not all) usage in the code:
A resource is an endpoint in the Kubernetes API that stores a collection of {{< glossary_tooltip text="API objects" term_id="object" >}} of a certain kind; for example, the built-in pods resource contains a collection of Pod objects.
So the thing this PR is trying to clarify is names of objects whose kind or resource is defined by a CRD object.
There was a problem hiding this comment.
A resource is an endpoint in the Kubernetes API that stores a collection of {{< glossary_tooltip text="API objects" term_id="object" >}} of a certain kind; for example, the built-in pods resource contains a collection of Pod objects.
That text isn't as helpful as we'd like it to be, because (at the HTTP level) all objects are also resources. In fact, and as I see it:
- all collections are resources
- all objects are resources
- some resources are objects
You can use a CRD to define resources that aren't objects, so we should use “resource” when we talk about those. When we talk about CRDs themselves, we can call them resources but every CRD itself is 100% an object.
There was a problem hiding this comment.
Yes, I agree, in the web world in general, "resource" is quite a general thing. However, in most of the Kubernetes code, "resource" is specifically a collection of objects. It makes me sad that the Kubernetes project chose to put a different meaning on that well established word. But here we are. In any case, my real problem here is understanding where I should put the remark about names of objects whose kind or resource is defined by a CRD.
There was a problem hiding this comment.
where I should put the remark about names of objects whose kind or resource is defined by a CRD.
The page could use a refactor for sure. However, given how it looks today, I'd put that remark within
https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/#customresourcedefinitions
We could use a tutorial page about writing (and installing) a CRD; something for another time though.
There was a problem hiding this comment.
Also, I am not sure I understand the remark that "You can use a CRD to define resources that aren't objects". Terminology still gets in the way of even that. If we start with the dichotomy that I quoted 30 minutes ago (between "resource" and "object"), then it is a tautology that a CRD defines a resource and a resource is not an object. But I suspect you mean that a CRD can define a collection of things that are not objects? If so, that is a surprises to me; can you please elaborate?
There was a problem hiding this comment.
Ignore what's in the comments and source code for a while; those aren't really part of the documentation.
A CRD can define a new API for [things with .metadata and .spec], which for short we call “objects”. So you could add a CRD and get a CompressedConfigMap API, and then create three CompressedConfigMaps.
I think you could also use a CRD to define an API for [things that have .fruitName] and no other fields. Those things would not be K8s objects, but they would be K8s resources. Sets of the things, outside of the Golang implementation of Kubernetes' API, would be collections (for example, deletecollection works on them).
The CompressedConfigMaps would be resources and the fruit things would be resources too, but the fruit things wouldn't be objects.
|
@MikeSpreitzer Whenever you get a chance, could you please take a look at the reviewers' feedback and respond to them? Thanks! |
|
.. and the force-push to 4bdf0e8 fixes a typo. |
content/en/docs/concepts/extend-kubernetes/api-extension/custom-resources.md
Outdated
Show resolved
Hide resolved
MikeSpreitzer
force-pushed
the
fix-46262
branch
2 times, most recently
from
2964d80
to
23585a7
Compare
|
the changes are technically accurate and I like having the update in the table for comparison lgtm |
|
@sftim: I think you have gotten what you need. |
content/en/docs/concepts/extend-kubernetes/api-extension/custom-resources.md
Show resolved
Hide resolved
Signed-off-by: Mike Spreitzer <mspreitz@us.ibm.com>
|
lgtm |
|
@sftim: back to you. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 590d5a989439b9f39458841f0196177c13e95909
|
| @@ -223,6 +224,7 @@ Aggregated APIs offer more advanced API features and customization of other feat | |||
| | strategic-merge-patch | The new endpoints support PATCH with `Content-Type: application/strategic-merge-patch+json`. Useful for updating objects that may be modified both locally, and by the server. For more information, see ["Update API Objects in Place Using kubectl patch"](/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/) | No | Yes | | |||
| | Protocol Buffers | The new resource supports clients that want to use Protocol Buffers | No | Yes | | |||
| | OpenAPI Schema | Is there an OpenAPI (swagger) schema for the types that can be dynamically fetched from the server? Is the user protected from misspelling field names by ensuring only allowed fields are set? Are types enforced (in other words, don't put an `int` in a `string` field?) | Yes, based on the [OpenAPI v3.0 validation](/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation) schema (GA in 1.16). | Yes | | |||
| | Instance Name | Does this extension mechanism impose any constraints on the names of objects whose kind/resource is defined this way? | Yes, such an object's name must be a valid DNS subdomain name. | No | | |||
There was a problem hiding this comment.
Just slightly more than a nit IMO:
| | Instance Name | Does this extension mechanism impose any constraints on the names of objects whose kind/resource is defined this way? | Yes, such an object's name must be a valid DNS subdomain name. | No | | |
| | Instance Name | Does this extension mechanism impose any constraints on the names of resources whose kind/resource is defined this way? | Yes, such an object's name must be a valid DNS subdomain name. | No | |
Aggregated APIs can serve things that aren't objects. Not sure about CRDs, maybe they can too (I can't think of an example though).
There was a problem hiding this comment.
A CRD can only define a kind of object.
There was a problem hiding this comment.
CRs are always objects with full CRUD. Non-persisted objects like SomethingReview you could implement in an aggregated apiserver are not possible with CRDs. So it's fine to talk about just objects for CRDs because all CRs are.
But even in the aggregated case, when you talk about "instance name" this by definition refers to ObjectMeta and that makes the resource an object.
There was a problem hiding this comment.
When an aggregated API server responds to a get or list, is this always an object or collection of objects? I'm thinking of MetricValue from custom.metrics.k8s.io/v1beta1 as a counterexample.
(We shouldn't assume that the aggregated API server uses our library or even what language it's written in, so long as it's conformant).
There was a problem hiding this comment.
If unsure, accept the suggestion.
There was a problem hiding this comment.
I think my suggestion is correct. If it's wrong, please explain why.
This PR has nothing to do with what can be delegated an external apiserver.
This feedback is about a change to a row added to a table; that table compares using an APIService to using a CustomResourceDefinition. Are you sure that APIService isn't relevant? I'd like evidence about why, because I can't see it myself.
There was a problem hiding this comment.
But even in the aggregated case, when you talk about "instance name" this by definition refers to ObjectMeta and that makes the resource an object.
@stts are you saying that for some resource fetched from a [conforming] extension API server using the get verb, if it can be fetched then it always has a .metadata.name (and is therefore an object)?
I know that typical implementations have .metadata and .metadata.name but I don't know that we guarantee it, or define conformance strictly enough that omitting it is formally an error. My best guess is that we don't make that guarantee, hence my suggestion.
There was a problem hiding this comment.
Oh, sorry @sftim, now I see why you are talking about the other form of aggregation.
I was hoping to avoid expanding the scope of the discussion. In fact, the suggested table edit is technically narrow: the first column holds a question, and in the suggested new row the question is only about objects. We could leave it like that, the row only asking and answering a question about objects.
If you insist on expanding the discussion, I will answer the question about APIService and what it registers. Looking at the definition of an APIService object, you see that its Spec holds an API group that says what the APIService is about. Only objects belong to API groups, so an APIService can only set up delegation for some objects.
There was a problem hiding this comment.
Let's omit the change to this line for now. I still think it misleads, and if we omit this one change, we can move things forward.
In a separate PR, we can discuss whether aggregated APIs can only ever serve objects with names. Not here.
There was a problem hiding this comment.
Actually, I'm persuaded. It risks misleading but not very much. We can merge without clearing thing up.
/hold cancel
|
/hold Adding this because of the confusion around #46263 (comment) |
k8s-ci-robot
added
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sftim 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
This PR clarifies the remark about the restrictions on the name of an object whose kind/resource is defined by a CRD.
Fixes #46262