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.

Sign up for GitHub

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

Jump to bottom

Implement az-cli warmup on ubuntu #11173

Merged
merged 1 commit into from
Jan 6, 2025
Merged

Implement az-cli warmup on ubuntu #11173

merged 1 commit into from
Jan 6, 2025
+37 −10

Conversation

lanni-energinet
Copy link
Contributor

@lanni-energinet lanni-energinet commented Dec 12, 2024 •
edited
Loading

Description

This PR fixes #10110.

  • Config + cli-extensions directory is placed in /opt but not in installation dir at /opt/az/ as this is bad practice.
  • Removed cli-extensions directory setup from devops extension as this setting belongs in az-cli setup
  • Added cleanup for telemetry / logs in config directory

Related issue:

#10110

Check list

  • Related issue / work item is attached
  • Tests are written (if applicable)
  • Documentation is updated (if applicable)
  • Changes are tested and related VM images are successfully generated

@lanni-energinet lanni-energinet mentioned this pull request Dec 12, 2024
Closed
14 tasks
subir0071
subir0071 approved these changes Jan 6, 2025
View reviewed changes
Copy link
Contributor

@subir0071 subir0071 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution @lanni-energinet .
We are merging this PR based on our internal test results.

@subir0071 subir0071 merged commit ffe7e6a into actions:main Jan 6, 2025
3 checks passed
@Tolsto
Copy link

Tolsto commented Jan 14, 2025

This change breaks azure/cli with azure/login for me. See Azure/cli#175

jiasli
jiasli reviewed Jan 15, 2025
View reviewed changes
images/ubuntu/scripts/build/install-azure-cli.sh

# AZURE_CONFIG_DIR shell variable defines where the CLI configuration file for managing behavior are stored
# https://learn.microsoft.com/en-us/cli/azure/azure-cli-configuration#cli-configuration-file
# This path SHOULD be different from the installation directory /opt/az/
Copy link

Choose a reason for hiding this comment

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

The configuration directory of Azure CLI on an agent is /home/runner/.azure.

You may see this using this action:

name: Run Azure Login

on:
  workflow_dispatch:

jobs:
  test:
    runs-on: ubuntu-24.04
    steps:   
    - uses: azure/login@v2
      with:
         creds: ${{ secrets.AZURE_CREDENTIALS }}

    - run: az account get-access-token --debug

The debug log shows:

DEBUG: cli.azure.cli.core.auth.persistence: build_persistence: location='/home/runner/.azure/service_principal_entries.json', encrypt=False

@jiasli
Copy link

jiasli commented Jan 15, 2025

⚠️ The Azure CLI product team demands an immediate revert of this PR!

This PR was merged without fully testing and communication with Azure CLI product team. It breaks azure/cli action (Azure/cli#175) because:

  • azure/cli action assumes AZURE_CONFIG_DIR points to $HOME/.azure
  • azure/cli action directly mounts $HOME/.azure into the docker container, instead of via AZURE_CONFIG_DIR

According to my experience, azure/cli action is not the only user of $HOME/.azure. Moving AZURE_CONFIG_DIR away from $HOME/.azure to another location may unexpectedly break many other users who rely on $HOME/.azure.

Therefore, please immediately revert this PR before this change is rolled out to more users and causes more damage.

@MoChilia MoChilia mentioned this pull request Jan 15, 2025
Closed
@jiasli jiasli mentioned this pull request Jan 15, 2025
Merged
@DergachevE DergachevE mentioned this pull request Jan 15, 2025
Closed
7 tasks
subir0071 added a commit that referenced this pull request Jan 15, 2025
@subir0071
Revert "Implemented az-cli warmup (#11173)"
73811db 
This reverts commit ffe7e6a.
@subir0071 subir0071 mentioned this pull request Jan 15, 2025
Merged
subir0071 added a commit that referenced this pull request Jan 15, 2025
@subir0071
Revert "Implemented az-cli warmup (#11173)" (#11400)
8b7d5f6 
This reverts commit ffe7e6a.
@lanni-energinet
Copy link
Contributor Author

Thanks for quickly reverting the PR - the goal was to make the ship sail faster, not sink it.

As @jiasli points out there's probably even more actions that assumes AZURE_CONFIG_DIR=$Home/.azure or even AZURE_CONFIG_DIR=/runner/.azure thus the paths need to remain as-is. I'm puzzled by the lack of impact on the windows runners given the similarity of how the windows warmup is implemented - But I'm guessing that none of the affected downstream actions support windows runners.

@subir0071 @kishorekumar-anchala
Can either of you reopen #10110

@jiasli If you could lend you knowledge in any way to fix the original issue it would be much appreciated

@lanni-energinet lanni-energinet deleted the ubuntu_azcli_warmup branch January 16, 2025 09:52
@jiasli jiasli mentioned this pull request Jan 20, 2025
Closed
3 tasks
@jiasli
Copy link

jiasli commented Jan 20, 2025

Config + cli-extensions directory is placed in /opt but not in installation dir at /opt/az/ as this is bad practice.

This statement is not 100% true. As I pointed out in #11173 (comment), the configuration directory of Azure CLI on an agent is /home/runner/.azure, not /opt/az/.

As for why AZURE_EXTENSION_DIR is modified to /opt/az/azcliextensions, I have no context. By default, it should be ~/.azure/cliextensions: https://github.com/Azure/azure-cli/blob/d1a5844544ba30da428505867ad2f1ea39f2b950/src/azure-cli-core/azure/cli/core/extension/__init__.py#L22-L23

@lanni-energinet
Copy link
Contributor Author

lanni-energinet commented Jan 20, 2025 •
edited
Loading

@jiasli
The reason for moving the config directory (as it has been done on windows), is because of the poor performance of azure-cli and github runners

Calling az version can sometimes take as long as 2m40s.

Here is the context
#8410
#8441
#10110

After investigating and looking through https://github.com/Azure/azure-cli and https://github.com/microsoft/knack it looks like the issue is related to the generation of commandIndex.json or close by. As I've only been able to reproduce this on the first command invocation. But only on github runners, never my local machine

Edit: attached log example 42secs.log

Try running this workflow - it will show how inconsistent it is

name: 'az version is slow on first run'

on: [push]

jobs:
  test:
    name: test
    runs-on: ubuntu-latest   
    strategy:
      matrix:
        run: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]
        
    steps:
    - run: echo ${{ matrix.run }}
    - run: az version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiasli jiasli jiasli left review comments

@subir0071 subir0071 subir0071 approved these changes

@shamil-mubarakshin shamil-mubarakshin Awaiting requested review from shamil-mubarakshin

Assignees
No one assigned
Labels
CI ubuntu-all
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

First run of az-cli is slow
5 participants
@lanni-energinet @Tolsto @jiasli @subir0071 @kishorekumar-anchala