Automatic installation of Calico on workload clusters #14

hama-25179 · 2023-01-24T00:50:27Z

What this PR does / why we need it:
If CNI is not installed, the sample HelmChartProxy of nginx-ingress will not work, so I want to install CNI automatically.
Registering HelmChartProxy of calico as a Tilt resource, developers will no longer have to worry about CNI.

Additions and modifications

develop.md: Add a calico auto-install configuration to the development docs.
calico-cni.yaml: Modify a HelmChartProxy manifest of calico to target all workload clusters.
setup-calico-autoinstallation.sh: Create a shell script to register HelmChartProxy manifest of calico as a Tilt resource.

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 #8

linux-foundation-easycla · 2023-01-24T00:50:31Z

The committers listed above are authorized under a signed CLA.

✅ login: hama-25179 / name: Disuke Hamashima (1edbd16)

k8s-ci-robot · 2023-01-24T00:50:35Z

Welcome @hama-25179!

It looks like this is your first PR to kubernetes-sigs/cluster-api-addon-provider-helm 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-addon-provider-helm has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

Jont828

Thanks for you contributions! Please sign the CLA when you have time in order to contribute to K8s open source projects.

Jont828 · 2023-01-25T02:57:10Z

config/samples/calico-cni.yaml

+  #  matchLabels:
+  #    calicoCNI: enabled
+  # Target all workload clusters.
+    matchExpressions:


The prototype at the moment doesn't actually support matchExpressions as it only passes in the value of matchLabels. I think it would make sense to do so but need to verify that it would work with the client as I'm passing in the fields individually. Would you mind opening an issue to log that we want to add support for the other label selector fields?

I see. I'll open the issue.
Since the goal is to target all workload clusters, should I change it to the following instead of using matchExpressions?
(This one is much simpler)

# Target all workload clusters. matchLabels: {}

I created the issue #15.

Thanks for opening the issue! And let me double check about that -- I'm not certain if an empty label selector would select all clusters or no clusters.

I have confirmed that if the "HelmChartProxy" matchlabels setting is set to empty, the matching is unconditional.
Since it is a rather common Kubernetes setting to set the label selector to an empty setting for unconditional matching, I'd like to change the setting.

Sure, I think that seems reasonable. Just to clarify, you're suggesting that empty labels should select everything like a wildcard or * sign?

That is correct.

Also, the URL for calico helm repo seems to have changed, so I updated the YAML.
(I intended to fix it in a222e9c)

https://docs.tigera.io/calico/3.25/getting-started/kubernetes/helm#download-the-helm-chart

Jont828 · 2023-01-25T02:59:30Z

hack/setup-calico-autoinstallation.sh

+# Check and create directories.
+[ -d "${CAPI_DIR}" ] && [ ! -d "${TILTD_DIR}" ] && mkdir -p ${TILTD_DIR}
+
+# Generate the calico_tiltfile.


This file seems reasonable to me and I'll test it out on my end when I have some time. I wonder if we should instead set up a separate Tiltfile for CAAPH instead and include this in there instead.

Is my understanding correct that prepare the Tiltfile in advance and copy it to tilt.d directory?.

So the Tiltfile is where we would run tilt up from. Currently, we're running it from a clone of the Cluster API repo, but for other providers like Cluster API Provider for Azure, it has its own Tiltfile and doesn't require CAPI to be present.

It might be worth bringing up in the CAPI office hours on how we could handle that.

I'm with you. Let us discuss it during the CAPI office hours.

@hama-25179 I tested it out and it seems to work, just make sure you run chmod +x setup-calico-autoinstallation.sh and commit that to the branch.

Thank you for testing. I set executable permission to the script.

hama-25179 · 2023-01-27T05:17:15Z

As for CLA, it will take a little more time for our company procedural reasons.

k8s-ci-robot · 2023-02-09T04:34:05Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hama-25179
Once this PR has been reviewed and has the lgtm label, please assign vincepri for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

CecileRobertMichon · 2023-02-09T18:59:48Z

docs/development.md

+### Automatically install calico in workload clusters with Tilt
+
+Automatically install [calico](https://projectcalico.docs.tigera.io/about/about-calico) in workload clusters using the [yaml manifest](https://github.com/Jont828/cluster-api-addon-provider-helm/blob/main/config/samples/calico-cni.yaml) provided by CAAPH.  


should the link point to the kubernetes-sigs repo now?

Thank you for pointing that out.
I corrected the link point.
Also I corrected the link for calico. (for inaccessible)

Jont828 · 2023-02-14T01:47:42Z

@hama-25179 I tried running off of your branch and had some issues. I've opened #24 so that the empty label selector you added here will allow it to select all workload clusters, and to fix some typos with the webhook. PTAL and once that merges we can rebase your current PR.

Jont828 · 2023-02-18T00:19:46Z

@hama-25179 I merged the PR to change the label selector as well as some of the CI tests. Can you rebase and try it out to make sure it still works?

hama-25179 · 2023-02-27T00:48:40Z

@Jont828 I have checked again, there are no problems.
Although unrelated to this correction,I thought the parameter "sync-period" default value of 10 min was long.I create a workload cluster after applying HelmChartProxy resource,It takes up to 10 minutes to start the installation of Helmchart.
How about changing the default value to about 1-2 minutes?

cluster-api-addon-provider-helm/main.go

Lines 73 to 74 in 3bfa919

    
           flag.DurationVar(&syncPeriod, "sync-period", 10*time.Minute, 
        
           	"The minimum interval at which watched resources are reconciled (e.g. 15m)")

Jont828 · 2023-02-28T20:12:53Z

/ok-to-test

Jont828 · 2023-02-28T20:15:29Z

I set it at 10 min to match the sync period for the CAPI controller but since it's a CLI flag you should be able to override it with something like this. IMO we could probably reduce the sync period for development at least though.

extra_args:
  helm:
    - "--sync-period=1m"

Jont828 · 2023-03-08T18:47:43Z

@hama-25179 This looks good to me! Could you squash your commits before I merge?

Add a calico auto-install configuration to the development docs. Modify a HelmChartProxy manifest of calico to target all workload clusters. Create a shell script to register HelmChartProxy manifest of calico as a Tilt resource.

k8s-ci-robot requested review from CecileRobertMichon and jackfrancis January 24, 2023 00:50

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jan 24, 2023

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 24, 2023

Jont828 reviewed Jan 25, 2023

View reviewed changes

hama-25179 mentioned this pull request Jan 26, 2023

Support for the other label selector fields #15

Closed

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jan 31, 2023

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 9, 2023

CecileRobertMichon reviewed Feb 9, 2023

View reviewed changes

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Feb 28, 2023

Automatic installation of Calico on workload clusters

43b63fe

Add a calico auto-install configuration to the development docs. Modify a HelmChartProxy manifest of calico to target all workload clusters. Create a shell script to register HelmChartProxy manifest of calico as a Tilt resource.

hama-25179 force-pushed the cni-auto-installation branch from 8a45c80 to 43b63fe Compare March 9, 2023 02:00

Jont828 merged commit 412dfd1 into kubernetes-sigs:main Mar 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic installation of Calico on workload clusters #14

Automatic installation of Calico on workload clusters #14

hama-25179 commented Jan 24, 2023

linux-foundation-easycla bot commented Jan 24, 2023 •

edited

k8s-ci-robot commented Jan 24, 2023

Jont828 left a comment

Jont828 Jan 25, 2023

hama-25179 Jan 25, 2023

hama-25179 Jan 26, 2023

Jont828 Feb 2, 2023

hama-25179 Feb 9, 2023

Jont828 Feb 14, 2023

hama-25179 Feb 15, 2023

hama-25179 Feb 16, 2023

Jont828 Jan 25, 2023

hama-25179 Jan 25, 2023

Jont828 Feb 2, 2023

Jont828 Feb 2, 2023

hama-25179 Feb 9, 2023

Jont828 Feb 14, 2023

hama-25179 Feb 15, 2023

hama-25179 commented Jan 27, 2023

k8s-ci-robot commented Feb 9, 2023

CecileRobertMichon Feb 9, 2023

hama-25179 Feb 12, 2023

Jont828 commented Feb 14, 2023 •

edited

Jont828 commented Feb 18, 2023

hama-25179 commented Feb 27, 2023 •

edited

Jont828 commented Feb 28, 2023

Jont828 commented Feb 28, 2023

Jont828 commented Mar 8, 2023

		### Automatically install calico in workload clusters with Tilt

		Automatically install [calico](https://projectcalico.docs.tigera.io/about/about-calico) in workload clusters using the [yaml manifest](https://github.com/Jont828/cluster-api-addon-provider-helm/blob/main/config/samples/calico-cni.yaml) provided by CAAPH.

Automatic installation of Calico on workload clusters #14

Automatic installation of Calico on workload clusters #14

Conversation

hama-25179 commented Jan 24, 2023

linux-foundation-easycla bot commented Jan 24, 2023 • edited

k8s-ci-robot commented Jan 24, 2023

Jont828 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hama-25179 commented Jan 27, 2023

k8s-ci-robot commented Feb 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jont828 commented Feb 14, 2023 • edited

Jont828 commented Feb 18, 2023

hama-25179 commented Feb 27, 2023 • edited

Jont828 commented Feb 28, 2023

Jont828 commented Feb 28, 2023

Jont828 commented Mar 8, 2023

linux-foundation-easycla bot commented Jan 24, 2023 •

edited

Jont828 commented Feb 14, 2023 •

edited

hama-25179 commented Feb 27, 2023 •

edited