Skip to content
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

Update the v1.7 calico manifest to Calico v3.17.1 #1328

Closed
wants to merge 1 commit into from

Conversation

lwr20
Copy link
Contributor

@lwr20 lwr20 commented Dec 14, 2020

What type of PR is this?
cleanup - updating Calico manifest in v1.7 from calico v3.15.1 to v3.17.1

Which issue does this PR fix:
n/a

What does this PR do / Why do we need it:
This PR updates Calico. Calico v3.16 introduced the ability BPF mode and v3.17 contained several bugfixes for EKS when running in BPF mode.

These changes are present in config/master/calico.yaml already. This PR attempts to bring them into the v1.7 calico manifest too. TBH, I'm not sure I understand why there are config/<version> directories - if this isn't the right way to go about getting this change into 1.7, please tell me what I should be doing instead.

I'm basing this all on the fact that https://docs.aws.amazon.com/eks/latest/userguide/calico.html links to https://raw.githubusercontent.com/aws/amazon-vpc-cni-k8s/v1.7.5/config/v1.7/calico.yaml

So this PR basically applies the changes from #1235, #1282 and #1326 to config/v1.7/calico.yaml.

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
n/a

Testing done on this change:

I've set up an EKS cluster using eksctl, applied this manifest and created various pods/services and checked they work as expected.

Automation added to e2e:

Existing Calico test cases should cover this?

Will this break upgrades or downgrades. Has updating a running cluster been tested?:
Not explicitly tested, but no breakages expected.

Does this change require updates to the CNI daemonset config files to work?:

This PR is itself an update of the CNI daemonset.

Does this PR introduce any user-facing change?:

I think a release note isn't required since the PRs it combines already have release notes?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@jayanthvn jayanthvn left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for this :)

@jayanthvn
Copy link
Contributor

jayanthvn commented Dec 14, 2020

Yes we are going away from having configs per version (#1271). 1.8 release onwards we will attach the sample manifests as release artifacts but for now we will have to support older releases. To get this to 1.7 branch, I can mark this to milestone 1.7.8 and then we will have merge this to release-1.7 branch. For now I can merge it to master but later I will cherry-pick this to 1.7 branch.

@jayanthvn jayanthvn added this to the v1.7.8 milestone Dec 14, 2020
@jayanthvn jayanthvn self-assigned this Jan 6, 2021
@jayanthvn
Copy link
Contributor

Hi, I missed that it should be ported to v1.7 branch. Generally before the release we port the fixes done on master to the release branch. I can mark these PRs - #1235, #1282 and #1326 as milestone for either 1.7.9 or 1.8 (we are discussing on this). Then we can cherrypick those PRs to the release branch. Please let me know if it fine?

@lwr20
Copy link
Contributor Author

lwr20 commented Jan 12, 2021

Sure, whichever you feel is appropriate. Do I need to do anything to this PR?

@jayanthvn
Copy link
Contributor

Thanks @lwr20 . I will close this PR and tag the other PRs for milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants