-
Notifications
You must be signed in to change notification settings - Fork 705
templates: k8s: support multi-node cluster #4136
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
base: master
Are you sure you want to change the base?
Conversation
``` limactl start --name k8s-0 --network lima:user-v2 template:k8s limactl shell k8s-0 sudo kubeadm token create --print-join-command limactl start --name k8s-1 --network lima:user-v2 --set '.param.kubeadmInit="false"' template:k8s limactl shell k8s-1 sudo <JOIN_COMMAND_FROM_ABOVE> ``` Fix issue 4100 Signed-off-by: Akihiro Suda <[email protected]>
hint: | | ||
See "/var/log/cloud-init-output.log" in the guest | ||
- description: "kubernetes images to be pulled" | ||
# FIXME: probes[].description doesn't seem to recognize .Param.kubeadmInit |
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.
FIXME.
Ideally it should be possible to wrap the entire probe entry in {{if}}
param: | ||
# Set to false for `kubeadm join` | ||
kubeadmInit: true | ||
# FIXME: message doesn't seem to recognize .Param.kubeadmInit |
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.
FIXME
- guest: "{{if eq .Param.kubeadmInit \"true\"}}/etc/kubernetes/admin.conf{{else}}/dev/null{{end}}" | ||
host: "{{if eq .Param.kubeadmInit \"true\"}}{{.Dir}}/copied-from-guest/kubeconfig.yaml{{else}}{{.Dir}}/copied-from-guest/null{{end}}" | ||
deleteOnStop: true | ||
# FIXME: set .portForwards[].ignore when kubeadmInit is false |
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.
FIXME. ignore
doesn't seem to support `{{if}}.
# FIXME: set .portForwards[].ignore when kubeadmInit is false | ||
param: | ||
# Set to false for `kubeadm join` | ||
kubeadmInit: true |
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.
Bikeshedding: kubeadmInit
or KUBEADM_INIT
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.
Sorry, only drive-by commenting because I saw the email notification:
I think we should use all uppercase names similar to environment variables for template-specific parameters. They also stand out much better in scripts, and look better if you use $PARAM_KUBEADM_INIT
.
I prefer to use the shell variables whenever possible because they remain visible in the cloud-init-output.log
whereas Go templates are just replaced, and the variable name is lost.
This also leaves lower-case params for use later as "global" conventions for stuff that applies to different templates. I don't expect that mixed-case names will have special meaning though, so the decision should be based on usability and aesthetics.
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.
all uppercase names similar to environment variables
These names can be easily confused as env vars?
docker.yaml
isUsingThisCase:
Lines 96 to 97 in c71bc48
param: | |
containerdSnapshotter: true |
copyToHost: | ||
- guest: "/etc/kubernetes/admin.conf" | ||
host: "{{.Dir}}/copied-from-guest/kubeconfig.yaml" | ||
- guest: "{{if eq .Param.kubeadmInit \"true\"}}/etc/kubernetes/admin.conf{{else}}/dev/null{{end}}" |
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.
This hacky workaround is needed because copyToHost
cannot be wrapped in {{if}}
Fix #4100