Repository navigation
Conversation
AI assistance was used to review existing guidance and draft these documentation clarifications. Signed-off-by: Davanum Srinivas <davanum@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dims The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@kflynn please review and let me know what else we can word smith for now. |
|
cc @kubernetes/api-reviewers |
|
|
||
| Checking whether a field is in use does not by itself handle incompatible changes | ||
| to its type or meaning. See [New field in existing API version](api_changes.md#new-field-in-existing-api-version) | ||
| for guidance on changing or removing alpha fields. |
There was a problem hiding this comment.
This looks like an odd place to put this comment. Was it added here because of "removing alpha fields"?
That's not what field wiping does: it's about removing fields which are currently disabled.
There was a problem hiding this comment.
To be honest, I would simply leave out this comment. I'm not sure what it's trying to tell me besides pointing to some other doc.
| Checking whether a field is in use does not by itself handle incompatible changes | ||
| to its type or meaning. See [New field in existing API version](api_changes.md#new-field-in-existing-api-version) | ||
| for guidance on changing or removing alpha fields. |
There was a problem hiding this comment.
Leaving the file as-is seems better:
| Checking whether a field is in use does not by itself handle incompatible changes | |
| to its type or meaning. See [New field in existing API version](api_changes.md#new-field-in-existing-api-version) | |
| for guidance on changing or removing alpha fields. |
There was a problem hiding this comment.
I think I disagree. The point of all of these changes is to provide better guidance around how apiVersion works and how alpha fields work, and it seems relevant to call out that knowing that an alpha field is in use is not a guarantee that it hasn't changed.
It's true that the details are in another doc, but someone reading this has not automatically read the other. Nor will they necessarily have the other doc in mind.
There was a problem hiding this comment.
@kflynn: I simply don't understand what you are trying to say here.
What does "checking is in use" refer to? What does "handle incompatible changes" imply for alpha field?
Here's my interpretation:
- "checking in use" is part of the drop-disabled-field logic during updates. It's unrelated to changing the field type.
- "handle incompatible changes" is typically not something we do for alpha-quality fields. They can be changed from one release to another, even if that breaks version-skew scenarios, existing clusters, or out-dated consumers. That's what "alpha" means. We try to avoid it, but it's better to make such breaking changes early when a feature is still new instead of increasing technical debt or making the API more complex.
| An object may be accessed through multiple API versions. The `apiVersion` returned | ||
| by the server identifies the representation being returned, not the API version | ||
| used to create the object. For custom resources, it also does not identify the | ||
| release of the CRD that defined the schema. |
There was a problem hiding this comment.
At an architectural level, Kubernetes doesn't record the API version used at creation time. This new test isn't wrong; it could perhaps be even more emphatic.
There was a problem hiding this comment.
"The apiVersion returned by the server identifies the representation being returned, not the API version used to create the object – Kubernetes does not record the apiVersion used to create the object."
??
|
|
||
| An object may be accessed through multiple API versions. The `apiVersion` returned | ||
| by the server identifies the representation being returned, not the API version | ||
| used to create the object. For custom resources, it also does not identify the |
There was a problem hiding this comment.
| used to create the object. For custom resources, it also does not identify the | |
| used to create or store the object. For custom resources, it also does not identify the |
There was a problem hiding this comment.
I would just move to store. I don't think create is correct here, create doesn't matter as soon as the create API request is complete right?
There was a problem hiding this comment.
Having both is more clear to me.
There was a problem hiding this comment.
I'm fine with just "store". Otherwise we have to add "update" and "patch"...
Had a chat with @kflynn who shared a doc with their experiences, trying to capture what we can from there for forward guidance to folks