VM support fixes #123

jrajahalme · 2021-03-12T20:08:29Z

Various changes to allow external workloads to be used when installing Cilium with the Cilium CLI tool:

Relax clustermesh validation on "enable"
Change default service type to NodePort on port 32379
Set etcd client Common Names to the corresponding user names
Add external-workload certs
Auto-detect datapath mode for Kind

These changes are required to run the updated GSG at cilium/cilium#15320.

jrajahalme · 2021-03-12T20:20:44Z

Fixed format.

tgraf

Looks awesome!

clustermesh/clustermesh.go

jrajahalme · 2021-03-18T02:09:51Z

@tgraf Updated 1st and 3rd commit as per your comments above, and added the last commit since your review, please re-review :-)

Allow enabling clustermesh with default cluster name and ID (zero) to allow external workloads to be used without explicitly setting cluster ID and/or name. Validate both local and remote cluster config fully when connecting to remote cluster instead. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

tklauser · 2021-03-18T09:31:10Z

@jrajahalme FYI, I've pushed two fixup commits addressing the staticcheck errors in the Go build GH action. You can apply them to the respective commits automatically by rebasing with git rebase --interactive --autosquash.

tklauser

Did an initial test of this on GKE using the docs in cilium/cilium#15320. Some of my review comments might be applicable here too cilium/cilium#15320 (review)

clustermesh/clustermesh.go

Add support for extracting service's ClusterIP and port rather than erroring out. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

…er type can not be used Error out if service type is not explicitly set or can not be auto-detected as LoadBalancer type. Warn if service type is set to HostPort. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

Client certificate's Common Name is used as etcd user account name once TLS based user auth (--client-cert-auth) is enabled. Use the user account names as CNs as follows: - Admin cert: root - Client cert: remote Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

Add a new cert to be used by External Workloads. Common Name is set to the etcd user account name that has write access to the registation key (externalworkload). Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

Auto-detect tunnel mode for Kind and disable kube-proxy replacement to be able to access NodePort services. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

This makes 'cilium clustermesh status' succeed with a warning message instead of failing when Cluster ID and/or Cluster Name has not been set when Cilium was installed. In that case warn like this: ✅ Service "clustermesh-apiserver" of type "NodePort" found ⚠️ Cluster not configured for clustermesh, use '--cluster-id' and '--cluster-name' with 'cilium install'. External workloads may still be configured. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

Add 'external-workload' (alias 'vm') subcommands to 'clustermesh': 'cilium clustermesh external-workload status' - Show the status of external workloads 'cilium clustermesh external-workload create <name...>' - Create new Cilium External Workload resource to allow a VM to join A new CEW resource with name <name> is created with a "default" namespace label. Options: '--namespace string' (alias '-n') Specify other than "default" as the namespace label '--labels' Pass a comma separated list of other labels for the identity of the external workload '--ipv4-alloc-cidr string' IPv4 allocation CIDR to be used instead the default picked by the VM (e.g., 10.15.0.0/30) '--ipv6-alloc-cidr string' IPv6 allocation CIDR to be used instead the default picked by the VM (e.g., f00d::a0f:0:0:0/126) 'cilium clustermesh external-workload delete <name...>' - Delete Cilium External Workload resources The named CEW resources will be deleted. External Workloads that have already registered may continue to communicate with the cluster, but may not rergister again. Options: '--all' Delete all CEW resources if none are named on the command line. 'cilium clustermesh external-workload install <file>' - Create an installation script to be used in external workloads to install or uninstall Cilium Write an installation script to the named file. Note that the script inlines the TLS credentials for external workload registration as well as the access details to the your k8s cluster. The file needs to be copied to the external workload (such as a VM) and executed there to install Cilium as a Docker container and connect to your k8s cluster. 'uninstall' parameter to the script will cause the script to uninstall Cilium from the external workload. All these commands require clustermesh to be enabled (via 'cilium clustermesh enable'). Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

jrajahalme · 2021-03-18T23:24:31Z

Tested with a GCP VM against GKE cluster, fixed the GSG PR by adding --config tunnel=vxlan. The generated install script needed fixes for:

