operate-first / hitchhikers-guide

Hitchhiker's guide to Operate-first
GNU General Public License v3.0
3 stars 8 forks source link

Replace usage of `kustomize edit add resource` in the notebooks with an alternative #111

Closed HumairAK closed 2 years ago

HumairAK commented 3 years ago

kustomize cli will reformat the file and also append changes to at the bottom, this makes kustomization.yaml files like the ones here really hard to read (we alphabetically sort these for human readability).

One instance where this has already caused some issues can be found here: https://github.com/operate-first/apps/pull/1236

HumairAK commented 3 years ago

Also should update the markdowns that these instructions are taken from.

durandom commented 3 years ago

I suggest to not use the opfcli for that at all. Can't we use python to manipulate the yaml files? Or cat <<HERE documents. So that it's not hidden from the reader what is actually added to those files.

@tumido @fridex what's a good YAML manipulation package in python?

fridex commented 3 years ago

I suggest to not use the opfcli for that at all. Can't we use python to manipulate the yaml files? Or cat <<HERE documents. So that it's not hidden from the reader what is actually added to those files.

@tumido @fridex what's a good YAML manipulation package in python?

PyYAML is a library to manipulate content in YAML files in Python. It does its job on the content level, not on the formatting level (if that is relevant here).

tumido commented 2 years ago

This is a never ending story and ongoing issue across all history of Operate First. I don't think this is a valid issue.

Yes, kustomize doesn't care about alphabetical order, instead it raises error on duplicate resources and supports a remove command. I think we should scrape this alphabetical order idea and rather embrace the component collections proposed by @larsks at https://github.com/operate-first/SRE/issues/369 We can either work hard on tooling and workarounds and scripts to support manual processing of automatically genereated resources or rather focus on going fully automated. I think alphabetical sorting is not important to the machines so we should rather embrace it and work towards more automated solution with less moving parts.

If a standardized tool formats a resource, there's a high chance that the output format is deterministic and if we apply the same tool on the resource again, it results in 0 diff. So if a tool prefers a formatting and we as maintainers can accept that formatting, we should start using it and favor it instead of battling the tools every time on every PR.

Yes, YAML is a whitespaced markup language, but in different flavors/versions/standards it uses different amount of spaces to indent nested properties. Actually even different versions of kustomize as well as kubectl and oc will use either 2 or 4 spaces to indent - all rely on go-yaml library and this is a loong story on its own. There's nothing we should be doing in this case - because patching an already patched tool for a hacked solution is not good pattern. https://github.com/kubernetes-sigs/kustomize/issues/3946 Either go fix it upstream or live with it.

We have a nice saying in Czech for this, not sure about the equivalent in English - "vyrábět narovnávák na ohejbák" - to create a "straightening tool" for a "bender tool" meaning that at first you work hard to bend something and then work even harder to straighten it again... This issue seems like it's suggesting such approach. I think we should invest our energy elsewhere.

tumido commented 2 years ago

I suggest to not use the opfcli for that at all.

:+1: to that... less custom tooling is always better.

sesheta commented 2 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

sesheta commented 2 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle rotten

sesheta commented 2 years ago

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten.

/close

sesheta commented 2 years ago

@sesheta: Closing this issue.

In response to [this](https://github.com/operate-first/hitchhikers-guide/issues/111#issuecomment-1093096293): >Rotten issues close after 30d of inactivity. >Reopen the issue with `/reopen`. >Mark the issue as fresh with `/remove-lifecycle rotten`. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.