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

azure-fence.py: use azure-identity instead of msrestazure as it has been deprecated #602

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

varkoly
Copy link

@varkoly varkoly commented Dec 18, 2024

Implementing cloud environment setup with azure.identy

Copy link

knet-jenkins bot commented Dec 18, 2024

Can one of the admins check and authorise this run please: https://ci.kronosnet.org/job/fence-agents/job/fence-agents-pipeline/job/PR-602/1/input

@oalbrigt oalbrigt changed the title Fix issue#599 lib/azure-fence.py.py: python-msrestazure deprecated azure-fence.py: use azure-identity instead of msrestazure as it has been deprecated Jan 2, 2025
@oalbrigt
Copy link
Collaborator

oalbrigt commented Jan 2, 2025

retest this please

Copy link
Collaborator

@oalbrigt oalbrigt left a comment

Choose a reason for hiding this comment

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

Looks good to me, but there's an indentation mismatch on one or more lines making CI fail (TabError: inconsistent use of tabs and spaces in indentation).

Copy link

knet-jenkins bot commented Jan 7, 2025

Can one of the admins check and authorise this run please: https://ci.kronosnet.org/job/fence-agents/job/fence-agents-pipeline/job/PR-602/3/input

@oalbrigt
Copy link
Collaborator

oalbrigt commented Jan 7, 2025

retest this please

@oalbrigt
Copy link
Collaborator

oalbrigt commented Jan 7, 2025

Looks good to me.

I'll do some testing later this week, and merge after I've confirmed it's working as expected.

@oalbrigt
Copy link
Collaborator

The azure-identity part doesnt work with the other libraries it seems like, and also azure-common still requires msrestazure, so we still need it even if we dont use it in our code.

NOTE: It seems like the RHEL8 code needs to be updated based on the RHEL9 azure_fence.py and possibly fence_azure_arm, so that output is just in case it might give some additional pointers on top of the RHEL9 output.

Output from RHEL8 fence_azure_arm -o list:

Traceback (most recent call last):
  File "./fence_azure_arm", line 260, in <module>
    main()
  File "./fence_azure_arm", line 238, in main
    compute_client = azure_fence.get_azure_compute_client(config)
  File "/usr/share/fence/azure_fence.py", line 333, in get_azure_compute_client
    credentials = get_azure_credentials(config)
  File "/usr/share/fence/azure_fence.py", line 315, in get_azure_credentials
    cloud_environment=cloud_environment
  File "/usr/lib/fence-agents/bundled/azure/msrestazure/azure_active_directory.py", line 352, in __init__
    self._configure(**kwargs)
  File "/usr/lib/fence-agents/bundled/azure/msrestazure/azure_active_directory.py", line 89, in _configure
    auth_endpoint = self._cloud_environment.endpoints.active_directory
AttributeError: 'str' object has no attribute 'endpoints' 

From RHEL9:

Traceback (most recent call last):
  File "/root/./fence_azure_arm", line 270, in <module>
    main()
  File "/root/./fence_azure_arm", line 252, in main
    compute_client = azure_fence.get_azure_compute_client(config)
  File "/usr/share/fence/azure_fence.py", line 374, in get_azure_compute_client
    credential_scope = cloud_environment.endpoints.resource_manager + "/.default"
AttributeError: 'str' object has no attribute 'endpoints'

@oalbrigt
Copy link
Collaborator

After updating the agent and library based on upstream and this patch I get the same output as I got in the RHEL9 test above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants