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

New python_format_string codegen for kubernetes #22034

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

grihabor
Copy link
Contributor

@grihabor grihabor commented Mar 3, 2025

The pr introduces a new python_format_string target that takes a python format string as an input and produces the output by substituting the variables into it. Right now it only produces k8s_sources, but it can be extended in the future.

Merge after #22040

@grihabor grihabor force-pushed the python-format-string-backend branch from 5996239 to 8bf173c Compare March 4, 2025 11:40
@tgolsson
Copy link
Contributor

tgolsson commented Mar 4, 2025

Isn't this at risk of overlapping quite a bit with kustomize? F.ex. the namespace example looks like it could be done more easily with two different kustomization.yaml.

@grihabor
Copy link
Contributor Author

grihabor commented Mar 5, 2025

Isn't this at risk of overlapping quite a bit with kustomize?

There are some things that you could both ways, but kustomize is a much more powerful tool that can do more things. Here python_format_string is just a simple way to pass variables from BUILD files into the kubernetes sources.

the namespace example looks like it could be done more easily with two different kustomization.yaml.

Yeah, it's just a simple example to demonstrate what you could do. The important use case is in docker images section

@grihabor grihabor force-pushed the python-format-string-backend branch from f9f68ad to 74477a3 Compare March 5, 2025 11:45
@grihabor grihabor mentioned this pull request Mar 5, 2025
@grihabor grihabor marked this pull request as ready for review March 5, 2025 12:07
@tgolsson
Copy link
Contributor

tgolsson commented Mar 5, 2025

the namespace example looks like it could be done more easily with two different kustomization.yaml.

Yeah, it's just a simple example to demonstrate what you could do. The important use case is in docker images section

Yeah. Doing that with kustomization is potentially not as simple, especially with tags. With sha-based usage, it can be done without an intermediate target as well 1. Have you seen how eg rules_gitops handles this? It's quite similar to what I do for my k8s plugin. And then that feeds into kustomization, with the images override section.

https://github.com/tgolsson/pants-backends/blob/66c98154d966a8ff91b9f832b4b0e8ced46cacee/examples/kustomize/welcome/base/kustomization.yaml#L11-L14

Footnotes

  1. WELL, I guess technically it doesn't matter if the override is a tag or a sha, the issue is driving it from the env. 🤷 But it's only a thing you'd consider doing if you use tags.

@grihabor
Copy link
Contributor Author

grihabor commented Mar 6, 2025

Have you seen how eg rules_gitops handles this?

spec:
  containers:
    - image: //examples:helloworld_image

Not until this point, thanks. Looks like it's just a more complicated way to do the same thing. Instead of a simple substitution, you now have to parse yaml and search for something. Users are now also in a bad spot - you can't just look at the yaml and figure out which image gets used, you have to go and find the target that defines the image.

One benefit is that you can make sure that you're using the correct image, but you can achieve this with substitution:

image_name = "my-image"
docker_image(name=image_name)
python_format_string(source='deployment.yaml', values={'VERSION': env('VERSION'), 'image_name': image_name}
image: {image_name}:{VERSION}

Alternatively, you can implement a linter that would parse the yaml and check, that your image names are sound. The image name has to be globally unique, so you can confidently map image names to targets.

We have a huge number of k8s objects, so I want the backend to be as simple and as fast as possible. It's ok to parse yaml in a linter, but it's not ok to parse it on deploy.

@tgolsson
Copy link
Contributor

tgolsson commented Mar 6, 2025

Looks like it's just a more complicated way to do the same thing. Instead of a simple substitution, you now have to parse yaml and search for something.

Funnily enough, I find your method more complicated. I also only do simple search-replace, though I'm not sure what rules_gitops does in its implementation. Only difference being I use the target address instead of a placeholder. ;-) But it's definitely more custom, and a bit more magic.

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.

2 participants