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

workflows/externalworkload: Avoid using --config when unnecessary #24567

Merged
merged 2 commits into from Mar 28, 2023

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented Mar 25, 2023

First commit makes use of --helm-set instead of --config. Second commit uses --datapath-mode=tunnel to enable tunneling mode.

Passing test run: https://github.com/cilium/cilium/actions/runs/4520866855.

We should pass the config directly via --helm-set instead of overriding
the ConfigMap after it's installed.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno added the release-note/ci This PR makes changes to the CI. label Mar 25, 2023
The previous commit changed the installation command to use
'--helm-set=tunnel=vxlan' instead of '--config tunnel=vxlan'.

This however ends up breaking the installation because the Helm flag
conflict with the datapath mode detected by the CLI and the datapath
mode takes precedence. Since this is running in GKE, the CLI detects gke
as the datapath mode and tunneling mode is disabled.

It used to work because '--config tunnel=vxlan' would override the value
in the ConfigMap after the installation is done.

Instead of fighting against the CLI, let's just tell it to use tunneling
mode, with --datapath-mode=tunnel.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno force-pushed the pr/pchaigno/workflow-use-helm-set branch from 520b1d6 to b67cbd3 Compare March 25, 2023 19:37
@pchaigno pchaigno changed the title workflows/externalworkload: Use --helm-set workflows/externalworkload: Avoid using --config when unnecessary Mar 25, 2023
@pchaigno pchaigno force-pushed the pr/pchaigno/workflow-use-helm-set branch from b67cbd3 to 793ebd5 Compare March 25, 2023 20:30
@pchaigno pchaigno marked this pull request as ready for review March 25, 2023 20:32
@pchaigno pchaigno requested review from a team as code owners March 25, 2023 20:32
@pchaigno pchaigno requested review from aanm and tklauser March 25, 2023 20:32
@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 28, 2023
@julianwiedmann julianwiedmann merged commit 9be3b28 into master Mar 28, 2023
42 checks passed
@julianwiedmann julianwiedmann deleted the pr/pchaigno/workflow-use-helm-set branch March 28, 2023 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants