Update Calico to v3.10.2 #8104
Update Calico to v3.10.2 #8104
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @hakman. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
size/XXL
|
/assign @KashifSaadat |
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
hakman
force-pushed
the
calico-3.10.2
branch
from
b326ef3 to
a806f10
Compare
| roleRef: | ||
| apiGroup: rbac.authorization.k8s.io | ||
| kind: ClusterRole | ||
| name: calico | ||
| name: calico-node |
There was a problem hiding this comment.
Due to the change in role name upstream, this becomes a breaking change for users upgrading as role references in bindings cannot be changed.
There was a problem hiding this comment.
ACK. Will change back this part.
| kind: ServiceAccount | ||
| metadata: | ||
| name: canal | ||
| namespace: kube-system |
There was a problem hiding this comment.
So this one has puzzled me. I would have expected that the ServiceAccount must exist before the deployment that references it is created, but I guess as all resources are applied in quick succession the SA will be created before the scheduler kicks in to create the DS pods.
There was a problem hiding this comment.
I did not see any issue with this. I agree that this is the recommended way to do it, but it would be different from the official manifest and make it harder to compare with official manifests.
I would first try to push the change in official manifests.
| {{- if .Networking.Canal.MTU }} | ||
| veth_mtu: "{{ .Networking.Canal.MTU }}" | ||
| {{- else }} | ||
| veth_mtu: "{{- if eq .CloudProvider "openstack" -}}1430{{- else -}}1440{{- end -}}" |
There was a problem hiding this comment.
If unset it previously defaulted to 1500, should that be 1440 now in all cases?
There was a problem hiding this comment.
I used the same logic from the Calico side. The value seemed a safer default for most environments, including when IP-in-IP is used (for cross subnet):
https://docs.projectcalico.org/v3.10/networking/mtu#determine-mtu-size
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman, KashifSaadat 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 |
k8s-ci-robot
added
the
approved
|
Thanks @KashifSaadat, will prepare the cherry-pick for 1.17 once it's merged. |
…pstream-release-1.17 Automated cherry pick of #8104: Add Calico v3.10.2
| spec: | ||
| nodeSelector: | ||
| beta.kubernetes.io/os: linux | ||
| kubernetes.io/role: master |
There was a problem hiding this comment.
This makes it much harder to run more than three Typha replicas. It would be better to have node affinity that expresses a preference, but not a requirement for Typha pods to run on the masters.
Please see #9608 for a follow-on suggestion.
|
Why was a nodeselector of |
|
@tmjd the change was first added in this commit: appvia@9b7a798 @KashifSaadat do you remember about this? ^^^ |
|
Ok so it was just copied from the Canal manifest, no specific reason then. |
|
Hey @hakman, just commented here: #7917 (comment) |
|
Thanks @KashifSaadat :) |
Calico v3.10 is here with some nice new stuff like bird/bird6 liveness checks.
Canal is already at 3.10 but Calico should be upgraded too.
This PR addresses also some other issues:
Commits should be reviewed one by one, as it will be much easier to compare each step.