Generate calico artifacts from helm #1541
Conversation
|
You're not removing |
Yeah, I haven't copied it to master/config. That makes sense, let me do that and will be easy to verify. |
|
@tmjd - I copied the generated |
|
The differences in the generated manifests are showing because you based your updates on the master branch but if the idea was to match what is currently in the calico-operator.yaml, the changes should have been based on the release-v3.17 branch.
I think keeping them in sync should work but that sync should be done on release branches and also the charts/aws-calico/values.yaml will need to also be updated to keep the tigeraOperator.tag field in sync with the release. |
Agreed, I will update it to match the release-v3.17 branch |
|
Thanks @tmjd |
|
I might be mistaken, but it looks like you've taken a Calico version that isn't compatible with the aws-vpc-cni. Basically any version of Calico from v3.16.0 to v3.19.x is incompatible with any non-Calico CNI due to a known issue blocking all network traffic after pod termination. The latest v3.20.0 (Tigera operator v1.20.0) seems to have resolved the issue (see eks-charts for more details as they've made this very mistake). |
|
@tmjd can you please comment on this? |
|
@stevehipwell I'm not sure what you mean by not compatible with aws-vpc-cni. I believe it is compatible and has been functioning with it, but yes there is a known bug with the version. I think it would be good for this project to upgrade to Calico v3.20 to pick up the fix for the bug but I think it is an exaggeration to say this PR is pulling in an incompatible version. Please if there is something else you're referring to or I'm misunderstanding something please let me know and provide a link to the Issue. If it is not possible to update to Calico v3.20, the fix for the termination bug has been backported to the v3.19, v3.18, and v3.17 branches though I'm not sure when those releases will happen. I'm hoping with the changes that @jayanthvn has just merged that it will be easier to keep up with Calico updates and it won't be much trouble to update to v3.20. |
|
Thanks for raising this @stevehipwell ! This is indeed a problem and only Calico 3.20 has the fix for EKS CNI compatibility. Helm charts may well be unaffected with only the appVersion changing.. to be verified. |
|
@stevehipwell I think I see what you're talking about. You're talking about the helm chart that is generated with this configuration. I didn't realize somewhere the helm chart this was updating was being used, I thought the purpose was just to be able to generate the manifest that is used in the AWS documentation which is the calico-operator.yaml. So you're correct that the helm chart does now include the bug that I referenced. |
|
We can upgrade it to v3.20.0. Happy to take PRs :) |
|
The TL;DR version is that using any version of Calico after v3.15.x and before v3.20.0 with the aws-vpc-cni will introduce a serious defect with pod termination. This is especially problematic with out of tree CSI drivers where any pods with PVCs will have to wait a significant time to restart after being on a terminated node. The aws-calico chart in eks-charts was updated to a version with this issue and caused us a significant amount of work to figure out what was wrong (there is still no response on that issue). I get that this project already had a broken version of Calico, but as the definitive way to install Calico in EKS replacing the aws-calico chart this should either be rolled back to a working v3.15.x version or forward to v3.20.0 and not left broken. |
|
@jayanthvn, @haouc, Whenever we are updating the Calico, we will be updating the manifest file as well right? |
* Generate calico manifests from helm * Update CNI charts image version * Testing release workflow<Do-not-merge> * Testing workflow * Updating workflow * set matchlabels while generating manifests * Copy generated manifests to master/config folder * update to match the release branch * update operator to v1.13.8 * update version
* Generate calico manifests from helm * Update CNI charts image version * Testing release workflow<Do-not-merge> * Testing workflow * Updating workflow * set matchlabels while generating manifests * Copy generated manifests to master/config folder * update to match the release branch * update operator to v1.13.8 * update version
Update documentation for PD (aws#1539) Update SDK Go version (aws#1542) * Update SDK Go version * missed mod file Bump helm.sh/helm/v3 from 3.2.0 to 3.6.1 in /test (aws#1545) Bumps [helm.sh/helm/v3](https://github.com/helm/helm) from 3.2.0 to 3.6.1. - [Release notes](https://github.com/helm/helm/releases) - [Commits](helm/helm@v3.2.0...v3.6.1) --- updated-dependencies: - dependency-name: helm.sh/helm/v3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Update CNI charts image version (aws#1543) * Update CNI charts image version * updated the labels * CRD update * update charts version * minor nits * fix charts ver * fix charts ver Cherry-pick to master - (aws#1551) (aws#1552) * Release - v1.9.0 (aws#1551) * update markdown Update prefix-and-ip-target.md (aws#1553) Grammar fixes Use codecov github action (aws#1550) In particular, this avoids `curl | bash` (the bash script is simply embedded in the github action image). It also provides a trivial upgrade path to the new nodejs-based uploader (`@v1` -> `@v2`), when we decide that is ready (perhaps now). Added Multus artifacts to config folder (aws#1563) Added Readme for Multus Installation Updated configMap in daemonset to use aws-vpc-cni as default delegate instead of flannel Co-authored-by: Chinmay Gadgil <cgadgil@amazon.com> set requests/limits for initcontainer (aws#1559) Documentation update (aws#1565) * Doc update * more updates set multus log level to panic instead of debug (aws#1567) Updated Readme for Multus logging info Changed log level to error instead of debug Co-authored-by: Chinmay Gadgil <cgadgil@amazon.com> WARM targets can be set non-negative (aws#1568) Generate calico artifacts from helm (aws#1541) * Generate calico manifests from helm * Update CNI charts image version * Testing release workflow<Do-not-merge> * Testing workflow * Updating workflow * set matchlabels while generating manifests * Copy generated manifests to master/config folder * update to match the release branch * update operator to v1.13.8 * update version Change github_token permission (aws#1577) Bandwidth plugin support (aws#1560) * bandwidth plugin support * update netns Update new instance types (aws#1576) Updated multus ds manifest file for v3.7.2-eksbuild.2 (aws#1583) * Updated multus ds manifest file for v3.7.2-eksbuild.2 Remove Node Affinity for amd64 from manifest so that it can run on arm64 as well * revert log-level to error for multus Co-authored-by: Chinmay Gadgil <cgadgil@amazon.com> Modify integ test workflow (aws#1579) * Change github_token permission * - Modified permissions for github_token in cron and integ test workflow - Modified integ test workflow to run on push to master and release branches Upgrading controller-runtime is test dir (aws#1582) Update Ginkgo command params example in the doc (aws#1589) Update CONTRIBUTING.md (aws#1591) Fix region/account for manifests generated from helm (aws#1592) * Yamls generated from helm was missing region/account override * fix domains Updated snat rule test logic Install iptables in the test agent image
What type of PR is this?
Enhancement
Which issue does this PR fix:
#1517
What does this PR do / Why do we need it:
Ported the required manifests from
https://github.com/projectcalico/calico/tree/master/_includes/charts. Going forward we won't have config/ folder instead, these artifacts will be built from a script and attached as release artifacts. The helm version we are using doesn't support--include-crdsand upgrading the helm version would need additional fixes in the other charts folder. Hence moved thecrdsintemplatefolder for now. Will restructure as a fast follow up post 1.9 release./cc @caseydavenport
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:
Yes
Automation added to e2e:
yes
Will this break upgrades or downgrades. Has updating a running cluster been tested?:
No
Does this change require updates to the CNI daemonset config files to work?:
No
Does this PR introduce any user-facing change?:
No
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.