Do not assume docker is suid root (use sudo explicitly in all docker invocations)
Use force option (-f) with mv and ln to avoid confusing sudo with a removed /etc/resolv.conf
Do not assume `/etc/resolv.conf', when a symbolic link, links to an absolute path.

…tunneling disabled As of now external workload installs rely on vxlan tunneling. Fail the install script generation if Cilium has tunneling disabled of not set to vxlan. In future consider testing with geneve and non-tunneled datapaths. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

jrajahalme · 2021-03-19T17:53:38Z

Added commit to fail external workload install script generation if Cilium datapath config in cluster is not using vxlan.

tklauser

I've just tested this with GKE and a GCE VM and it worked like a charm 🚀

There is one complaint from the staticcheck GitHub action, otherwise LGTM.

…slog Define $SUDO as an empty string if running as root. Use 'local' docker log driver to not depend on syslog. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

jrajahalme · 2021-03-20T17:01:07Z

Added one more commit: clustermesh: Allow VM install script to run as root, do not assume syslog

This allows the script to run as root and in environments where syslog is not available. Needed this to mock a VM with a Ubuntu 20.10 docker image, and running cilium agent inside as a "docker-in-docker" image.

jrajahalme · 2021-03-20T17:03:11Z

@tgraf It should be pretty straightforward to add a GH action run to test VM support using kind and docker. We have also successfully tested with GKE & GCP, would that be desirable for a GH CI run as well?

tklauser · 2021-03-20T19:07:44Z

@tgraf It should be pretty straightforward to add a GH action run to test VM support using kind and docker. We have also successfully tested with GKE & GCP, would that be desirable for a GH CI run as well?

I'll take care of adding GH actions to test VM support on GKE & GCP in a follow-up PR. I can try to add one covering kind & docker as well.

'make staticcheck' does not allow error messages starting with a capital letter, so do not use 'Cilium' to start an error message. Correctly spell 'DaemonSet' in error messages. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

jrajahalme · 2021-03-22T15:25:06Z

Fixed error message spelling and capitalization.

`cilium clustermesh status` was inadvertently broken by #123 when clusters have not been connected yet. Fix this by expliticly checking cluster's name and id instead of checking the number of connections. Add a warning about missing cluster name and/or id to `cilium clustermesh enable` with a note that external workloads do not need either. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

`cilium clustermesh status` was inadvertently broken by #123 when clusters have not been connected yet. Fix this by expliticly checking cluster's name and id instead of checking the number of connections. Add a warning about missing cluster name and/or id to `cilium clustermesh enable` with a note that external workloads do not need either. Fixes: #166 Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

`cilium clustermesh status` was inadvertently broken by #123 when clusters have not been connected yet. Fix this by expliticly checking cluster's name and id instead of checking the number of connections. Add the same warning about missing cluster name and/or id to `cilium clustermesh enable` as well with a note that external workloads do not need either. Fixes: #166 Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

`cilium clustermesh status` was inadvertently broken by cilium#123 when clusters have not been connected yet. Fix this by expliticly checking cluster's name and id instead of checking the number of connections. Add the same warning about missing cluster name and/or id to `cilium clustermesh enable` as well with a note that external workloads do not need either. Fixes: cilium#166 Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

`cilium clustermesh status` was inadvertently broken by cilium/cilium-cli#123 when clusters have not been connected yet. Fix this by expliticly checking cluster's name and id instead of checking the number of connections. Add the same warning about missing cluster name and/or id to `cilium clustermesh enable` as well with a note that external workloads do not need either. Fixes: #166 Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

jrajahalme added the kind/feature New feature or request label Mar 12, 2021

jrajahalme mentioned this pull request Mar 12, 2021

docs: Update VM support to use cilium-cli cilium/cilium#15320

Merged

jrajahalme force-pushed the vm-support branch from a49357c to e865ec2 Compare March 12, 2021 20:20

tgraf requested changes Mar 17, 2021

View reviewed changes

clustermesh/clustermesh.go Outdated Show resolved Hide resolved

clustermesh/clustermesh.go Outdated Show resolved Hide resolved

jrajahalme force-pushed the vm-support branch 2 times, most recently from d7f0f11 to a921cc4 Compare March 18, 2021 02:08

jrajahalme requested a review from tgraf March 18, 2021 02:08

jrajahalme force-pushed the vm-support branch from a921cc4 to 3789814 Compare March 18, 2021 02:14

jrajahalme force-pushed the vm-support branch from 3789814 to 97746f6 Compare March 18, 2021 02:24

jrajahalme requested a review from tklauser March 18, 2021 02:26

tklauser force-pushed the vm-support branch from 2ea14f7 to 068ba33 Compare March 18, 2021 09:38

tklauser reviewed Mar 18, 2021

View reviewed changes

clustermesh/clustermesh.go Outdated Show resolved Hide resolved

clustermesh/clustermesh.go Show resolved Hide resolved

jrajahalme added 2 commits March 18, 2021 10:57

clustermesh: Add support for ClusterIP

0fd940e

Add support for extracting service's ClusterIP and port rather than erroring out. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

jrajahalme force-pushed the vm-support branch from 068ba33 to ecbb0e2 Compare March 18, 2021 18:18

jrajahalme added 5 commits March 18, 2021 16:09

clustermesh: Add support for external-workload cert

843200b

Add a new cert to be used by External Workloads. Common Name is set to the etcd user account name that has write access to the registation key (externalworkload). Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

install: Auto-detect datapath mode for Kind

17ebab0

Auto-detect tunnel mode for Kind and disable kube-proxy replacement to be able to access NodePort services. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

jrajahalme force-pushed the vm-support branch from ecbb0e2 to e5d594e Compare March 18, 2021 23:19

jrajahalme requested a review from tklauser March 19, 2021 17:52

tklauser approved these changes Mar 19, 2021

View reviewed changes

clustermesh: Allow VM install script to run as root, do not assume sy…

baaa86e

…slog Define $SUDO as an empty string if running as root. Use 'local' docker log driver to not depend on syslog. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

jrajahalme force-pushed the vm-support branch from 171e948 to baaa86e Compare March 20, 2021 16:58

clustermesh: Fix error capitalization and typos

98283cd

'make staticcheck' does not allow error messages starting with a capital letter, so do not use 'Cilium' to start an error message. Correctly spell 'DaemonSet' in error messages. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>

tgraf approved these changes Mar 25, 2021

View reviewed changes

tklauser merged commit ac78a56 into master Mar 25, 2021

tklauser deleted the vm-support branch March 25, 2021 11:23

jrajahalme mentioned this pull request Apr 15, 2021

clustermesh: cilium clustermesh status fails if clusters are not connected yet #166

Closed

jrajahalme mentioned this pull request Apr 20, 2021

clustermesh: Fix clustermesh status when clusters are not connected #178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VM support fixes #123

VM support fixes #123

jrajahalme commented Mar 12, 2021

jrajahalme commented Mar 12, 2021

tgraf left a comment

jrajahalme commented Mar 18, 2021

tklauser commented Mar 18, 2021

tklauser left a comment

jrajahalme commented Mar 18, 2021

jrajahalme commented Mar 19, 2021

tklauser left a comment

jrajahalme commented Mar 20, 2021

jrajahalme commented Mar 20, 2021

tklauser commented Mar 20, 2021

jrajahalme commented Mar 22, 2021

VM support fixes #123

VM support fixes #123

Conversation

jrajahalme commented Mar 12, 2021

jrajahalme commented Mar 12, 2021

tgraf left a comment

Choose a reason for hiding this comment

jrajahalme commented Mar 18, 2021

tklauser commented Mar 18, 2021

tklauser left a comment

Choose a reason for hiding this comment

jrajahalme commented Mar 18, 2021

jrajahalme commented Mar 19, 2021

tklauser left a comment

Choose a reason for hiding this comment

jrajahalme commented Mar 20, 2021

jrajahalme commented Mar 20, 2021

tklauser commented Mar 20, 2021

jrajahalme commented Mar 22, 2021