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

Deprecate and remove AWS cloud-controller-manager Helm Chart #933

Open
dims opened this issue May 31, 2024 · 18 comments
Open

Deprecate and remove AWS cloud-controller-manager Helm Chart #933

dims opened this issue May 31, 2024 · 18 comments
Labels
needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@dims
Copy link
Member

dims commented May 31, 2024

We have a helm chart which currently defaults to support for k8s 1.27. We do not have a CI job that tests this chart though.

Looking through usages in the wild:
https://github.com/search?type=code&q=%2Fhelm+repo+add+aws-cloud-controller-manager%2F&p=1

kops et al do use manifests directly:
https://github.com/search?q=repo%3Akubernetes%2Fkops+aws-cloud-controller-manager&type=code&p=4

Given community k8s 1.27 is EOL in June 2024 we should just drop the chart entirely by the time k8s 1.31 is released.

@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If cloud-provider-aws contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label May 31, 2024
@dims
Copy link
Member Author

dims commented May 31, 2024

cc @cartermckinnon @kmala

@kmala
Copy link
Member

kmala commented May 31, 2024

/triage accepted

@k8s-ci-robot
Copy link
Contributor

@kmala: The label triage/accepted cannot be applied. Only GitHub organization members can add the label.

In response to this:

/triage accepted

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-sigs/prow repository.

@cartermckinnon
Copy link
Contributor

I think it's worth keeping around, it has a decent number of users (including Rancher: https://github.com/rancher/rke1-docs). Can we switch the values.yaml to use a latest tag/something low maintenance instead?

@dims
Copy link
Member Author

dims commented May 31, 2024

@cartermckinnon we don't have a CI job to verify it does work .. so can't guarantee the functionality

@atsai1220
Copy link

What would be the recommended way to install this cloud-provider?

@dims
Copy link
Member Author

dims commented Jun 4, 2024

What would be the recommended way to install this cloud-provider?

https://github.com/kubernetes/cloud-provider-aws/blob/master/docs/getting_started.md#upgrading-an-existing-cluster

@atsai1220
Copy link

atsai1220 commented Jun 4, 2024 via email

@et304383
Copy link

What would be the recommended way to install this cloud-provider?

https://github.com/kubernetes/cloud-provider-aws/blob/master/docs/getting_started.md#upgrading-an-existing-cluster

We are expected to apply a static example bunch of yaml files? They're still referencing version 1.27.1. What is the correct way to install the latest? Use this directory and do a bunch of sed commands for the latest?

Why not support the helm chart as the installation mechanism like the EBS CSI driver is doing? https://github.com/kubernetes-sigs/aws-ebs-csi-driver/releases

I hate to be blunt, but it's bad enough that the documentation for this CRITICAL project is severely lacking, but now a stable installation mechanism isn't even provided?

I REALLY wish AWS would chime in and take over this project so it has proper support instead of just expecting everyone to use EKS.

@dims
Copy link
Member Author

dims commented Jun 12, 2024

@et304383 can you please share where / how exactly you are using this chart?

@et304383
Copy link

@et304383 can you please share where / how exactly you are using this chart?

We are managing our own K8s control plane for over 40 clusters, and obviously have to install this controller to support 1.27+

So far, this is how script to install:

helm repo add aws-cloud-controller-manager https://kubernetes.github.io/cloud-provider-aws
helm repo update
helm pull aws-cloud-controller-manager/aws-cloud-controller-manager --version 0.0.8 --untar

cd aws-cloud-controller-manager
cluster_cidr=$(grep cluster-cidr /etc/kubernetes/manifests/kube-controller-manager.yaml | cut -d = -f2)
cluster_name=$(grep cluster-name /etc/kubernetes/manifests/kube-controller-manager.yaml | cut -d = -f2)
yq -i '.args += ["--allocate-node-cidrs=false", "--configure-cloud-routes=false", "--cluster-cidr='$cluster_cidr'", "--cluster-name='$cluster_name'"]' values.yaml

export KUBECONFIG=/etc/kubernetes/admin.conf
helm upgrade --install -n kube-system aws-cloud-controller-manager .

You can see we already have to edit the values file to specify cluster cidr and cluster name (which there is zero documentation for, mind you), so adding another update to change the image version is just another line of code.

@atsai1220
Copy link

We have been managing k8s cluster on ec2 instances with cloud-provider=external then installing this aws cloud-provider. We leverage the helm-controller inside k3s/rke2 to bootstrap the cluster with this cloud-provider.

apiVersion: helm.cattle.io/v1
kind: HelmChart
metadata:
  name: cloud-provider-aws
  namespace: kube-system
spec:
  chart: https://github.com/kubernetes/cloud-provider-aws/releases/download/helm-chart-aws-cloud-controller-manager-0.0.7/aws-cloud-controller-manager-0.0.7.tgz
  targetNamespace: kube-system
  bootstrap: true
  valuesContent: |-
    args:
      - --v=2
      - --cloud-provider=aws
      - --controllers=cloud-node,cloud-node-lifecycle,service,-route
      - '--cloud-config=/tmp/cloud-provider-config'
    nodeSelector:
      node-role.kubernetes.io/control-plane: "true"
    tolerations:
    - effect: NoSchedule
      key: ""
      operator: Exists
    - effect: NoExecute
      key: ""
      operator: Exists
    extraVolumes:
    - hostPath:
        path: /var/lib/rancher/rke2/etc/config-files/cloud-provider-config
        type: File
      name: cloud-provider-config
    extraVolumeMounts:
    - mountPath: /tmp/cloud-provider-config
      name: cloud-provider-config

@apt-x4869
Copy link
Contributor

We are managing multiple cluster. We need cloud provider helm chart to install for kubernetes cluster to work. We have automated process for upgrading and testing clusters so it is easier to use helm chart values to update , rather that manually updating in a yaml file. if something goes wrong in testing , it could be rolled back easily using helm.

I think its better to keep the helm chart.

#954

@jobcespedes
Copy link

Currently using helm chart for self managed clusters

@sdlarsen
Copy link

sdlarsen commented Aug 2, 2024

We're using it for self-managed clusters as well, specifying the tag in a values.yaml.

@gabrielbull
Copy link

gabrielbull commented Oct 18, 2024

We are also managing many k8s clusters and using this helm chart to install this important project. I can also confirm that currently, tagging the latest version (1.31.1) using the values.yaml does not work correctly and leaves this taint on the nodes:

node.cloudprovider.kubernetes.io/uninitialized: true

Using the default version (1.27.1) still works fine even on k8s clusters on version 1.31.1, but it would be nice to have the helm charts updated and even better to have them updated automatically using a ci process.

@shaunofneuron
Copy link

To confirm for those seeing issues with default nodeSelector keys getting merged with ones that are provided, the 0.0.6 chart doesn't exhibit the issue and for those needing a workaround and want to use the latest chart version, here's a hack - supply a null value to get a single selector like so:

nodeSelector:
  node-role.kubernetes.io/master: ''
  node-role.kubernetes.io/control-plane: null

results in:

nodeSelector:
  node-role.kubernetes.io/master: ''

This chart needs proper attention from AWS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests