KEP-3104: Introduce kuberc - update for 1.31 #4705
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
k8s-ci-robot
added
approved
| @@ -324,8 +326,13 @@ kind: Preferences | |||
| command: | |||
| aliases: | |||
| - alias: getdbprod | |||
| command: get pods -l what=database --namespace us-2-production | |||
|
|
|||
| command: get pods | |||
There was a problem hiding this comment.
nit: Would it make sense to also change the L:343 to interactive to not mislead people?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Oh, I missed it - fixed in latest push.
| @@ -492,7 +499,7 @@ in back-to-back releases. | |||
|
|
|||
| #### GA | |||
|
|
|||
| - Allowing time for feedback. | |||
| - Address feedback. | |||
There was a problem hiding this comment.
Do you think we should add e2e test condition at least force ourselves adding more tests?
There was a problem hiding this comment.
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 😉
| @@ -10,8 +10,10 @@ creation-date: 2022-06-13 | |||
| reviewers: | |||
| - "@soltysh" | |||
| - "@liggitt" | |||
| - "@eddiezane" | |||
There was a problem hiding this comment.
Since Eddie is in approvers and reviewers list, could you please add me in authors list?
There was a problem hiding this comment.
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.
| feature-gates: | ||
| - name: KUBECTL_KUBERC | ||
| components: | ||
| - kubectl |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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 😅
soltysh
force-pushed
the
update_preferences
branch
from
1cf93fc to
0773162
Compare
|
@ardaguclu @mpuckett159 updated /assign @johnbelamaric |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
@soltysh you probably should have held this if you wanted me to take a look :) All good though, LGTM |
Sorry about that 😞 I thought I have. After this small hiccup I'm very careful when approving PRs and adding hold automatically 😉 |
One-line PR description: Update kuberc document.
Issue link: Separate kubectl user preferences from cluster configs #3104
/assign @ardaguclu @eddiezane