Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Update calico to 3.16.4 #1113

Merged
merged 3 commits into from
Oct 28, 2020
Merged

Update calico to 3.16.4 #1113

merged 3 commits into from
Oct 28, 2020

Conversation

ipochi
Copy link
Member

@ipochi ipochi commented Oct 22, 2020

This PR updates calico to 3.16.4

Signed-off-by: Imran Pochi <imran@kinvolk.io>
This commit adds calico-kube-controllers ServiceAccount,
ClusterRoleBinding and ClusterRole to the correct location
`cluster-role-binding.yaml`, `cluster-role.yaml` and
`service-account.yaml`.

Signed-off-by: Imran Pochi <imran@kinvolk.io>
This commit updates Calico to the version 3.16.4.

Renames the file `daemonset.yaml` to correctly reflect the resource it
contains i.e `calico-node.yaml`

Renames the file `deployment.yaml` to correctly reflect the resource it
contains i.e `calico-kube-controllers.yaml`

Updates the calico config.

Signed-off-by: Imran Pochi <imran@kinvolk.io>
@@ -0,0 +1,50 @@
# See https://github.com/projectcalico/kube-controllers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this change on purpose?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean moving from deployment.yaml to this file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I mean , this makes the file naming correct to the resource it holds.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the file was named deployment.yaml, it contain the deployment resource. I renamed it reflect better.
Similar with daemonset.yaml

deployment.yaml - calico-kube-controllers.yaml
daemonset.yaml - calico-node.yaml

Copy link
Member

@surajssd surajssd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits, otherwise LGTM.

@@ -3,7 +3,7 @@ kind: CustomResourceDefinition
metadata:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

- calico: update calico CRDS
+ calico: update calico CRDs

@@ -1,3 +1,4 @@
---
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, the best would be to have everything in calico.yaml, as from wget https://docs.projectcalico.org/v3.16.4/manifests/calico.yaml

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, we should look into integrating Tigera Operator in our helm chart.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not ?

Copy link
Member

@invidian invidian Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, perhaps this is the way to go.

Created #1125 for tracking.

@ipochi ipochi merged commit 579f999 into master Oct 28, 2020
@ipochi ipochi deleted the imran/update-calico-to-3.16.4 branch October 28, 2020 13:09
@invidian invidian mentioned this pull request Nov 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants