-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Add documents for kuberc #50913
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.
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
Add documents for kuberc #50913
Conversation
@ardaguclu: GitHub didn't allow me to request PR reviews from the following users: sftim. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Several nits, but mostly lgtm
Relevant to kubernetes/enhancements#5300 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
from technical pov, it will be updated to beta at a later stage as part of kubernetes/enhancements#3104 promotion (ref kubernetes/enhancements#5300)
LGTM label has been added. Git tree hash: 35c4172d2b68b19002ebc212338c977303f1e65a
|
/cc @tengqm |
/assign @divya-mohan0209 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could merge this, but I do have corrective feedback to offer.
Aside: once we document writing and using a kubeconfig, we should also document writing and using kuberc.
## Disable Kuberc | ||
|
||
To temporarily disable the kuberc functionality, simply export the environment variable `KUBERC` with the value `off`: | ||
|
||
```shell | ||
export KUBERC=off | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given it's an alpha feature, couldn't you just unset something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've mentioned disabling via feature gate as well. Please let me know what you think.
The config API for kuberc is auto-generated using the |
Great idea, I wasn't even aware we have that issue. |
So the expectation is we'll merge this before kubernetes/kubernetes#131818. Once kubernetes/kubernetes#131818 merges, we'll update the documents accordingly? |
This PR should document what's already in released Kubernetes (as alpha). We'll need a new, separate update PR ahead of cutting a release that includes the changes from kubernetes/kubernetes#131818 |
Correct, the beta updates will be part of 1.34, and as usual the changes in the linked PR will be submitted there. |
@lmktfy is there any pending action from me?. I'm wondering maybe we can merge this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
I'd like to see more fixes, but this looks OK to merge as-is.
LGTM label has been added. Git tree hash: c01b3842fb5743e404b1b458a74e702e494d04c8
|
@lmktfy should I apply your changes or ping someone for approval? |
let's get the suggested fixes applied right away, otherwise we'll loose track of them |
An approver is welcome to merge this and then open one or more good-first-issue issues (we can always use a supply of these, and k/website likes to have very low difficulty first issues). Applying fixes? Also fine. |
I can apply the changes tomorrow.@lmktfy as far as I can see, you are also the approver? |
I've updated the PR based on the review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/LGTM
If we merge this, the docs will be better than if we did not.
compiled-in default value. | ||
{{< /note >}} | ||
|
||
#### Example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we don't end headings with punctuation.
Try this
#### Example: | |
#### Example {#overrides-example} |
Within a `kuberc`, configuration, command line arguments are termed _flags_ (even if they do not represent a boolean type). | ||
You can use `flags` to set the default value of a command line argument. | ||
|
||
If you explicitly specify a flag on your terminal, explicit value will always take precedence over the value you defined in kuberc using `overrides`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why indent this? That doesn't look right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM label has been added. Git tree hash: 1fd1cbcf38c9a82d2bb44cadae1fa8d7f538d1cd
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: katcosgrove, 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:
Approvers can indicate their approval by writing |
This PR adds documentations for the kuberc (KEP issue: kubernetes/enhancements#3104).
This PR is complementary of this #50961.