Addon: add headlamp #7

yolossn · 2022-06-02T11:45:48Z

Signed-off-by: Santhosh Nagaraj S santhosh@kinvolk.io

illume

Awesome.

I didn't try it yet (I will in a while!). Been looking forward to using this :)

Where is the description of the addon? Probably this needs to be added somewhere.
Why would this be added as well as the dashboard addon? Probably write that it's extensible dashboard.

go.mod

pkg/addons/config.go

joaquimrocha

Pretty cool! What would be the commands for users to be able to expose/access Headlamp in the browser? Have you checked whether we can expose an endpoint, so we can minikube addons open headlamp?

I'm not sure this is possible but my idea is that new k8s users could set up Headlamp and access it in the browser without any prior knowledge of how to expose services in k8s.

cmd/minikube/cmd/config/enable.go

yolossn · 2022-06-06T08:53:08Z

Where is the description of the addon? Probably this needs to be added somewhere.

The description should be added in the upstream PR.

Why would this be added as well as the dashboard addon? Probably write that it's extensible dashboard.

Will include this in the upstream PR description 👍🏽

illume · 2022-06-06T15:17:26Z

The description should be added in the upstream PR.

Cool.

A few other things for your consideration:

I found this page about addons: https://minikube.sigs.k8s.io/docs/contrib/addons/ I guess you saw it already? There's some things about testing it in there. I'm not sure if there's some other tests we need to add for the addon? There is test/integration/addons_test.go which has integration tests for other addons.
I saw this PR review process posted by bots into other Minikube PRs. Probably worth checking out if you haven't seen it already.
Addons can have a documentation page in this path: https://github.com/kubernetes/minikube/tree/master/site/content/en/docs/handbook/addons I suggest adding one to the PR. Maybe the structure of these other addon docs (and the dashboard docs) could be used to guide documentation for this addon.
The dashboard addon documentation had something interesting... it had a --url option. Perhaps we could also have that to match the dashboard addon?
I saw this upcoming PR Add doc URL to the addon list table kubernetes/minikube#14123 which might require changes from this PR (adding a link to the docs).
Some PRs for other merged addons: kong addon [Addon] Kong Ingress Controller kubernetes/minikube#13326), portainer, CSI Hostpath Driver & VolumeSnapshots addons. There are some unmerged addons too.
This upcoming security scanning PR is interesting. They are going to be testing addon images for reported security issues.

yolossn · 2022-06-08T15:16:28Z

deploy/addons/headlamp/README.md

