Skip to content

Conversation

jibi
Copy link
Member

@jibi jibi commented May 30, 2022

Passing ${{ github.workspace }}/pull-request to the init.sh script as
path of the PR checkout is incorrect, as inside the VM the repository
will be under a different path.

Instead of that, pass the correct absolute path for the PR checkout.

Moreover, checkout first the upstream repo (under cilium/cilium) and
only after that the PR one (under cilium/cilium/pull-request), as
otherwise the upstream checkout will nuke the directory with PR one.

This PR includes only the changes related to test/l4lb/test.sh, as the
workflow is being updated on the master PR #20007.

Fixes: #20004

@jibi jibi requested a review from a team as a code owner May 30, 2022 13:32
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.10 kind/backports This PR provides functionality previously merged into master. labels May 30, 2022
@jibi jibi requested a review from nbusseneau May 30, 2022 13:32
Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one nit.

@@ -4,7 +4,7 @@ set -eux

IMG_OWNER=${1:-cilium}
IMG_TAG=${2:-latest}
PULL_REQUEST_CHECKOUT=${3:-../..}
HELM_CHART_DIR=${3:-/vagrant/pull-request/install/kubernetes/cilium}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backward compatibility:

Suggested change
HELM_CHART_DIR=${3:-/vagrant/pull-request/install/kubernetes/cilium}
HELM_CHART_DIR=${3:-/vagrant/install/kubernetes/cilium}

Passing ${{ github.workspace }}/pull-request to the init.sh script as
path of the PR checkout is incorrect, as inside the VM the repository
will be under a different path.

Instead of that, pass the correct absolute path for the PR checkout.

Moreover, checkout first the upstream repo (under cilium/cilium) and
only after that the PR one (under cilium/cilium/pull-request), as
otherwise the upstream checkout will nuke the directory with PR one.

This PR includes only the changes related to test/l4lb/test.sh, as the
workflow is being updated on the master PR #20007.

Fixes: #20004
Signed-off-by: Gilberto Bertin <jibi@cilium.io>
@jibi jibi force-pushed the pr/jibi/110-l4lb-helm-fix branch from 518cb74 to 8a0fe99 Compare May 30, 2022 14:23
@jibi jibi added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 30, 2022
@jibi jibi merged commit e577e4e into v1.10 May 30, 2022
@jibi jibi deleted the pr/jibi/110-l4lb-helm-fix branch May 30, 2022 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants