Skip to content

Helm Chart + bootstrap installation script for Harbor#3987

Closed
nuzz wants to merge 1 commit intogoharbor:masterfrom
nuzz:feature/helm_chart
Closed

Helm Chart + bootstrap installation script for Harbor#3987
nuzz wants to merge 1 commit intogoharbor:masterfrom
nuzz:feature/helm_chart

Conversation

@nuzz
Copy link
Copy Markdown

@nuzz nuzz commented Jan 9, 2018

A bootstrap script + Helm Chart to install a fully functioning Harbor registry on a Kubernetes cluster. This chart is intended to be provider agnostic. This installation has been tested on AWS (self managed k8s cluster), Google GKE, and Micorsoft AKS.

See README.md for more information

@vmwclabot
Copy link
Copy Markdown

@nuzz, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.3%) to 60.174% when pulling a4d2e8f on nuzz:feature/helm_chart into 5456c0b on vmware:master.

@reasonerjt
Copy link
Copy Markdown
Contributor

Thanks for the PR!
We're going to review this but please bear with the delay as there are other focus.

@reasonerjt reasonerjt self-assigned this Jan 10, 2018
{{ $key }}: {{ $value | quote }}
{{- end }}
spec:
type: LoadBalancer
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get a NodePort alternative as well? Maybe something like https://github.com/kubernetes/charts/blob/master/stable/postgresql/templates/svc.yaml?

- containerPort: 5432
name: postgres-port
protocol: TCP
volumeClaimTemplates:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to have storageClassName configurable. Defining that in annotations is being deprecated

config.yml: |+
version: 0.1
log:
level: info
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configurable log-level?

@xeor
Copy link
Copy Markdown

xeor commented Jan 10, 2018

Looking forward for this PR :) But can you also implement:

Some might be for a v2 :)

@paulczar
Copy link
Copy Markdown
Contributor

@nuzz I've also been working on helm charts for harbor, it looks like we've done fairly similar stuff but we've each made a bit more progress in different ways - https://github.com/paulczar/helm-harbor.

@paulczar
Copy link
Copy Markdown
Contributor

I'm not a big fan of the bootstrap scripts, I'd rather see defaults that make it work out of the box for minikube installations and documentation on what to change for different install environments.

I'm also not a huge fan of the seperate secrets file loading into a common namespace, I'd rather see the secrets stick to being part of the same namespace as the harbor install. I would also rather keep the secrets as values set in helm ( I know helm doesn't fully support proper secrets, but its coming real soon helm/helm#2721 ).

@reasonerjt
Copy link
Copy Markdown
Contributor

reasonerjt commented Feb 5, 2018

Two overall comments:

  1. I think we should use ingress instead of introducing nginx container in helm chart.
  2. Echoing paulczar, I don't think we want a bootstrap script. We should just provide default values in the yaml and provide the reference on how to modify/generate them. For deployment we should rely on helm to generate all necessary artifacts.

@paulczar
Copy link
Copy Markdown
Contributor

I've been working with the upstream helm charts team with my own harbor charts here - helm/charts#3383

@jessehu
Copy link
Copy Markdown
Contributor

jessehu commented Feb 12, 2018

Hello guys, I'm Jesse Hu from Harbor dev team, will in charge of making Harbor Helm Chart as a formal community feature into harbor github repo. Thanks very much to @paulczar @nuzz @xeor for sharing your harbor chart code, and the PR helm/charts#3383 for adding incubator/harbor chart.
I think the patch by @paulczar https://github.com/paulczar/helm-harbor is a good start, so could you @paulczar please send a formal pull request :)
The bootstrap script in @nuzz is also a good idea which is used to generate random password and SSL certificates. And I think we can consider using the chart template function to implement the bootstrap script https://masterminds.github.io/sprig/crypto.html, then it's a part of the harbor chart, not a standalone script outside harbor chart.
Any thought? :)

@jessehu
Copy link
Copy Markdown
Contributor

jessehu commented Feb 12, 2018

@paulczar I have a fork to make your code work with latest Harbor 1.4.0 release jessehu/helm-harbor@c2818c7

@vmwclabot
Copy link
Copy Markdown

@nuzz, your company's legal contact did not review your signed contributor license agreement within the 14 day limit. The merge can not proceed. Click here to resign the agreement.

@jessehu
Copy link
Copy Markdown
Contributor

jessehu commented Feb 17, 2018

We have merged #4271

@reasonerjt
Copy link
Copy Markdown
Contributor

Thanks, @paulczar, @jessehu and @nuzz
I'm closing this PR.

@reasonerjt reasonerjt closed this Feb 22, 2018
@jessehu
Copy link
Copy Markdown
Contributor

jessehu commented Mar 10, 2018

Hi @nuzz @xeor and other guys interested in Harbor Helm Chart, we have merged #4373 'Update Harbor helm chart to deploy Harbor 1.4.0 release' into harbor master branch https://github.com/vmware/harbor/tree/master/contrib/helm/harbor.

Welcome to try it out and your continuous idea and code contribution to Harbor are still appreciated. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants