Skip to content

/kubespray

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

Add calico 3.7.3 support #4953

Merged
merged 4 commits into from Jul 9, 2019

Conversation

@jlacoline
Copy link
Contributor

jlacoline commented Jul 8, 2019

What type of PR is this?
/kind feature

What this PR does / why we need it:
Adds support for calico version up to 3.7.3 (current max version is 3.4.4)

Special notes for your reviewer:
I made the templates as close as possible to calico installation manifests from here: https://docs.projectcalico.org/v3.7/getting-started/kubernetes/installation/calico
I were also inspired by the calico helm templates: https://github.com/projectcalico/calico/tree/master/_includes/v3.7/charts/calico/templates

Does this PR introduce a user-facing change?:

Default calico version is now 3.7.3
@k8s-ci-robot

This comment has been minimized.

Sign in to view
Copy link
Contributor

k8s-ci-robot commented Jul 8, 2019

Welcome @jlacoline!

It looks like this is your first PR to kubernetes-sigs/kubespray 🎉. 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/kubespray 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. 😃
@k8s-ci-robot k8s-ci-robot requested review from holmsten and mirwan Jul 8, 2019
@k8s-ci-robot k8s-ci-robot added the size/L label Jul 8, 2019
@jlacoline
Add calico 3.7.3 support
Loading status checks…
afebbbd
@jlacoline jlacoline force-pushed the jlacoline:calico3.7 branch to afebbbd Jul 8, 2019
@mirwan mirwan mentioned this pull request Jul 8, 2019
Closed
@jlacoline
add calico_datastore variable to policy controller role
Loading status checks…
693f360
@jlacoline jlacoline changed the title WIP: Add calico 3.7.3 support Add calico 3.7.3 support Jul 8, 2019
@mirwan

This comment has been minimized.

Sign in to view
Copy link
Contributor

mirwan commented Jul 8, 2019

ci check this
@AlexeyKasatkin
AlexeyKasatkin reviewed Jul 8, 2019
View changes
roles/network_plugin/calico/templates/calico-node.yml.j2 Show resolved Hide resolved
@mirwan
mirwan approved these changes Jul 8, 2019
View changes
@AlexeyKasatkin
AlexeyKasatkin reviewed Jul 9, 2019
View changes
roles/kubernetes-apps/policy_controller/calico/templates/calico-kube-controllers.yml.j2
env:
{% if calico_datastore == "kdd" and calico_version is version('v3.6.0', '>=') %}

This comment has been minimized.

Sign in to view
Copy link
@AlexeyKasatkin

AlexeyKasatkin Jul 9, 2019

Contributor

Does kube-controllers work for you? I had to change 'calico-kube-controllers' ClusterRole according to calico manifest to make it work properly.

This comment has been minimized.

Sign in to view
Copy link
@jlacoline

jlacoline Jul 9, 2019 •
edited

Author Contributor

I actually did forgot this part, and did not detect it in my deployment because the cluster role got updated as a side effect of other tests.

Fixed by latest commit

This comment has been minimized.

Sign in to view
Copy link
@AlexeyKasatkin

AlexeyKasatkin Jul 9, 2019

Contributor

Thanks. Although, here and in the latest commit there are conditionals that prevent us from running calico+kdd for calico < v3.6 at all. So, those conditionals should be changed or it should be some formal restriction to prevent user from installing calico <v3.6 using kdd.

This comment has been minimized.

Sign in to view
Copy link
@jlacoline

jlacoline Jul 9, 2019

Author Contributor

In the latest commit I added the calico-version >= 3.6 limitation because no cluster role is defined for kube-controller in previous versions in kdd mode (I looked at 3.4 and 3.5 templates).
I used the same reasoning for the other parts in this PR that have a constraint on calico_version.
So in this regard there should be no regression concerning previous versions.

This comment has been minimized.

Sign in to view
Copy link
@AlexeyKasatkin

AlexeyKasatkin Jul 9, 2019 •
edited

Contributor

Agree. Sorry, I didn't check that.
Seems, it would be better to exclude whole calico-kube-controllers Deployment from installation for calico < v3.6 using kdd then.

This comment has been minimized.

Sign in to view
Copy link
@jlacoline

jlacoline Jul 9, 2019

Author Contributor

Apparently calico kube controller seems irrelevant when kdd mode is used in versions < 3.6 so I did as you suggested (commit d988cca).

I can remove this commit if this technical choice isn't approved by everybody
@jlacoline
add missing clusterrole rules for calico policy controller
Loading status checks…
3ce7b2c
@mattymo

This comment has been minimized.

Sign in to view
Copy link
Contributor

mattymo commented Jul 9, 2019

/lgtm
/approve
@k8s-ci-robot k8s-ci-robot added the lgtm label Jul 9, 2019
@k8s-ci-robot

This comment has been minimized.

Sign in to view
Copy link
Contributor

k8s-ci-robot commented Jul 9, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlacoline, mattymo

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@jlacoline
disable calico kube controller when kdd mode is used for versions < 3.6
Loading status checks…
d988cca
@k8s-ci-robot k8s-ci-robot removed the lgtm label Jul 9, 2019
@AlexeyKasatkin
AlexeyKasatkin approved these changes Jul 9, 2019
View changes
@holmsten

This comment has been minimized.

Sign in to view
Copy link
Member

holmsten commented Jul 9, 2019

/lgtm
@k8s-ci-robot k8s-ci-robot added the lgtm label Jul 9, 2019
@k8s-ci-robot k8s-ci-robot merged commit 20c7e31 into kubernetes-sigs:master Jul 9, 2019
21 of 22 checks passed
21 of 22 checks passed
tide Not mergeable. Needs lgtm label.
Details
Kubespray CI - ansible-lint Build Succeeded
Details
Kubespray CI - ci-authorized Build Succeeded
Details
Kubespray CI - packet_centos7-flannel-addons Build Succeeded
Details
Kubespray CI - packet_centos7-kube-router Build Succeeded
Details
Kubespray CI - packet_debian9-calico-upgrade Build Succeeded
Details
Kubespray CI - packet_debian9-macvlan-sep Build Succeeded
Details
Kubespray CI - packet_ubuntu-canal-kubeadm Build Succeeded
Details
Kubespray CI - packet_ubuntu-contiv-sep Build Succeeded
Details
Kubespray CI - packet_ubuntu18-calico-aio Build Succeeded
Details
Kubespray CI - shellcheck Build Succeeded
Details
Kubespray CI - syntax-check Build Succeeded
Details
Kubespray CI - tf-ovh_coreos-calico Build Succeeded
Details
Kubespray CI - tf-ovh_ubuntu18-calico Build Succeeded
Details
Kubespray CI - tf-validate-aws Build Succeeded
Details
Kubespray CI - tf-validate-openstack Build Succeeded
Details
Kubespray CI - tf-validate-packet Build Succeeded
Details
Kubespray CI - vagrant-validate Build Succeeded
Details
Kubespray CI - yamllint Build Succeeded
Details
Kubespray CI Pipeline Pipeline Succeeded
Details
cla/linuxfoundation jlacoline authorized
Details
kubespray-ci/pipeline Pipeline Succeeded
Details
@woopstar

This comment has been minimized.

Sign in to view
Copy link
Member

woopstar commented Jul 11, 2019

@holmsten @jlacoline README.md needs to be updated since 3.4.0 is not default anymore.
@jlacoline jlacoline mentioned this pull request Jul 11, 2019
Merged
@AlexeyKasatkin AlexeyKasatkin mentioned this pull request Jul 24, 2019
Merged
LuckySB added a commit to southbridgeio/kubespray that referenced this pull request Aug 4, 2019
@jlacoline @LuckySB
Add calico 3.7.3 support (kubernetes-sigs#4953)
fc25c8d 
* Add calico 3.7.3 support

* add calico_datastore variable to policy controller role

* add missing clusterrole rules for calico policy controller

* disable calico kube controller when kdd mode is used for versions < 3.6
geneliu added a commit to geneliu/kubespray that referenced this pull request Oct 3, 2019
@jlacoline @geneliu
Add calico 3.7.3 support (kubernetes-sigs#4953)
e341be3 
* Add calico 3.7.3 support

* add calico_datastore variable to policy controller role

* add missing clusterrole rules for calico policy controller

* disable calico kube controller when kdd mode is used for versions < 3.6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexeyKasatkin AlexeyKasatkin

@mirwan mirwan

@holmsten holmsten

Assignees

@mattymo mattymo

@holmsten holmsten

Labels
approved cncf-cla: yes kind/feature lgtm size/L
Projects
None yet
Milestone
No milestone
7 participants
@jlacoline @k8s-ci-robot @mirwan @mattymo @holmsten @woopstar @AlexeyKasatkin
You can’t perform that action at this time.