Skip to content

KEP-3104: Introduce kuberc - update for 1.31 #4705

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

Merged
merged 1 commit into from
Jun 10, 2024
Merged

KEP-3104: Introduce kuberc - update for 1.31 #4705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 54 additions & 21 deletions keps/sig-cli/3104-introduce-kuberc/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,13 @@ Items marked with (R) are required *prior to targeting to a milestone / release*
- [x] (R) Design details are appropriately documented
- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input (including test refactors)
- [ ] e2e Tests for all Beta API Operations (endpoints)
- [ ] (R) Ensure GA e2e tests for meet requirements for [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md)
- [ ] (R) Ensure GA e2e tests for meet requirements for [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md)
- [ ] (R) Minimum Two Week Window for GA e2e tests to prove flake free
- [ ] (R) Graduation criteria is in place
- [ ] (R) [all GA Endpoints](https://github.com/kubernetes/community/pull/1806) must be hit by [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md)
- [ ] (R) Production readiness review completed
- [ ] (R) Production readiness review approved
- [ ] "Implementation History" section is up-to-date for milestone
- [ ] (R) [all GA Endpoints](https://github.com/kubernetes/community/pull/1806) must be hit by [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md)
- [x] (R) Production readiness review completed
- [x] (R) Production readiness review approved
- [x] "Implementation History" section is up-to-date for milestone
- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes

Expand Down Expand Up @@ -179,7 +179,7 @@ updates.
[documentation style guide]: https://github.com/kubernetes/community/blob/master/contributors/guide/style-guide.md
-->

This proposal introduces an optional `kuberc` file that is used to separate cluster credentials and server configuration from user preferences.
This proposal introduces an optional `kuberc` file that is used to separate cluster credentials and server configuration from user preferences.

## Motivation

Expand All @@ -191,9 +191,11 @@ demonstrate the interest in a KEP within the wider Kubernetes community.

[experience reports]: https://github.com/golang/go/wiki/ExperienceReports
-->
kubectl is one of the oldest parts of the Kubernetes project and has a strong guarantee on backwards compatibility. We want users to be able to opt in to new behaviors (like delete confirmation) that may be considered breaking changes for existing CI jobs and scripts.
kubectl is one of the oldest parts of the Kubernetes project and has a strong guarantee on backwards compatibility. We want users to be able to opt in to new behaviors (like delete confirmation) that may be considered breaking changes for existing CI jobs and scripts.

[kubeconfigs already have a field for preferences](https://github.com/kubernetes/kubernetes/blob/474fd40e38bc4e7f968f7f6dbb741b7783b0740b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go#L43) that is currently underutilized. The reason for not using this existing field is that creating a new cluster generally yields a new kubeconfig file which contains credentials and host information. While kubeconfigs can be merged and specified by path, we feel there should be a clear separation between server configuration and user preferences.
[kubeconfig already has a field for preferences](https://github.com/kubernetes/kubernetes/blob/474fd40e38bc4e7f968f7f6dbb741b7783b0740b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go#L43) that is currently underutilized. The reason for not using this existing field is that creating a new cluster generally yields a new kubeconfig file which contains credentials and host information. While kubeconfigs can be merged and specified by path, we feel there should be a clear separation between server configuration and user preferences.

Additionally, users can split kubeconfig files into various locations, while maintaining a single preference file that will apply no matter which `--kubeconfig` flag or `$KUBECONFIG` env var is pointing to.

### Goals

Expand Down Expand Up @@ -303,7 +305,7 @@ required) or even code snippets. If there's any ambiguity about HOW your
proposal will be implemented, this is the place to discuss them.
-->

During alpha this feature will be enabled through the `ENABLE_KUBERC=true` environment variable. The file will default to being located in `~/.kube/kuberc`. A flag will allow overriding this default location with a path i.e. `kubectl --kuberc /var/kube/rc`.
During alpha this feature will be enabled through the `KUBECTL_KUBERC=true` environment variable. The file will default to being located in `~/.kube/kuberc`. A flag will allow overriding this default location with a path i.e. `kubectl --kuberc /var/kube/rc`.

Three initial top level keys are proposed.

Expand All @@ -324,16 +326,21 @@ kind: Preferences
command:
aliases:
- alias: getdbprod
command: get pods -l what=database --namespace us-2-production

command: get pods
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Would it make sense to also change the L:343 to interactive to not mislead people?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, this was written before we had settled on the interactive flag for delete I believe so updating that to match would be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I missed it - fixed in latest push.

flags:
- name: labels
default: what=database
- name: namespace
default: us-2-production

overrides:
- command: apply
flags:
- name: server-side
default: "true"
- command: delete
flags:
- name: confirm
- name: interactive
default: "true"
- command: "*"
flags:
Expand Down Expand Up @@ -387,10 +394,16 @@ https://testgrid.k8s.io/sig-testing-canaries#ci-kubernetes-coverage-unit

This can inform certain test coverage improvements that we want to do before
extending the production code to implement this enhancement.
-->

- `<package>`: `<date>` - `<test coverage>`

-->

We're planning unit tests covering:
- basic functionality
- config API fuzzy tests
- input validation and correctness

##### Integration tests

<!--
Expand All @@ -399,10 +412,15 @@ For Alpha, describe what tests will be added to ensure proper quality of the enh

For Beta and GA, add links to added tests together with links to k8s-triage for those tests:
https://storage.googleapis.com/k8s-triage/index.html
-->

- <test>: <link to test coverage>

-->

We're planning at least the following integration tests:
- `KUBECTL_KUBERC` enablement and disablement
- basic functionality

##### e2e tests

<!--
Expand Down Expand Up @@ -483,7 +501,7 @@ in back-to-back releases.

#### Alpha

- Initial implementation behind environment variable.
- Initial implementation behind `KUBECTL_KUBERC` environment variable.

#### Beta

Expand All @@ -492,7 +510,7 @@ in back-to-back releases.

#### GA

- Allowing time for feedback.
- Address feedback.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should add e2e test condition at least force ourselves adding more tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we get any buy-in from using e2e. We can safely assume that if both unit and integration tests are covering all the paths I wouldn't be so strong on the e2e. We only need a running apiserver, and that is available in integration tests. But I don't object 😉



### Upgrade / Downgrade Strategy
Expand Down Expand Up @@ -572,7 +590,7 @@ well as the [existing list] of feature gates.
- Feature gate name:
- Components depending on the feature gate:
- [x] Other
- Describe the mechanism: The environment variable `ENABLE_KUBERC=true`.
- Describe the mechanism: The environment variable `KUBECTL_KUBERC=true`.
- Will enabling / disabling the feature require downtime of the control
plane? No.
- Will enabling / disabling the feature require downtime or reprovisioning
Expand Down Expand Up @@ -701,10 +719,10 @@ Recall that end users cannot usually observe component logs or access metrics.
-->

- [ ] Events
- Event Reason:
- Event Reason:
- [ ] API .status
- Condition name:
- Other field:
- Condition name:
- Other field:
- [x] Other (treat as last resort)
- Details: The command will be logged with kubectl -v 2

Expand Down Expand Up @@ -739,7 +757,7 @@ Pick one more of these and delete the rest.
- Components exposing the metric:
- [ ] Other (treat as last resort)
- Details:
-
-
Not applicable.

###### Are there any missing metrics that would be useful to have to improve observability of this feature?
Expand Down Expand Up @@ -862,6 +880,20 @@ This through this both in small and large cases, again with respect to the

No.

###### Can enabling / using this feature result in resource exhaustion of some node resources (PIDs, sockets, inodes, etc.)?

<!--
Focus not just on happy cases, but primarily on more pathological cases
(e.g. probes taking a minute instead of milliseconds, failed pods consuming resources, etc.).
If any of the resources can be exhausted, how this is mitigated with the existing limits
(e.g. pods per node) or new limits added by this KEP?

Are there any tests that were run/should be run to understand performance characteristics better
and validate the declared limits?
-->

No.

### Troubleshooting

<!--
Expand Down Expand Up @@ -909,6 +941,7 @@ Major milestones might include:

* 2021-06-02: [Proposal to add delete confirmation](https://github.com/kubernetes/enhancements/issues/2775)
* 2022-06-13: This KEP created.
* 2024-06-07: Update KEP with new env var name and template.

## Drawbacks

Expand Down
20 changes: 13 additions & 7 deletions keps/sig-cli/3104-introduce-kuberc/kep.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@ title: Introduce kuberc
kep-number: 3104
authors:
- "@eddiezane"
- "@soltysh"
owning-sig: sig-cli
participating-sigs:
- sig-cli
status: implementable
creation-date: 2022-06-13
reviewers:
- "@soltysh"
- "@liggitt"
approvers:
- "@eddiezane"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Eddie is in approvers and reviewers list, could you please add me in authors list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll drop Eddie from approvers, and since I joined co-authors you'll be the sole approver for that KEP. I think that makes the most sense.

- "@soltysh"
- "@mpuckett159"
approvers:
- "@ardaguclu"

see-also:
- https://github.com/kubernetes/enhancements/issues/2775
Expand All @@ -23,17 +26,20 @@ stage: alpha
# The most recent milestone for which work toward delivery of this KEP has been
# done. This can be the current (upcoming) milestone, if it is being actively
# worked on.
latest-milestone: "v1.25"
latest-milestone: "v1.31"

# The milestone at which this feature was, or is targeted to be, at each stage.
milestone:
alpha: "v1.25"
beta: "v1.27"
stable: "v1.28"
alpha: "v1.31"
beta: ""
stable: ""

# The following PRR answers are required at alpha release
# List the feature gate name and the components for which it must be enabled
feature-gates: []
feature-gates:
- name: KUBECTL_KUBERC
components:
- kubectl
Comment on lines +39 to +42
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an environment variable, not a feature gate.

I'd like feature gates to only mean https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/; if we want kubectl feature flags listed there, let's first change that page to list them, or put up a separate page (the two pages could link to each other).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kep.yaml template doesn't have an option for other, like we have in main document. I don't want to break automation build around that bit 😅

disable-supported: true

# The following PRR answers are required at beta release
Expand Down