-
Notifications
You must be signed in to change notification settings - Fork 163
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
base: main
Are you sure you want to change the base?
Conversation
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 |
retest this please |
There was a problem hiding this 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).
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 |
retest this please |
Looks good to me. I'll do some testing later this week, and merge after I've confirmed it's working as expected. |
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:
From RHEL9:
|
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. |
Implementing cloud environment setup with azure.identy