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

Please remove reference to helm chart from README.md #847

Closed
3oris opened this issue Feb 17, 2020 · 7 comments
Closed

Please remove reference to helm chart from README.md #847

3oris opened this issue Feb 17, 2020 · 7 comments
Labels
config Helm, raw configuration manifests, deployment artifacts

Comments

@3oris
Copy link

3oris commented Feb 17, 2020

The VPC CNI plugin is included in the official AWS announcements and documentation. So we can assume this is an officially backed product. Unfortunately and obviously the helm chart is not:

@jaypipes
Copy link
Contributor

Hi @3oris, the Helm chart for the VPC CNI is officially supported and is not poorly maintained. The Helm chart is deliberately behind the latest CNI release because we bake the CNI releases before updating the Helm chart (this is why the eks-charts repo has a "stable/" directory, to indicate that what it deploys is not the equivalent of the latest Docker image tag).

That said, of course there's plenty of room for improvement. Our plan of action regarding how the CNI repo's configuration files are handled in the future is as follows (and actually, we're going to take this approach for all AWS curated repos):

  1. Add a charts/ directory to the CNI source repo. This will contain a single Helm chart with a values.yaml file containing production defaults for configuration variables, and will install the CNI daemonset and CNI plugin along with the metrics helper. The Helm chart will install the latest CNI images.
  2. Generate a release artifact for a single flat YAML manifest that will install the CNI daemonset and plugin without Helm using the production values defaults. You can see here for the pull request in the node-termination-handler repo where we are doing this generation using helm template.
  3. Create a script that will push a copy of the Helm chart from the CNI source repo to the eks-charts repo when a release tag is created in the CNI source repo. The CNI Helm chart in eks-charts will continue to be under stable/ and the images referenced will match the CNI release. Automation scripts in the eks-charts source repo will be the only thing that ever publishes to the eks Helm repository.
  4. Remove the config directory entirely from the CNI source repo

Hope this gives you some idea about what our roadmap looks like for getting our Helm and configuration files in order!

Thanks,
-jay

@3oris
Copy link
Author

3oris commented Feb 18, 2020

Hi @jaypipes ,

thanks a lot for your comprehensive response explaining the general roadmap regarding the helm chart releases in https://github.com/aws/eks-charts/ . I am very much looking forward to it.

The fact that the chart cannot be installed in the first place

$ helm version
version.BuildInfo{Version:"v3.0.2", GitCommit:"19e47ee3283ae98139d98460de796c1be1e3975f", GitTreeState:"clean", GoVersion:"go1.13.5"}

$ helm upgrade --install --recreate-pods --force aws-vpc-cni --namespace kube-system eks/aws-vpc-cni
Flag --recreate-pods has been deprecated, functionality will no longer be updated. Consult the documentation for other methods to recreate pods
Release "aws-vpc-cni" does not exist. Installing it now.
Error: rendered manifests contain a resource that already exists. Unable to continue with install: existing resource conflict: kind: ServiceAccount, namespace: kube-system, name: aws-node

because a newly created cluster already contains the resources in question will hopefully be addressed here: aws/eks-charts#57 .

@jaypipes jaypipes added the config Helm, raw configuration manifests, deployment artifacts label Feb 20, 2020
@jaypipes
Copy link
Contributor

@3oris please see #758 for a fuller description of the plans.

@jaypipes jaypipes reopened this Feb 21, 2020
@mogren
Copy link
Contributor

mogren commented Apr 29, 2020

@jaypipes Why was this one reopened?

@jaypipes
Copy link
Contributor

@jaypipes Why was this one reopened?

Because it's not fixed yet :)

@jayanthvn
Copy link
Contributor

jayanthvn commented Dec 9, 2020

Merged #1271 to master. Will pick this up in the next CNI release. Automation to copy the configs to eks charts is tracked here - #1319

@jayanthvn
Copy link
Contributor

Fix for #1319 is merged and we will be taking this approach from 1.9.0 release onwards (most probably next month). Ref #1497, #1430 and #1271

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Helm, raw configuration manifests, deployment artifacts
Projects
None yet
Development

No branches or pull requests

4 participants