KEP-2590: graduate kubectl subresource support to beta #3729

nikhita · 2023-01-12T12:45:20Z

One-line PR description:
We’ve received good feedback for this feature and would like to move it to beta in v1.27. Some examples of feedback:

Issue link: #2590

/assign @soltysh
cc @MadhavJivrajani

rikatz · 2023-01-12T13:00:14Z

User feedback here:

This feature is a must and has been a life saver for a while to me. Previously I've been using kubectl-edit_status plugin, and as soon as this feature was released on kubectl I've just migrated directly to it.

I have faced no problem so far, or inconsistency on the behavior. Just using kubectl --subresource=status has been enough for the majority of my daily tasks :)

Nice work folks!

soltysh

I'd also suggest updating the KEP to use the new template https://github.com/kubernetes/enhancements/blob/master/keps/NNNN-kep-template/README.md

soltysh · 2023-01-12T15:43:28Z

keps/sig-cli/2590-kubectl-subresource/README.md

+
+#### Beta
+
+- Add the `--subresource` flag to the apply command.


I'm not quite convinced we want to support --subresource in apply command, given its general purpose of being a declarative command, which we covered in this presentation https://youtu.be/eOxy1PS5TyQ?t=780.

I'm not quite convinced we want to support --subresource in apply command

It's also not straightforward to implement. Ref: #2600 (comment), #2600 (comment)

Is it ok to move this KEP to beta without adding --subresource support for apply?

Although, then I'm not sure what "beta" or "GA" for this feature would even mean 🤔

I'm not quite convinced we want to support --subresource in apply command

It's also not straightforward to implement. Ref: #2600 (comment), #2600 (comment)

Is it ok to move this KEP to beta without adding --subresource support for apply?

It's totally fine, just add a note, that due to additional complexity and the nature of kubectl apply command we've decided not to expand it with this flag.

Although, then I'm not sure what "beta" or "GA" for this feature would even mean thinking

You can promote to beta, w/o any test, which can be added during promotion, or after. For GA we need tests covering every are, ideally unit, integration (cmd) and e2e.

Addressed feedback and updated to the new KEP template.

soltysh

one nit, but otherwise this lgtm

soltysh · 2023-01-12T17:12:33Z

keps/sig-cli/2590-kubectl-subresource/README.md

+
+#### GA
+
+- e2e tests are added.


These tests are required for GA, so you want them being added at beta stage, since that will be one of reqs for GA promotion.

nikhita · 2023-01-12T17:28:25Z

/assign @johnbelamaric

soltysh

/lgtm
/approve
from sig-cli pov

BenTheElder · 2023-01-30T22:59:29Z

Hi @nikhita, Production Readiness Review Shadows are going to do first pass PRR reviews this cycle (recently announced here), I'll be shadowing this KEP and @johnbelamaric will be following later with a follow-up review and PRR approval.

BenTheElder · 2023-01-30T23:52:33Z

keps/sig-cli/2590-kubectl-subresource/README.md

+#### Beta
+
+- Gather feedback from users.
+- e2e tests are added.


BenTheElder · 2023-01-30T23:53:21Z

keps/sig-cli/2590-kubectl-subresource/README.md

+#### Alpha

- [ ] Collect user feedback on adding support of `--subresource` for `apply`
+- Add the `--subresource` flag to get, patch, edit and replace commands.


Also unit tests?

Added unit and integration tests.

soltysh

/lgtm

johnbelamaric · 2023-02-07T19:21:27Z

In alpha, looks like we didn't make you set an env var in order to make the flag valid. Usually we do that now for kubectl. It is essentially a positive acknowledgement by the user that "I know I am using an alpha feature".

That's fine, no change needed now. But in the future that would be the right approach.

/approve

k8s-ci-robot · 2023-02-07T19:21:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnbelamaric, nikhita, soltysh

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:

~~keps/prod-readiness/OWNERS~~ [johnbelamaric]
~~keps/sig-cli/OWNERS~~ [johnbelamaric,soltysh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

soltysh · 2023-02-08T10:09:19Z

In alpha, looks like we didn't make you set an env var in order to make the flag valid. Usually we do that now for kubectl. It is essentially a positive acknowledgement by the user that "I know I am using an alpha feature".

Yeah, that's my bad, I usually ensure that we have it behind an env var 😞 Sorry for that.

k8s-ci-robot assigned soltysh Jan 12, 2023

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 12, 2023

k8s-ci-robot requested a review from KnVerey January 12, 2023 12:45

k8s-ci-robot added the kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory label Jan 12, 2023

k8s-ci-robot requested a review from soltysh January 12, 2023 12:45

k8s-ci-robot added sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 12, 2023

nikhita mentioned this pull request Jan 12, 2023

Add subresource support to kubectl #2590

Closed

12 tasks

nikhita force-pushed the subresource-kep-beta branch 2 times, most recently from 29703cf to 12ec0b6 Compare January 12, 2023 12:53

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 12, 2023

nikhita force-pushed the subresource-kep-beta branch from 12ec0b6 to dba604b Compare January 12, 2023 12:57

soltysh requested changes Jan 12, 2023

View reviewed changes

nikhita force-pushed the subresource-kep-beta branch from dba604b to d0d40b5 Compare January 12, 2023 16:37

soltysh approved these changes Jan 12, 2023

View reviewed changes

nikhita force-pushed the subresource-kep-beta branch from d0d40b5 to a3b7f20 Compare January 12, 2023 17:28

k8s-ci-robot assigned johnbelamaric Jan 12, 2023

soltysh approved these changes Jan 13, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 13, 2023

BenTheElder reviewed Jan 30, 2023

View reviewed changes

Graduate kubectl subresource support to beta

505daaa

nikhita force-pushed the subresource-kep-beta branch from a3b7f20 to 505daaa Compare February 3, 2023 05:30

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2023

soltysh approved these changes Feb 3, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 7, 2023

k8s-ci-robot merged commit a86925e into kubernetes:master Feb 7, 2023

k8s-ci-robot added this to the v1.27 milestone Feb 7, 2023

ardaguclu mentioned this pull request Sep 26, 2024

add --subresource to kubectl apply kubernetes/kubernetes#127634

Merged


		#### Beta

		- Add the `--subresource` flag to the apply command.

KEP-2590: graduate kubectl subresource support to beta #3729

KEP-2590: graduate kubectl subresource support to beta #3729

Conversation

nikhita commented Jan 12, 2023

Uh oh!

rikatz commented Jan 12, 2023

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikhita commented Jan 12, 2023

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

BenTheElder commented Jan 30, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

johnbelamaric commented Feb 7, 2023

Uh oh!

k8s-ci-robot commented Feb 7, 2023

Uh oh!

soltysh commented Feb 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants