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

fix: Azure BYOCNI workflow #2546

Merged
merged 1 commit into from
May 14, 2024
Merged

fix: Azure BYOCNI workflow #2546

merged 1 commit into from
May 14, 2024

Conversation

viktor-kurchenko
Copy link
Contributor

@viktor-kurchenko viktor-kurchenko commented May 13, 2024

This PR introduces a couple of fixes:

  1. Workaround for the Azure BYOCNI workflow: asynchronous Azure OIDC token fetch to avoid token assertion issues.
  2. Azure location changed to the eastus2 that used less intensive.

Workflow run example: https://github.com/cilium/cilium-cli/actions/runs/9063109497/job/24898372774?pr=2546

Workaround for: #2478

This PR introduces a couple of fixes:
1. Workaround for the Azure BYOCNI workflow: asynchronous Azure OIDC
   token fetch to avoid token assertion issues.
2. Azure location changed to the `eastus2` that used less intensive.

Signed-off-by: viktor-kurchenko <[email protected]>
@viktor-kurchenko viktor-kurchenko changed the title fix: Azure OIDC login fix: Azure BYOCNI workflow May 13, 2024
@viktor-kurchenko viktor-kurchenko marked this pull request as ready for review May 13, 2024 13:16
@viktor-kurchenko viktor-kurchenko requested review from a team as code owners May 13, 2024 13:16
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 14, 2024
@viktor-kurchenko
Copy link
Contributor Author

@michi-covalent I think you can mark the AKS (BYOCNI) as required after the merge.

@michi-covalent
Copy link
Contributor

AKS (BYOCNI) still failing in this pull request no? anyways i'm merging this 🚀

@michi-covalent michi-covalent merged commit 6d7ea2e into main May 14, 2024
12 of 13 checks passed
@michi-covalent michi-covalent deleted the pr/vk/azure/oidc/fix branch May 14, 2024 15:32
@viktor-kurchenko
Copy link
Contributor Author

AKS (BYOCNI) still failing in this pull request no? anyways i'm merging this 🚀

Due to the pull_request_target.

token=$(curl -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" "${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=api://AzureADTokenExchange" | jq .value -r)
az login --service-principal -u ${{ secrets.AZURE_PR_CLIENT_ID }} -t ${{ secrets.AZURE_PR_TENANT_ID }} --federated-token $token --output none
# Sleep for 4 minutes
sleep 240
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not 242?

Copy link
Member

@joestringer joestringer May 16, 2024

Choose a reason for hiding this comment

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

Can we achieve this with a periodic check for availability and a nice message when the failure is due to timeout?

EDIT: Ah never mind, I see that the overall step is doing the loop every 4 mins. I expected a much quicker loop.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants