add official helm chart and repo #1306

jackfrancis · 2022-03-21T21:59:31Z

What type of PR is this?

What this PR does / why we need it:

This PR introduces an official helm chart inhelm/cloud-provider-azure, as well as an official helm repo with artifacts in helm/repo (available to the world at the reference https://raw.githubusercontent.com/kubernetes-sigs/cloud-provider-azure/master/helm/repo).

The implementation for the components has been copied from reference resources in the repo.

Helm repo and chart validation has been added (as a new test-helm make target, which is integrated into the existing test-check target), as well as helm repo packaging scripts.

Which issue(s) this PR fixes:

Fixes #1157

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

site/content/en/install/azure-ccm.md

coveralls · 2022-03-21T22:04:20Z

Coverage remained the same at 80.206% when pulling aa65f8d on jackfrancis:helm-chart into 807ee31 on kubernetes-sigs:master.

CecileRobertMichon · 2022-03-22T17:15:44Z

helm/cloud-provider-azure/Chart.yaml

+- kubernetes-sigs
+- kubernetes
+maintainers:
+- email: [email protected]


maintainers should probably match https://github.com/kubernetes-sigs/cloud-provider-azure/blob/master/OWNERS#L1

@feiskyer @andyzhangx what do you think? I would be happy to maintain this going forward (updating with every release). Perhaps @lzhecheng can be a co-maintainer?

Also happy to add everyone who is a repo maintainer.

Please add me to the list. Thank you.

yes, thanks for joining as co-maintainer

MartinForReal · 2022-03-23T02:42:32Z

/assign @lzhecheng

lzhecheng

Overall lgtm. A few nits left and rebase is needed. Besides, do we need to include .tgz into the repo?

helm/cloud-provider-azure/Chart.yaml

hack/verify-helm-repo.sh

jackfrancis · 2022-03-29T00:01:57Z

@lzhecheng including the actual .tgz of the chart version in the repo is how to fulfill the helm repository specification. See:

https://helm.sh/docs/topics/chart_repository/

The advantage of maintaining a definitive repo is so that users are able to issue helm commands like this:

$ helm install --repo https://raw.githubusercontent.com/kubernetes-sigs/cloud-provider-azure/master/helm/repo cloud-provider-azure

If we don't maintain a helm repo then users will be forced to git clone the cloud-provider-repository locally, or download the helm chart in another way, which is a tedious step.

Maintaining a repo isn't entirely without effort, but I've attempted to make that effort very easy by scripting everything, and adding CI verification to ensure that future chart changes are properly versioned and published in the repo.

lzhecheng · 2022-03-29T03:16:29Z

/lgtm
Thank you for the effort! @jackfrancis

jackfrancis · 2022-03-29T19:35:39Z

@lzhecheng do you have a public e-mail you're willing to include in the Chart.yaml maintainers section?

helm/cloud-provider-azure/templates/cloud-provider-azure.yaml

lzhecheng · 2022-03-30T00:08:13Z

@lzhecheng do you have a public e-mail you're willing to include in the Chart.yaml maintainers section?

Please use [email protected]

jackfrancis · 2022-03-30T00:15:08Z

@lzhecheng do you have a public e-mail you're willing to include in the Chart.yaml maintainers section?

Please use [email protected]

Thanks! Done.

lzhecheng · 2022-03-30T00:17:15Z

/lgtm
/approve

feiskyer · 2022-03-30T00:18:59Z

helm/cloud-provider-azure/templates/cloud-provider-azure.yaml

+    name: cloud-node-manager
+    namespace: kube-system
+---
+apiVersion: apps/v1


could you also include the Daemonset for Windows nodes?

Sure thing, is this the right reference to use?

https://github.com/kubernetes-sigs/cloud-provider-azure/blob/master/examples/out-of-tree/cloud-node-manager.yaml#L102-L155

I think so.

lzhecheng · 2022-03-30T00:21:04Z

/hold
Let's add windows ds.

jackfrancis · 2022-03-30T02:27:43Z

helm/cloud-provider-azure/templates/cloud-provider-azure.yaml

+        command: ["/cloud-node-manager.exe"]
+        args:
+          - --node-name=$(NODE_NAME)
+          {{- if hasKey .Values.cloudNodeManager "cloudConfig" }}


@feiskyer @lzhecheng Windows cloud-node-manager daemonset added. Lemme know if any of the below configurable options (--cloud-config, --kube-api-burst, --kube-api-content-type, etc) are not relevant for the windows cloud-node-manager.exe runtime.

feiskyer

Thanks for addressing the comments

/lgtm

k8s-ci-robot · 2022-03-30T02:30:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: feiskyer, jackfrancis, lzhecheng

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:

~~OWNERS~~ [feiskyer,lzhecheng]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

lzhecheng · 2022-03-30T02:32:30Z

/lgtm
/hold cancel

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 21, 2022

k8s-ci-robot requested review from andyzhangx and lzhecheng March 21, 2022 21:59

jackfrancis commented Mar 21, 2022

View reviewed changes

site/content/en/install/azure-ccm.md Outdated Show resolved Hide resolved

jackfrancis force-pushed the helm-chart branch 3 times, most recently from 685e6d7 to 2d4b837 Compare March 22, 2022 17:13

CecileRobertMichon reviewed Mar 22, 2022

View reviewed changes

k8s-ci-robot assigned lzhecheng Mar 23, 2022

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 23, 2022

lzhecheng reviewed Mar 24, 2022

View reviewed changes

helm/cloud-provider-azure/Chart.yaml Outdated Show resolved Hide resolved

hack/verify-helm-repo.sh Show resolved Hide resolved

jackfrancis force-pushed the helm-chart branch from 2d4b837 to 55d9bd6 Compare March 28, 2022 23:53

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 28, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 29, 2022

jackfrancis force-pushed the helm-chart branch from 55d9bd6 to d535b1f Compare March 29, 2022 18:09

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 29, 2022

jackfrancis force-pushed the helm-chart branch 5 times, most recently from 5b25b13 to 116a25b Compare March 29, 2022 19:29

jackfrancis force-pushed the helm-chart branch from 116a25b to 451c725 Compare March 29, 2022 21:04

jackfrancis commented Mar 29, 2022

View reviewed changes

helm/cloud-provider-azure/templates/cloud-provider-azure.yaml Show resolved Hide resolved

jackfrancis force-pushed the helm-chart branch from 451c725 to 11c95f3 Compare March 29, 2022 21:58

jackfrancis force-pushed the helm-chart branch from 11c95f3 to 05302bc Compare March 30, 2022 00:14

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 30, 2022

feiskyer reviewed Mar 30, 2022

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 30, 2022

add official helm chart and repo

aa65f8d

jackfrancis force-pushed the helm-chart branch from 05302bc to aa65f8d Compare March 30, 2022 02:25

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 30, 2022

jackfrancis commented Mar 30, 2022

View reviewed changes

feiskyer approved these changes Mar 30, 2022

View reviewed changes

k8s-ci-robot assigned feiskyer Mar 30, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 30, 2022

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 30, 2022

k8s-ci-robot merged commit 85e09b5 into kubernetes-sigs:master Mar 30, 2022

jackfrancis deleted the helm-chart branch March 30, 2022 16:03

jackfrancis mentioned this pull request Mar 31, 2022

e2e: use helm to install out-of-tree cloud-provider-azure kubernetes-sigs/cluster-api-provider-azure#2209

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add official helm chart and repo #1306

add official helm chart and repo #1306

jackfrancis commented Mar 21, 2022

coveralls commented Mar 21, 2022 •

edited

Loading

CecileRobertMichon Mar 22, 2022

jackfrancis Mar 28, 2022

lzhecheng Mar 29, 2022

feiskyer Mar 30, 2022

MartinForReal commented Mar 23, 2022

lzhecheng left a comment

jackfrancis commented Mar 29, 2022

lzhecheng commented Mar 29, 2022

jackfrancis commented Mar 29, 2022

lzhecheng commented Mar 30, 2022

jackfrancis commented Mar 30, 2022

lzhecheng commented Mar 30, 2022

feiskyer Mar 30, 2022

jackfrancis Mar 30, 2022

lzhecheng Mar 30, 2022

lzhecheng commented Mar 30, 2022

jackfrancis Mar 30, 2022

feiskyer left a comment

k8s-ci-robot commented Mar 30, 2022

lzhecheng commented Mar 30, 2022

add official helm chart and repo #1306

add official helm chart and repo #1306

Conversation

jackfrancis commented Mar 21, 2022

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

coveralls commented Mar 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinForReal commented Mar 23, 2022

lzhecheng left a comment

Choose a reason for hiding this comment

jackfrancis commented Mar 29, 2022

lzhecheng commented Mar 29, 2022

jackfrancis commented Mar 29, 2022

lzhecheng commented Mar 30, 2022

jackfrancis commented Mar 30, 2022

lzhecheng commented Mar 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzhecheng commented Mar 30, 2022

Choose a reason for hiding this comment

feiskyer left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 30, 2022

lzhecheng commented Mar 30, 2022

coveralls commented Mar 21, 2022 •

edited

Loading