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: e2e: bump Cilium CLI to v0.14.2 #25194

Merged
merged 1 commit into from May 9, 2023
Merged

Conversation

jibi
Copy link
Member

@jibi jibi commented Apr 28, 2023

which includes a new basic egress gateway test.

Bumping required also some extra logic to install static routes for
external CIDRs and figure out the correct external node IPs value to
pass to the --external-from-cidrs connectivity test parameters

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. labels Apr 28, 2023
@cilium cilium deleted a comment from maintainer-s-little-helper bot Apr 28, 2023
@cilium cilium deleted a comment from maintainer-s-little-helper bot Apr 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. and removed dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. labels Apr 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 1, 2023
@cilium cilium deleted a comment from maintainer-s-little-helper bot May 1, 2023
@jibi jibi force-pushed the pr/jibi/egressgw-tests branch 4 times, most recently from be11d07 to 2be65bb Compare May 1, 2023 09:16
@jibi jibi added area/CI Continuous Integration testing issue or flake sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/ci This PR makes changes to the CI. feature/egress-gateway Impacts the egress IP gateway feature. labels May 1, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels May 1, 2023
@jibi jibi force-pushed the pr/jibi/egressgw-tests branch 4 times, most recently from f73594c to 31afa89 Compare May 2, 2023 09:38
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 2, 2023
@cilium cilium deleted a comment from maintainer-s-little-helper bot May 2, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 2, 2023
@jibi
Copy link
Member Author

jibi commented May 8, 2023

@jibi jibi marked this pull request as ready for review May 8, 2023 15:16
@jibi jibi requested review from a team as code owners May 8, 2023 15:16
@jibi jibi requested a review from brlbil May 8, 2023 15:16
@pchaigno
Copy link
Member

pchaigno commented May 8, 2023

Is it expected that we are now updating the CLI in workflows piecemeal?

@jibi
Copy link
Member Author

jibi commented May 8, 2023

Is it expected that we are now updating the CLI in workflows piecemeal?

not sure what you mean here

@brlbil
Copy link
Contributor

brlbil commented May 8, 2023

cilium-cli is configured in renovate https://github.com/cilium/cilium/blob/main/.github/renovate.json5#L247
but I am not sure why it is not updated by it.

@maintainer-s-little-helper

This comment was marked as resolved.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 8, 2023
@pchaigno
Copy link
Member

pchaigno commented May 8, 2023

Is it expected that we are now updating the CLI in workflows piecemeal?

not sure what you mean here

Why aren't we updating the CLI in all workflows?

@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 8, 2023
@jibi
Copy link
Member Author

jibi commented May 8, 2023

cilium-cli is configured in renovate https://github.com/cilium/cilium/blob/main/.github/renovate.json5#L247 but I am not sure why it is not updated by it.

probably because we just released it a hour ago, I can close this PR and wait for the renovate one (but that will need some manual adjustments anyway)

@jibi
Copy link
Member Author

jibi commented May 8, 2023

Is it expected that we are now updating the CLI in workflows piecemeal?

not sure what you mean here

Why aren't we updating the CLI in all workflows?

ah, right, because this PR started with just the e2e suite, but I can surely update all the workflows

@brb
Copy link
Member

brb commented May 8, 2023

This PR is adding an additional coverage to the e2e tests (fromCIDR, EGW tests). To avoid slipping unexpected blockers again, I think it's fine to let the renovate to eventually pick other workflows in separate PR, and meanwhile merge this PR.

@pchaigno
Copy link
Member

pchaigno commented May 8, 2023

This PR is adding an additional coverage to the e2e tests (fromCIDR, EGW tests). To avoid slipping unexpected blockers again, I think it's fine to let the renovate to eventually pick other workflows in separate PR, and meanwhile merge this PR.

I'm not against merging that without waiting, but let's note that Renovate is unable to test the workflow changes. So we can't rely on Renovate to update the CLI in workflows (a change that clearly needs to be tested).

Copy link
Contributor

@brlbil brlbil left a comment

Choose a reason for hiding this comment

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

The consensus seems to be it is OK to merge only this. LGTM.

@jibi
Copy link
Member Author

jibi commented May 9, 2023

I'll fix the conflict and update the v1.13 workflow and then we can merge this 👍

After that (in a separate PR to avoid additional blockers) I'll update all the remaining workflows since I'm already on it, as they'll also need the additional bash logic to set EXTERNAL_NODE_IPS_PARAM and the person approving the renovate PR will probably not have context on that

which includes a new basic egress gateway test.

Bumping required also some extra logic to install static routes for
external CIDRs and figure out the correct external node IPs value to
pass to the --external-from-cidrs connectivity test parameters

Signed-off-by: Gilberto Bertin <jibi@cilium.io>
@jibi jibi force-pushed the pr/jibi/egressgw-tests branch 2 times, most recently from b55e99e to c1a4f69 Compare May 9, 2023 07:41
@jibi
Copy link
Member Author

jibi commented May 9, 2023

@jibi jibi added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 9, 2023
@youngnick youngnick merged commit 772398d into main May 9, 2023
44 checks passed
@youngnick youngnick deleted the pr/jibi/egressgw-tests branch May 9, 2023 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake feature/egress-gateway Impacts the egress IP gateway feature. 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. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants