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

Enforce parity between Jsonnet and Helm #2067

Merged
merged 42 commits into from
Jul 22, 2022
Merged

Conversation

Logiraptor
Copy link
Contributor

@Logiraptor Logiraptor commented Jun 9, 2022

What this PR does

Use kustomize to compare Jsonnet with Helm automatically.

See the section in docs for more information.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@Logiraptor Logiraptor force-pushed the logiraptor/use-k8s-diff branch 2 times, most recently from b176096 to 328d307 Compare June 14, 2022 18:21
@Logiraptor Logiraptor force-pushed the logiraptor/use-k8s-diff branch 2 times, most recently from 0368970 to 6778644 Compare June 14, 2022 20:35
@Logiraptor Logiraptor changed the title Logiraptor/use k8s diff Enforce parity between Jsonnet and Helm Jun 15, 2022
@Logiraptor Logiraptor force-pushed the logiraptor/use-k8s-diff branch 2 times, most recently from 1346d82 to 535f176 Compare June 16, 2022 21:26
@Logiraptor Logiraptor force-pushed the logiraptor/use-k8s-diff branch from 7ae9392 to e9eba30 Compare June 22, 2022 19:07
@Logiraptor Logiraptor marked this pull request as ready for review June 23, 2022 14:34
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

that's how far i got today

Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

LGTM, it's a hefty piece of work

metadata:
name: mimir-config

# TODO(logiraptor): Helm adds a grpc port to the overrides exporter, Jsonnet does not.
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov Jun 24, 2022

Choose a reason for hiding this comment

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

Are you calling dibs on all the TODOs? /s

Comment on lines +202 to +205
flagSet.Bool("config.expand-env", false, "")
flagSet.Int("mem-ballast-size-bytes", 0, "")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these two needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment to help explain. It's a little hacky tbh. Basically these are the only two used in either jsonnet or helm that aren't defined in the configuration struct.

@krajorama krajorama force-pushed the logiraptor/use-k8s-diff branch from ce8860e to 6e100d8 Compare July 21, 2022 07:31
@krajorama
Copy link
Contributor

trivial rebase

Copy link
Contributor

@krajorama krajorama left a comment

Choose a reason for hiding this comment

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

Some nits and a question about the Makefile

Makefile Outdated Show resolved Hide resolved
@Logiraptor Logiraptor requested a review from krajorama July 21, 2022 22:13
@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2022

Helm <> Jsonnet Diff

🎉 No differences where detected between Helm and Jsonnet 🎉

@github-actions

This comment was marked as duplicate.

@Logiraptor Logiraptor enabled auto-merge (squash) July 22, 2022 21:04
@Logiraptor Logiraptor merged commit 0380c6d into main Jul 22, 2022
@Logiraptor Logiraptor deleted the logiraptor/use-k8s-diff branch July 22, 2022 21:09
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.

5 participants