Update kubeadm v1alpha3 example configuration #68836
Conversation
k8s-ci-robot
added
the
release-note-none
k8s-ci-robot
added
the
size/M
k8s-ci-robot
added
cncf-cla: yes
|
/kind bug |
k8s-ci-robot
added
kind/bug
k8s-ci-robot
added
the
approved
timothysc
added
priority/critical-urgent
timothysc
added
the
approved
| // apiServerCertSANs: | ||
| // - "10.100.1.1" | ||
| // - "ec2-10-100-0-1.compute-1.amazonaws.com" | ||
| // certificateDirectory: "/etc/kubernetes/pki" | ||
| // certificatesDir: "/etc/kubernetes/pki" |
| // imageRepository: "k8s.gcr.io" | ||
| // unifiedControlPlaneImage: "k8s.gcr.io/controlplane:v1.12.0" | ||
| // auditPolicyConfiguration: | ||
| // auditPolicy: |
There was a problem hiding this comment.
[1]
yep, these names do not match the struct types and are misleading. :
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/apis/kubeadm/v1alpha3/types.go#L116
my vote would be to make them match.
| // the preferred way to configure kubeadm is to pass a YAML file in with the | ||
| // --config option. | ||
| // the preferred way to configure kubeadm is to pass a single YAML file with | ||
| // multiple configuration types in with the --config option. |
There was a problem hiding this comment.
Although in the future the users may have the choice to store different kinds in different YAML files, currently we don't. So this must be single YAML file and we should emphasize on that here.
Also, InitConfiguration and JoinConfiguration cannot share a single YAML file at the moment (kubeadm bails out). So we should probably mention that too. Not sure for ClusterConfiguration and JoinConfiguration though.
There was a problem hiding this comment.
Not sure for ClusterConfiguration and JoinConfiguration though.
yeah, we probably want to handle all scenarios.
to exclude the possibility of related k/kubeadm issue reports, that is.
There was a problem hiding this comment.
i'll add some additional text to catch edge cases.
| // | ||
| // A fully populated example of the schema: | ||
| // A fully populated example of a configuration file: |
There was a problem hiding this comment.
Perhaps s/a configuration/an init configuration ?
There was a problem hiding this comment.
init configuration is a little misleading as we have an object called "InitConfiguration" and "ClusterConfiguration" when a cluster configuration is actually the useful thing to use during kubeadm init, but we could use both.
I think configuration file gets the meaning across while avoiding this confusion, but if you feel strongly I can change it.
There was a problem hiding this comment.
My idea here is not to confuse a user, that JoinConfiguration can be appended to this file. So if we manage to cover that case with some kind of word shuffling in either this or the previous paragraph, I am totally fine with it.
| // hostPath: "/etc/some-path" | ||
| // mountPath: "/etc/some-pod-path" | ||
| // writable: true | ||
| // pathType: File | ||
| // apiServerCertSANs: |
There was a problem hiding this comment.
this sort of looks like it should be indented one level less?
There was a problem hiding this comment.
Actually this whole section's indentation is off
There was a problem hiding this comment.
yes, it totally is, good 👀
| // imageRepository: "k8s.gcr.io" | ||
| // unifiedControlPlaneImage: "k8s.gcr.io/controlplane:v1.12.0" | ||
| // auditPolicyConfiguration: | ||
| // auditPolicy: |
| // - token: "9a08jv.c0izixklcxtmnze7" | ||
| // description: "kubeadm bootstrap token" | ||
| // ttl: "24h" | ||
| // usages: |
There was a problem hiding this comment.
Since both the "usages" and "groups" fields have the default values here, it would be more helpful to show that it's not necessary to specify these details for a bootstrap token, and maybe include a second token that does require these fields be specified with different, non-default values.
There was a problem hiding this comment.
I'm with you, +1 for sure
chuckha
force-pushed
the
update-godoc
branch
2 times, most recently
from
104219a to
6c13c60
Compare
| // * ClusterConfiguration | ||
| // | ||
| // InitConfiguration and JoinConfiguration cannot share a single YAML file, | ||
| // however it is expected thatInitConfiguration and ClusterConfiguration will |
There was a problem hiding this comment.
space after that, before Init...
closes kubernetes/kubeadm#1132 Signed-off-by: Chuck Ha <ha.chuck@gmail.com>
chuckha
force-pushed
the
update-godoc
branch
from
6c13c60 to
7ddc873
Compare
|
we are probably going to get ❓ from users regardless, but this is the right direction. |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chuckha, neolit123, timothysc 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 |
|
/test pull-kubernetes-kubemark-e2e-gce-big |
|
/retest |
1 similar comment
|
/retest |
|
/test pull-kubernetes-e2e-gce-device-plugin-gpu |
|
/retest |
closes kubernetes/kubeadm#1132
Signed-off-by: Chuck Ha ha.chuck@gmail.com
What this PR does / why we need it:
This updates the godoc example.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes kubernetes/kubeadm#1132
Special notes for your reviewer:
This is easy to typo and has no test/validation as it's a comment. Please double check typos.
/assign @timothysc
/cc @liztio @rosti @neolit123