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

Gojsonnet inside the kapicorp/kapitan:v0.30.0-rc.0 docker image ? #778

Open
jperville opened this issue Aug 16, 2021 · 17 comments
Open

Gojsonnet inside the kapicorp/kapitan:v0.30.0-rc.0 docker image ? #778

jperville opened this issue Aug 16, 2021 · 17 comments
Labels

Comments

@jperville
Copy link
Contributor

Describe the bug/feature

The release notes for v0.30.0-rc.0 say that gojsonnet support has been merged, yet the kapicorp/kapitan:v0.30.0-rc.0 docker image does not embed the gojsonnet python package.

To Reproduce

# check for gojsonnet support inside the kapitan source
docker run --rm -ti -w /opt/venv --entrypoint /bin/bash kapicorp/kapitan:v0.30.0-rc.0 -ce 'grep -r gojsonnet .'
# check for the gojsonnet library inside the kapitan virtualenv
docker run --rm -ti -w /opt/venv --entrypoint /bin/bash kapicorp/kapitan:v0.30.0-rc.0 -ce 'find | grep gojsonnet'

Expected behavior

We can see that the current kapitan source in /opt/venv has the support for gojsonnet as documented in the release notes.
However the gojsonnet python module is not present in the image (the find command above should return it somewhere in the python3.7 site-packages) resulting in gojsonnet support not being available in the docker image.

** If it's a bug (please complete the following information):**

  • python --version: Python 3.7.11
  • pip3 --version: pip 21.2.4 from /opt/venv/lib/python3.7/site-packages/pip (python 3.7)
  • Are you using pyenv or virtualenv?: using virtualenv inside the official docker image

Additional context

I tested this rc.0 image to play with the gojsonnet support.
I have high hopes that gojsonnet can help reduce significantly the compilation time of my project.

@ramaro
Copy link
Member

ramaro commented Aug 16, 2021

Hey @jperville at the moment this is still optional support in the sense that gojsonnet will be used if the module is present in the system. So that is why you won't see it installed in the docker image, but if using pip you can install gojsonnet additionally. Maybe going forward it makes sense to have a flag in compile to use gojsonnet instead?

@ramaro
Copy link
Member

ramaro commented Aug 16, 2021

Another option would be to support gojsonnet and jsonnet types in the compile item configuration.

@ramaro
Copy link
Member

ramaro commented Aug 16, 2021

@pvanderlinden @simu @janeklb what do you think?

@janeklb
Copy link
Contributor

janeklb commented Aug 16, 2021

Could we publish a separate docker image with gojsonnet? Or always include the gojsonnet package but have a cli flag to enable it? Happy to submit a PR for the latter

@janeklb
Copy link
Contributor

janeklb commented Aug 16, 2021

Another option would be to support gojsonnet and jsonnet types in the compile item configuration.

I don’t think it makes sense to differentiate the configuration type based on the jsonnet implementation

@pvanderlinden
Copy link
Contributor

pvanderlinden commented Aug 16, 2021

I think the main issue with gojsonnet is that it might be to complicated for some to install? With a docker image the user would not have that issue, so maybe it would be good to just have gojsonnet as a default in the docker image? Unless @janeklb thinks it needs more testing in the wild before making it the default.

@jperville
Copy link
Contributor Author

Thank you everyone for the feedback, looking forward to having a jsonnet-enabled docker image to test.

@janeklb
Copy link
Contributor

janeklb commented Aug 16, 2021

Imo we have the following options:

  1. Ship an additional gojsonnet-enabled docker image (eg. kapicorp/kapitan:0.30.0-gojsonnet, or maybe even more official via a new image name kapikorp/kapitan-gojsonnet)
  2. Always install gojsonnet (ie, take it out of setup.py's extras section) and allow users to opt in via something like a --gojsonnet cli flag or USE_GO_JSONNET=true envar (prob not a good idea cause it would mean building the gojsonnet python package, which requiress go etc.)
  3. Ship kapitan with gojsonnet built in from the start

While i've personally been using the gojsonnet variant for a good while now, I'm not confident that my usage has exercised all the edge-cases that may or may not cause problems under the go implementation of jsonnet. My preference would be to ship it via option 1 or 2 - I don't mind which.

Re users installing it locally (ie. pip install kapitan), the solution there is just to run pip install gojsonnet==<whatever-version-we-use> in the same python environment, and it'll automatically work 🎉 work as long as they have golang installed etc

@jperville
Copy link
Contributor Author

I tried to install gojsonnet inside the kapitan 0.30.0.rc.0 docker image, not that easy:

docker run --rm -ti -u 0 -w /opt/venv --entrypoint /bin/bash kapicorp/kapitan:v0.30.0-rc.0 -ce '
apt-get update && apt-get install -qy golang-go && export GOPATH=/opt
source bin/activate
pip3 install gojsonnet==0.17.0

Even if I manually install the golang-go package and go get github.com/google/go-jsonnet in the GOPATH,
the build of go-jsonnet fails. I think that there should be extra instructions in the Dockerfile to build with go-jsonnet support.

Since go adds a lot of dependencies I vote for option 1 (additional gojsonnet-enabled docker image).

@simu
Copy link
Contributor

simu commented Aug 16, 2021

Another option would be to support gojsonnet and jsonnet types in the compile item configuration.

I don’t think it makes sense to differentiate the configuration type based on the jsonnet implementation

I agree that it doesn't make sense to differentiate the compile type based on the Jsonnet implementation.

Otherwise we (https://github.com/projectsyn) are not affected by the discussion/decision here, as we're building our own Docker images.

@janeklb
Copy link
Contributor

janeklb commented Aug 16, 2021

I've opened up a poc PR (#779) to show how it could work, but we'll need to update the github actions as well as the Dockerfile.ci file to accommodate this.

@janeklb
Copy link
Contributor

janeklb commented Aug 16, 2021

docker run --rm -ti -u 0 -w /opt/venv --entrypoint /bin/bash kapicorp/kapitan:v0.30.0-rc.0 -ce '
apt-get update && apt-get install -qy golang-go && export GOPATH=/opt
source bin/activate
pip3 install gojsonnet==0.17.0

@jperville reason the above doesn't work is cause debian stretch only has golang 1.7.x available :(

@ramaro
Copy link
Member

ramaro commented Oct 30, 2021

@janeklb @simu @jperville I still believe there's value in keeping both jsonnet and gojsonnet. I feel that perhaps the way to go is to either introduce a compile flag (e.g. --use-go-jsonnet) or and env var (e.g. USE_GO_JSONNET=1). #779 is a good idea but the decision to package a specific version at build might be problematic, I think.

wdty?

@janeklb
Copy link
Contributor

janeklb commented Oct 30, 2021

I feel that perhaps the way to go is to either introduce a compile flag (e.g. --use-go-jsonnet) or and env var (e.g. USE_GO_JSONNET=1).

For the dockerised version that would make sense; however, imo it would result in a worse user experience for those using the python module directly. Why? In order for that flag to work, gojsonnet must already be installed into your python env, and in order to install gojsonnet you also need golang. This means that kapitan would not be pip-installable on environments where a recent version of golang is not available.

That said, if you're looking to primarily distribute kapitan as a docker image, then that might make sense since you would have full control over what you bundle into the image (though I would selfishly discourage that because kapitan via docker is not my preferred way of compiling locally due to docker volume mount performance).

Another option could be to not list gojsonnet as a python module dependency, and error out if the --use-go-jsonnet flag is passed but gojsonnet is not available. This would mean re-working the current import mechanism; however, it does have the (big) benefit of being explicit rather than implicit.

In terms of what kapitan's "default" jsonnet implementation should be.. maybe start by offering gojsonnet as the new alternative, and then eventually (in X versions?) make the c++ bindings version as the "old/deprecated" option. Ideally, if gojsonnet becomes pip-installable with ease, and gojsonnet has 100% feature parity with jsonnet (which it may already?), the old/deprecated version could actually be completely dropped.

#779 is a good idea but the decision to package a specific version at build might be problematic, I think.

Please elaborate... I'm now curious why it might be problematic :)

@janeklb
Copy link
Contributor

janeklb commented Oct 30, 2021

To keep it simple for now I would suggest the following as a release:

  • python module 0.30.0 with an explicit dependency on c++ jsonnet (same as 0.29.5). This enables folks to opt-in by installing gosjonnet into their env, but doesn't cause problems for environments without go.
    • you could have pip install kapitan==0.30.0 gojsonnet=0.17.0 and --use-go-jsonnet in the README for folks that want to try it
  • two docker images: 0.30.0 (c++ jsonnet), and 0.30.0-gojsonnet incase the gojsonnet image is significantly bigger and there's an appetite for slim kapitan images.

@ramaro
Copy link
Member

ramaro commented Nov 19, 2021

@janeklb very valid points! Let's go with that 👍🏼

Copy link

This issue is stale because it has been open for 1 year with no activity.
Remove the stale label or comment if this issue is still relevant for you.
If not, please close it yourself.

@github-actions github-actions bot added the Stale label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants