Skip to content
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

viz: Add HA Option through CLI #5470

Merged
merged 4 commits into from
Jan 7, 2021
Merged

viz: Add HA Option through CLI #5470

merged 4 commits into from
Jan 7, 2021

Conversation

Pothulapati
Copy link
Contributor

@Pothulapati Pothulapati commented Jan 4, 2021

This PR adds --ha flag for viz install which overrides with
the values-ha.yaml of the viz chart. This PR adds these functions
in pkg/charts so that the same can be re-used elsewhere.

Testing

tarun in dev in on  k3d-deep () linkerd2 on  tarun/viz-ha-nits [$?] via 🐹 v1.15.4 took 2s
❯ ./bin/go-run cli viz install | grep 1024

tarun in dev in on  k3d-deep () linkerd2 on  tarun/viz-ha-nits [$?] via 🐹 v1.15.4 took 2s
❯ ./bin/go-run cli viz install --ha | grep 1024
            memory: "1024Mi"

tarun in dev in on  k3d-deep () linkerd2 on  tarun/viz-ha-nits [$?] via 🐹 v1.15.4 took 2s
❯ ./bin/go-run cli viz install --ha --set grafana.resources.memory.limit=1023Mi | grep 1023
            memory: "1023Mi"

Signed-off-by: Tarun Pothulapati [email protected]

This PR adds `--ha` flag for `viz install` which overrides with
the `values-ha.yaml` of the viz chart. This PR adds these functions
in `pkg/charts` so that the same can be re-used elsewhere.

Signed-off-by: Tarun Pothulapati <[email protected]>
@Pothulapati Pothulapati requested a review from a team as a code owner January 4, 2021 14:52
Comment on lines +197 to +215
// MergeMaps returns the merge result of two maps
func MergeMaps(a, b map[string]interface{}) map[string]interface{} {
out := make(map[string]interface{}, len(a))
for k, v := range a {
out[k] = v
}
for k, v := range b {
if v, ok := v.(map[string]interface{}); ok {
if bv, ok := out[k]; ok {
if bv, ok := bv.(map[string]interface{}); ok {
out[k] = MergeMaps(bv, v)
continue
}
}
}
out[k] = v
}
return out
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this should depend #5433 since it adds the same function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes 👍

pkg/charts/charts.go Outdated Show resolved Hide resolved
Signed-off-by: Tarun Pothulapati <[email protected]>
Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@cpretzer cpretzer left a comment

Choose a reason for hiding this comment

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

LGTM, all tests passed locally

@kleimkuhler kleimkuhler merged commit 8e3a7d7 into main Jan 7, 2021
@kleimkuhler kleimkuhler deleted the tarun/viz-ha-nits branch January 7, 2021 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants