pkg/components: add Headlamp component #981

iaguis · 2020-09-17T12:48:05Z

Fixes #988

pkg/components/headlamp/component.go

assets/charts/components/headlamp/Chart.yaml

assets/charts/components/headlamp/values.yaml

invidian

Hmm, I think we should also include headlamp in CI configuration and have some basic e2e tests for it.

assets/charts/components/headlamp/Chart.yaml

assets/charts/components/headlamp/templates/NOTES.txt

assets/charts/components/headlamp/values.yaml

assets/charts/components/headlamp/templates/deployment.yaml

assets/charts/components/headlamp/values.yaml

pkg/components/headlamp/component.go

iaguis

Hmm, I think we should also include headlamp in CI configuration and have some basic e2e tests for it.

Will add basic tests.

surajssd · 2020-09-23T12:44:28Z

@iaguis can you address the reviews. Or do you mind someone taking it over?

invidian · 2020-09-30T09:07:32Z

@iaguis make sure to pull the reviewers again when you are done addressing review comments. Resolving existing conversations when they are addressed is also helpful while doing another rounds of reviews :)

iaguis · 2020-09-30T09:27:56Z

@iaguis make sure to pull the reviewers again when you are done addressing review comments. Resolving existing conversations when they are addressed is also helpful while doing another rounds of reviews :)

Yes, I was waiting for CI to pass and I didn't realize it was passing already :D

assets/charts/components/headlamp/templates/_helpers.tpl

invidian

Looks mostly good, though I'd change commits a bit, as right now it is difficult to review the PR commit by commit.

assets/charts/components/headlamp/templates/deployment.yaml

ci/aks/aks-cluster.lokocfg.envsubst

pkg/components/headlamp/component.go

assets/charts/components/headlamp/templates/deployment.yaml

docs/configuration-reference/components/headlamp.md

pkg/components/headlamp/component.go

knrt10

NIT picks. We are using unmarshaling in our test cases elsewhere.

pkg/components/headlamp/component_test.go

invidian

Awesome update @iaguis, I think there is still few bits which could be improved, but it looks very nice already!

docs/configuration-reference/components/headlamp.md

pkg/components/headlamp/component_test.go

test/ingress/ingress.go

go.mod

docs/configuration-reference/components/headlamp.md

pkg/components/headlamp/template.go

test/components/headlamp/headlamp_test.go

test/ingress/aws/aws_test.go

ci/aks/aks-cluster.lokocfg.envsubst

ci/aws/aws-cluster.lokocfg.envsubst

iaguis · 2020-10-13T14:21:48Z

Addressed review. I've moved it to the lokomotive-system namespace and dropped all capabilities too. PTAL.

test/components/headlamp/headlamp_test.go

test/ingress/headlamp/ingress_test.go

knrt10

Small test changes

invidian

Just one tiny nit, but I'm fine with it.

Nice work @iaguis, LGTM 👍 ✔️ 🎉

pkg/components/headlamp/component_test.go

It checks that the deployment is created and converges.

We weren't taking into account the test structure component name and were using the hardcoded "httpbin" string. This means we were only testing the httpbin component. This uses the component name from the test structure. Also, this changes the hardcoded "get" subpath to a parameter so we can access the right Prometheus Operator URL.

This refactors the HTTP access code so it can be reused later for the Headlamp ingress test.

knrt10

Nice work. Looks great 🔥

knrt10 reviewed Sep 17, 2020

View reviewed changes

pkg/components/headlamp/component.go Outdated Show resolved Hide resolved

knrt10 reviewed Sep 17, 2020

View reviewed changes

assets/charts/components/headlamp/Chart.yaml Show resolved Hide resolved

knrt10 reviewed Sep 17, 2020

View reviewed changes

assets/charts/components/headlamp/values.yaml Outdated Show resolved Hide resolved

invidian suggested changes Sep 17, 2020

View reviewed changes

iaguis commented Sep 21, 2020

View reviewed changes

iaguis force-pushed the iaguis/headlamp branch 3 times, most recently from 6aa6c7d to 8c693f2 Compare September 29, 2020 09:53

iaguis force-pushed the iaguis/headlamp branch from 8c693f2 to 45121b0 Compare September 30, 2020 11:01

iaguis requested review from invidian and knrt10 September 30, 2020 11:01

knrt10 reviewed Sep 30, 2020

View reviewed changes

assets/charts/components/headlamp/templates/_helpers.tpl Show resolved Hide resolved

invidian suggested changes Sep 30, 2020

View reviewed changes

iaguis force-pushed the iaguis/headlamp branch 7 times, most recently from f59eaf7 to e40715d Compare October 6, 2020 13:37

iaguis requested review from knrt10 and invidian October 6, 2020 13:38

knrt10 reviewed Oct 6, 2020

View reviewed changes

pkg/components/headlamp/component_test.go Show resolved Hide resolved

pkg/components/headlamp/component_test.go Show resolved Hide resolved

invidian suggested changes Oct 6, 2020

View reviewed changes

iaguis force-pushed the iaguis/headlamp branch 3 times, most recently from 7d3f101 to 2d9074e Compare October 12, 2020 12:52

iaguis requested review from invidian and knrt10 October 12, 2020 13:29

invidian suggested changes Oct 12, 2020

View reviewed changes

pkg/components: add Headlamp component

1752496

iaguis force-pushed the iaguis/headlamp branch from 2d9074e to c70187f Compare October 13, 2020 14:20

iaguis requested a review from invidian October 13, 2020 14:21

knrt10 reviewed Oct 13, 2020

View reviewed changes

test/components/headlamp/headlamp_test.go Outdated Show resolved Hide resolved

knrt10 reviewed Oct 13, 2020

View reviewed changes

test/ingress/headlamp/ingress_test.go Outdated Show resolved Hide resolved

knrt10 suggested changes Oct 13, 2020

View reviewed changes

iaguis force-pushed the iaguis/headlamp branch from c70187f to a615385 Compare October 13, 2020 15:09

invidian previously approved these changes Oct 13, 2020

View reviewed changes

pkg/components/headlamp/component_test.go Outdated Show resolved Hide resolved

iaguis added 8 commits October 13, 2020 17:54

pkg/headlamp: add unit test

b17f128

docs/components: update list of available components

b7e928f

headlamp: add simple deployment test

2c1d3c5

It checks that the deployment is created and converges.

test/ingress/aws: fix typo

3b1d1dc

test/ingress: refactor and fix bug

3f7893b

This refactors the HTTP access code so it can be reused later for the Headlamp ingress test.

test/ingress: test access to Headlamp

6ef3644

ci: add headlamp

983e618

iaguis dismissed invidian’s stale review via 983e618 October 13, 2020 15:54

iaguis force-pushed the iaguis/headlamp branch from a615385 to 983e618 Compare October 13, 2020 15:54

knrt10 approved these changes Oct 13, 2020

View reviewed changes

invidian approved these changes Oct 13, 2020

View reviewed changes

iaguis merged commit ac50c36 into master Oct 13, 2020

iaguis deleted the iaguis/headlamp branch October 13, 2020 18:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkg/components: add Headlamp component #981

pkg/components: add Headlamp component #981

iaguis commented Sep 17, 2020 •

edited

Loading

invidian left a comment

iaguis left a comment

surajssd commented Sep 23, 2020

invidian commented Sep 30, 2020

iaguis commented Sep 30, 2020

invidian left a comment

knrt10 left a comment •

edited

Loading

invidian left a comment

iaguis commented Oct 13, 2020

knrt10 left a comment

invidian left a comment

knrt10 left a comment

pkg/components: add Headlamp component #981

pkg/components: add Headlamp component #981

Conversation

iaguis commented Sep 17, 2020 • edited Loading

invidian left a comment

Choose a reason for hiding this comment

iaguis left a comment

Choose a reason for hiding this comment

surajssd commented Sep 23, 2020

invidian commented Sep 30, 2020

iaguis commented Sep 30, 2020

invidian left a comment

Choose a reason for hiding this comment

knrt10 left a comment • edited Loading

Choose a reason for hiding this comment

invidian left a comment

Choose a reason for hiding this comment

iaguis commented Oct 13, 2020

knrt10 left a comment

Choose a reason for hiding this comment

invidian left a comment

Choose a reason for hiding this comment

knrt10 left a comment

Choose a reason for hiding this comment

iaguis commented Sep 17, 2020 •

edited

Loading

knrt10 left a comment •

edited

Loading