Apply fixes to in place support VPA AEP #5877
Conversation
Looks like the first PR merged a bit too early, while there were open coments
k8s-ci-robot
added
kind/cleanup
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jbartosik 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
| [in-place update feature](https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/1287-in-place-update-pod-resources) | ||
| [available in Kubernetes 1.27.](https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.27.md#api-change-3) | ||
| [in-place update feature]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/1287-in-place-update-pod-resources | ||
| [available in Kubernetes 1.27.]: https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.27.md#api-change-3 |
There was a problem hiding this comment.
@jbartosik can we mention the following link instead of the above link:
https://github.com/kubernetes/kubernetes/pull/102884
because it redirects to the Changelog of k8s v1.27.0-alpha.3, and we have to search the PR for the in-place update feature.
So I think we can mention the direct link of the PR.
There was a problem hiding this comment.
The changelog says that the feature is available in 1.27, which is what the doc is talking about in this place
In another place I link to KEP tracking the feature. I'm not sure if it's a good idea to link to the PR - the feature will (hopefully) change and go to beta so I'd rater refer to places that might but up to date
There was a problem hiding this comment.
Understood. Thanks, @jbartosik for the clarification.
There was a problem hiding this comment.
So can we change the link to the
https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.27.md#api-change-1
instead of api-change-3 it should api-change-1 because it is for the stable release i.e. k8s v1.27 and the corresponding text is mentioned there and not in the api-change-3 which is for v1.27.0-alpha.3. other than that is it mentioned under v1.27.0-alpha.2. IMO we should go with k8s v1.27, not with the pre-releases.
There was a problem hiding this comment.
@jbartosik, your thoughts?
|
/lgtm |
k8s-ci-robot
added
the
lgtm
/kind cleanup
/kind documentation
Apply comments from review of #5755 that I couldn't apply directly in the PR before it merged
If there are any other comments please leave them on either PR and I'll address them here.
#4016