buildbox: add new helm unittest plugin fork#18945
Conversation
There was a problem hiding this comment.
This will break the Drone as the image update also requires updating the .drone.yml (check the comment above).
We've also never published .1 release on master. Any changes to solve this issue without changing the image tag?
There was a problem hiding this comment.
@r0mant also challenged me on the image tag. Changing the helm-unittest binary in the current buildbox tag will cause all pipelines from the past 3 weeks to be non-retyable and all commits on master between the v11 -> v12 change and this commit would have make test failing.
For the drone part I wanted to build the image in this PR but start using it in the next one. The image needs to be here for the tests to pass, but the changing the image requires code change as 12 and 12.1 are not compatible and it would break tests.
There was a problem hiding this comment.
Yeah, I think updating buildbox version here is not too bad. We gotta have some way to introduce backwards incompatible changes to the buildboxes and using version for that seems ok. Just need to make sure to update all pipelines to use this version after it's built.
There was a problem hiding this comment.
Following our Slack discussion, I changed the approach and installed the plugin a second time in a different location. The plugin lookup path is controlled through the HELM_PLUGINS variable. This variable will be set in:
build.assets/Makefile.cloudbuild/ci/unit-tests.yaml.github/workflows/unit-tests-helm.yaml
@jakule @r0mant could you both re-review the PR and confirm this approach is saner?
There was a problem hiding this comment.
Yeah, I think updating buildbox version here is not too bad. We gotta have some way to introduce backwards incompatible changes to the buildboxes and using version for that seems ok. Just need to make sure to update all pipelines to use this version after it's built.
hugoShaka
force-pushed
the
hugo/buildbox-12.1
branch
from
fa372c9 to
9281516
Compare
hugoShaka
changed the title
hugoShaka
changed the title
| rm -r helm-tarball*) | ||
|
|
||
| # TODO(hugoShaka): remove this backward compatible hack with teleportv13 buildbox | ||
| RUN helm plugin install https://github.com/quintush/helm-unittest && \ |
There was a problem hiding this comment.
Shouldn't we set HELM_PLUGINS for the helm plugin install command, otherwise it would install it in the same location as the old plugin below, no?
There was a problem hiding this comment.
I tried to do this, but helm did not respect the variable during installation 🙄 this is why the plugin is copied somewhere else, then deleted with helm plugin uninstall unittest. In the end we have 3 plugin installed:
- new version in /home/ci/.local/share/helm/plugins-new
- old version in /home/ci/.local/share/helm/plugins
- old version in /root/.local/share/helm/plugins-new
3 participants
This PR installs a second instance of the helm-unittest plugin in the teleport12 buildbox.
helm-unittestplugin is unmaintained and everyone forked. We are relying on an unmaintained fork with several broken features, including features I need to test the split auth/proxy chart.The plugin name is the same for the multiple forks but the signature and behaviour are radically different. Changing the plugin in the current
teleport12buildbox would cause all past jobs relying onteleport12to fail (it breaksmake testandmake -C build.assets test).A subsequent PR will come with the updated tests, makefile command, GHA and GCB pipeline to use the new plugin fork.