Skip to content

adding helm-repo as github pages #744

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

Merged
merged 1 commit into from
Dec 6, 2018
Merged

adding helm-repo as github pages #744

merged 1 commit into from
Dec 6, 2018

Conversation

M00nF1sh
Copy link
Collaborator

@M00nF1sh M00nF1sh commented Nov 21, 2018

Changes done:

  1. Make our github pages as an helm repo as well 🤣 (History images are also maintained😆 )
  2. Partially resolve Feature Request: AWS ALB Ingress Controller Helm Chart in Helm repository #693

Test done:
An helm repo build via make docs-deploy, which contains different versions of alb-ingress-controller-helm: https://m00nf1sh.github.io/aws-alb-ingress-controller

How to install:
helm repo add aws-alb https://m00nf1sh.github.io/aws-alb-ingress-controller
helm repo update
helm repo install aws-alb/alb-ingress-controller-helm --name=test --set extraArgs.v=1 or
helm repo install aws-alb/alb-ingress-controller-helm --version=0.1.3 --name=test --set extraArgs.v=1

Source code of helm-repo plugin:
https://github.com/M00nF1sh/mkdocs-helm

Future work:

  1. Tune our helm charts.
  2. Update alb-ingress-controller-helm into alb-ingress-controller or aws-alb-ingress-controller, since the -helm suffix is not need after migrate off quay.io
  3. Update the docs for install instructions
  4. Move the chart to centralized helm repo :(

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 21, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: M00nF1sh

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 21, 2018
@Jeeppler
Copy link

@M00nF1sh I get an error while trying to install alb-ingress-controller via CLI using this command:

helm repo install aws-alb/alb-ingress-controller-helm --name=test --set extraArgs.v=1
Error: unknown flag: --name

the correct command is:

helm install aws-alb/alb-ingress-controller-helm --name=test --set extraArgs.v=1

and to check if the alb-ingress-controller is installed:

$ kubectl --namespace=default get pods -l "app=alb-ingress-controller,component=controller,release=test"

NAME                                                      READY     STATUS    RESTARTS   AGE
test-alb-ingress-controller-controller-8654c45bf5-28786   1/1       Running   0          1m

@Jeeppler
Copy link

I realized when I installed the chart like this:

helm install --name alb-ingress --namespace kube-system --set extraArgs.v=1 aws-alb/alb-ingress-controller-helm

The installation works, but the deployment name will be:

alb-ingress-alb-ingress-controller-controller

The name contains a lot of redundant terms.

@M00nF1sh
Copy link
Collaborator Author

M00nF1sh commented Nov 26, 2018

I realized when I installed the chart like this:

helm install --name alb-ingress --namespace kube-system --set extraArgs.v=1 aws-alb/alb-ingress-controller-helm

The installation works, but the deployment name will be:

alb-ingress-alb-ingress-controller-controller

The name contains a lot of redundant terms.

Hi, do you have suggestions for this 😄 , currently the deployment name is generated as 'releaseName-alb-ingress-controller'. I noticed other projects like helm used similar name pattern..

@rifelpet
Copy link
Contributor

The "helm create" command in 2.11.0 has updated the default _helpers.tpl functions which will address that redundancy. It also has different default labels. We may want to consider recreating the chart with helm 2.11.0.

@M00nF1sh
Copy link
Collaborator Author

M00nF1sh commented Nov 26, 2018

The "helm create" command in 2.11.0 has updated the default _helpers.tpl functions which will address that redundancy. It also has different default labels. We may want to consider recreating the chart with helm 2.11.0.

Nice! I'll recreate the helm chart.
BTW, the current helm chart is creating the resources in default namespace, should we change it to kube-system to align with our example yaml?

@rifelpet
Copy link
Contributor

Most charts that i've seen dont specify a namespace in their resource templates, allowing users to specify a namespace in their helm upgrade command. Maybe we include an example helm upgrade --namespace kube-system in the docs?

@Jeeppler
Copy link

I agree with @rifelpet, I would not specify the kube-system namespace in the template, but what is important for me is to be able to set the name and the namespace to what I want.

@M00nF1sh
Copy link
Collaborator Author

M00nF1sh commented Nov 26, 2018

Most charts that i've seen dont specify a namespace in their resource templates, allowing users to specify a namespace in their helm upgrade command. Maybe we include an example helm upgrade --namespace kube-system in the docs?

Cool! Then an example usage in README.md should be enough, thanks for the explanation! 👍

@bigkraig
Copy link

bigkraig commented Dec 6, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 6, 2018
@k8s-ci-robot k8s-ci-robot merged commit 68a0a74 into kubernetes-sigs:master Dec 6, 2018
@M00nF1sh M00nF1sh deleted the local_helm_repo branch December 7, 2018 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: AWS ALB Ingress Controller Helm Chart in Helm repository
5 participants