@@ -0,0 +1,41 @@
+## Headlamp Addon
+[Headlamp](https://github.com/kinvolk/headlamp) - Headlamp is an easy-to-use and extensible Kubernetes web UI.


Have added documentation for the addon.

BTW, why not just add the link + the description: Headlamp is an easy-to-use ....

I have also noticed that apparently other addons don't have a README.md file, so not sure if they will accept it for us.

yolossn · 2022-06-08T16:50:07Z

Addons can have a documentation page in this path: https://github.com/kubernetes/minikube/tree/master/site/content/en/docs/handbook/addons I suggest adding one to the PR. Maybe the structure of these other addon docs (and the dashboard docs) could be used to guide documentation for this addon.

Done

The dashboard addon documentation had something interesting... it had a --url option. Perhaps we could also have that to match the dashboard addon?

This looks custom made for the dashboard command, I don't think something like this is possible unless we propose a headlamp command.

I saw this upcoming PR Add doc URL to the addon list table kubernetes/minikube#14123 which might require changes from this PR (adding a link to the docs).

Great, have added README for headlamp too 👍🏽

Some PRs for other merged addons: kong addon [Addon] Kong Ingress Controller kubernetes/minikube#13326), portainer, CSI Hostpath Driver & VolumeSnapshots addons. There are some unmerged addons too.

👍🏽

This upcoming security scanning PR is interesting. They are going to be testing addon images for reported security issues.

Interesting!

joaquimrocha

It's looking very good. I found some issues related to language, so left some comments.
Also, I see you added the volume claim but I haven't seen instructions on how to set up the plugins. If the instructions should be in headlamp's docs, maybe we shouldn't have the volume claim here and just have instructions on how to set up plugins in a generic (in-cluster) way and point minikube's users there on a successful addon enabling.

cmd/minikube/cmd/config/enable.go

joaquimrocha · 2022-06-08T19:57:57Z

cmd/minikube/cmd/config/enable.go

+export SECRET=$(kubectl get secrets --namespace headlamp -o custom-columns=":metadata.name" | grep "headlamp-token")
+kubectl get secret $SECRET --namespace headlamp --template=\{\{.data.token\}\} | base64 --decode


This is slightly different from what we have as instructions in https://kinvolk.github.io/headlamp/docs/latest/installation/#create-a-service-account-token .
Should be make more similar? (the current way looks good, just wonder if there's a way to approximate either case)

I think it makes sense to update the documentation as it involves the user to copy the headlamp-token name, whereas this solution can be run directly.

deploy/addons/headlamp/README.md

cmd/minikube/cmd/config/enable.go

deploy/addons/headlamp/README.md

joaquimrocha · 2022-06-08T20:08:12Z

deploy/addons/headlamp/README.md

@@ -0,0 +1,41 @@
+## Headlamp Addon
+[Headlamp](https://github.com/kinvolk/headlamp) - Headlamp is an easy-to-use and extensible Kubernetes web UI.


BTW, why not just add the link + the description: Headlamp is an easy-to-use ....

joaquimrocha · 2022-06-08T20:09:08Z

deploy/addons/headlamp/README.md

@@ -0,0 +1,41 @@
+## Headlamp Addon
+[Headlamp](https://github.com/kinvolk/headlamp) - Headlamp is an easy-to-use and extensible Kubernetes web UI.


I have also noticed that apparently other addons don't have a README.md file, so not sure if they will accept it for us.

site/content/en/docs/handbook/addons/headlamp.md

illume · 2022-06-08T21:37:31Z

The dashboard addon documentation had something interesting... it had a --url option. Perhaps we could also have that to match the dashboard addon?

This looks custom made for the dashboard command, I don't think something like this is possible unless we propose a headlamp command.

Seems to be some other optional addons have a command. Like this gvisor one: kubernetes#3399 In perhaps a future PR I think it's worth asking about adding an optional headlamp cmd... perhaps on a future PR. Being able to run code could be useful for making some of these things run as smoothly as minkube dashboard.

ps. The I noticed the service command works with --url similar to the minikube dashboard --url.

illume

lgtm 🎈🎉

Signed-off-by: Santhosh Nagaraj S <santhosh@kinvolk.io>

illume reviewed Jun 2, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

pkg/addons/config.go Outdated Show resolved Hide resolved

joaquimrocha requested changes Jun 2, 2022

View reviewed changes

cmd/minikube/cmd/config/enable.go Outdated Show resolved Hide resolved

cmd/minikube/cmd/config/enable.go Show resolved Hide resolved

cmd/minikube/cmd/config/enable.go Outdated Show resolved Hide resolved

yolossn force-pushed the headlamp_addon branch 2 times, most recently from 5700408 to 2f6af6d Compare June 6, 2022 07:18

yolossn requested review from joaquimrocha and illume June 6, 2022 09:24

yolossn force-pushed the headlamp_addon branch 4 times, most recently from 47f46c6 to afae655 Compare June 8, 2022 15:14

yolossn commented Jun 8, 2022

View reviewed changes

yolossn force-pushed the headlamp_addon branch from afae655 to 9051780 Compare June 8, 2022 16:38

joaquimrocha requested changes Jun 8, 2022

View reviewed changes

yolossn force-pushed the headlamp_addon branch 2 times, most recently from a729874 to cd455e5 Compare June 9, 2022 04:27

illume approved these changes Jun 9, 2022

View reviewed changes

Addon: add headlamp

1fdd8db

Signed-off-by: Santhosh Nagaraj S <santhosh@kinvolk.io>

yolossn force-pushed the headlamp_addon branch from cd455e5 to 1fdd8db Compare June 10, 2022 05:46

yolossn closed this Jun 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addon: add headlamp #7

Addon: add headlamp #7

yolossn commented Jun 2, 2022

illume left a comment

joaquimrocha left a comment

yolossn commented Jun 6, 2022 •

edited

Loading

illume commented Jun 6, 2022 •

edited

Loading

yolossn Jun 8, 2022

joaquimrocha Jun 8, 2022

joaquimrocha Jun 8, 2022

yolossn commented Jun 8, 2022

joaquimrocha left a comment

joaquimrocha Jun 8, 2022

yolossn Jun 9, 2022

joaquimrocha Jun 8, 2022

joaquimrocha Jun 8, 2022

illume commented Jun 8, 2022

illume left a comment

		@@ -0,0 +1,41 @@
		## Headlamp Addon
		[Headlamp](https://github.com/kinvolk/headlamp) - Headlamp is an easy-to-use and extensible Kubernetes web UI.

		export SECRET=$(kubectl get secrets --namespace headlamp -o custom-columns=":metadata.name" \| grep "headlamp-token")
		kubectl get secret $SECRET --namespace headlamp --template=\{\{.data.token\}\} \| base64 --decode

Addon: add headlamp #7

Addon: add headlamp #7

Conversation

yolossn commented Jun 2, 2022

illume left a comment

Choose a reason for hiding this comment

joaquimrocha left a comment

Choose a reason for hiding this comment

yolossn commented Jun 6, 2022 • edited Loading

illume commented Jun 6, 2022 • edited Loading

yolossn Jun 8, 2022

Choose a reason for hiding this comment

joaquimrocha Jun 8, 2022

Choose a reason for hiding this comment

joaquimrocha Jun 8, 2022

Choose a reason for hiding this comment

yolossn commented Jun 8, 2022

joaquimrocha left a comment

Choose a reason for hiding this comment

joaquimrocha Jun 8, 2022

Choose a reason for hiding this comment

yolossn Jun 9, 2022

Choose a reason for hiding this comment

joaquimrocha Jun 8, 2022

Choose a reason for hiding this comment

joaquimrocha Jun 8, 2022

Choose a reason for hiding this comment

illume commented Jun 8, 2022

illume left a comment

Choose a reason for hiding this comment

yolossn commented Jun 6, 2022 •

edited

Loading

illume commented Jun 6, 2022 •

edited

Loading