[kube-prometheus-stack][tooling] Migrate the jsonnet tooling to use native golang scripts
What this PR does / why we need it
Replacing the Python based (which is really C<->C++) jsonnet tooling with Golang native tooling.
Which issue this PR fixes
- Addresses: https://github.com/prometheus-community/helm-charts/issues/5852
Special notes for your reviewer
Checklist
- [x] DCO signed
- [n/a] Chart Version bumped - Not sure if needs to be bumped as no effective changes were made?
- [x] Title of the PR starts with chart name (e.g.
[prometheus-couchdb-exporter])
I have mixed feeling here. I agreed that the python code was spaghetti code, but not sure how many kube-prometheus-stack maintainers can maintain go code.
If agreed, I will do a deeper review here.
I have mixed feeling here. I agreed that the python code was spaghetti code, but not sure how many kube-prometheus-stack maintainers can maintain go code.
If agreed, I will do a deeper review here.
Same feeling, but in reality the entire ecosystem is in go (k8s, helm, prometheus, alertmanager, etc) so I think it makes sense.
In the end it's all about opinion. For the record I prefer golang, and I use it pretty much every day. I can also review this later.
I also use go on a daily basis but I would tend to not change things that work but if more people want to do that, it's fine for me and we can review and maintain this
The main issue with this PR is that's it's a big bang and it's doing too much. 85 changed files and more than 3000 added lines is too much.
If @jkroepke and @GMartinez-Sisti are fine with this (wait for their response) then I would prefer that you split this PR into multiple ones with at least:
- 1 for skaffolding
- 1 for rules and switching to use the new go one
- 1 for dashboards and switching to use the new go one
- 1 to update the resources
then I would prefer that you split this PR into multiple ones
This is always a good approach, so I support it.
Also I would only expect the checksum on the header of the generated files to change, the files have changes on the content and github doesn't make it easy to understand what actually changed.
the files have changes on the content and github doesn't make it easy to understand what actually changed.
The only changes to files are superficial ones related to how yaml is marshalled. Of note:
- k8s-coredns.yaml path is corrected - the python relative path was incorrect for source VS dest paths. (So this fixed a bug)
- All dashboards have a superficial change of field order change (golang vs python order of
allValuefield) - Some
descriptionfields use different yaml string format; I think this is because python was splitting lines at 1000 characters.
So it will be very hard to get an exact 1:1 result out of golang and python due to subtle differences in yaml library implementations.
then I would prefer that you split this PR into multiple ones
This is always a good approach, so I support it.
TBH I wasn't sure how much interest there would be in this, so didn't know how to prepare it. I'm happy to split up this effort into multiple PRs if that's the preferred way to digest it.
As mentioned this all came from an idea to add this to internal tooling my team uses, so pulling it out of the original source where I created it is why it's quite a big bang PR. As I have time this week, I will split up this PR into the suggested PRs and close this one.
Hi, could you please skip the changes on the template for now? I'm unable to perform a review. Somehow the Github UI is terrible slow on the file diffs.
Appreciate the reviews and feedback - just wanted to clarify my game plan here. I do want to keep this moving too; but actual work projects came up & I over looked the holiday, so both those delayed any progress on this so far.
🤞 I'm hoping this week will be better for finding time to prioritize this. I had already started splitting up the changes based on this feedback. So I wasn't planning on directly modifying or submitting this PR further.
- 1 for skaffolding
- 1 for rules and switching to use the new go one
- 1 for dashboards and switching to use the new go one
- 1 to update the resources
Sorry all, I haven't been able to circle back to allocate time to this community work. Hopefully in the coming weeks 🤞 .