Upgrade to new azure-go-sdk and refactor old go-autorest references #2368

mabhi · 2023-09-30T14:03:55Z

Change Overview

As per Azure guidelines, few modules from azure library are retiring support from 31 March 2023 . Due to which there is need to replace such modules with azure recommended azure-go-sdk modules.

go-autorest being used in the blockstorage package will be out of support from March 2023. Follow the documentation below to replace it with a new supported package.
https://azure.microsoft.com/en-us/updates/support-for-azure-sdk-libraries-that-do-not-conform-to-our-current-azure-sdk-guidelines-will-be-retired-as-of-31-march-2023/

Pull request type

Please check the type of change your PR introduces:

Issues

fixes #issue-number

Test Plan

💪 Manual
⚡ Unit test
💚 E2E

pkg/blockstorage/azure/client.go

viveksinghggits · 2023-10-05T07:38:46Z

Hi @leuyentran ,
Can you please have a look into this PR?

leuyentran

Pre-lim review on my end. Thank you for working on this integration 🙏 The new package is so much nicer to use 😄 I will come back and finish off the reviews soon

pkg/blockstorage/azure/auth.go

pkg/blockstorage/azure/client.go

leuyentran

Part 2 of my pre-lim. azuredisk.go is a beast so I'd like to do another pass later. Awesome job with the integration in that file 🙏

I'm a little nervous on the extent of changes. This is not at all about the quality of PR, but more on the scope of things that need to change that might cause us to miss a thing a two even w PR reviews. What are our testing plans?

pkg/blockstorage/azure/client.go

pkg/blockstorage/azure/client_test.go

pkg/blockstorage/azure/environments.go

pkg/blockstorage/helpers.go

pkg/blockstorage/azure/azuredisk.go

leuyentran

Part 2 of my pre-lim. azuredisk.go is a beast so I'd like to do another pass later. Awesome job with the integration in that file 🙏

mabhi

Addressed review comments

pkg/blockstorage/azure/auth.go

pkg/blockstorage/azure/environments.go

pkg/blockstorage/azure/auth.go

pkg/blockstorage/azure/client.go

pkg/blockstorage/helpers.go

leuyentran

Partial re-review. Need to take a closer look to azuredisk.go

pkg/blockstorage/azure/auth.go

pkg/blockstorage/helpers.go

leuyentran

I have reviewed azuredisk.go as well as the changes made since the last partial reviews. Thank you for the quick around 🙏 . Overall the changes look good to me besides some concerns for nil pointer deref. Please also check Vivek's comment on import ordering 😄

pkg/blockstorage/azure/azuredisk.go

leuyentran

LGTM. Thank you for breaking down the review changes by commits. it really helped w reviewing.

I have a follow up question on which data is considered critical and should results in errors in case of nil pointer. Adn on that topic, I'd like to follow up on whether disSize should be considered a critical data. However, I think these can be follow uped after this PR

pkg/blockstorage/azure/azuredisk.go

pkg/blockstorage/azure/auth.go

pkg/blockstorage/helper_test.go

pkg/blockstorage/azure/azuredisk.go

pkg/blockstorage/helper_test.go

pkg/blockstorage/azure/environments.go

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Signed-off-by: Abhijit Mukherjee <abhijit.mukherjee@infracloud.io>

…rences (#2368)" This reverts commit 94ac01a.

…rences (#2368)" (#2490) This reverts commit 94ac01a.

infraq added this to In Progress in Kanister Sep 30, 2023

mabhi force-pushed the update-azure-sdk-lib branch from 5cffae4 to e991881 Compare October 2, 2023 06:01

mabhi marked this pull request as ready for review October 3, 2023 07:11

mabhi mentioned this pull request Oct 3, 2023

Update Azure SDK Libraries in Kanister #2326

Closed

10 tasks

mabhi requested review from pavannd1 and PrasadG193 October 3, 2023 07:13

akankshakumari393 reviewed Oct 3, 2023

View reviewed changes

pkg/blockstorage/azure/client.go Outdated Show resolved Hide resolved

mabhi force-pushed the update-azure-sdk-lib branch from 62b7902 to f6223fc Compare October 5, 2023 06:42

viveksinghggits requested a review from leuyentran October 5, 2023 07:38

mabhi force-pushed the update-azure-sdk-lib branch 2 times, most recently from 95eabcc to 8a3e451 Compare October 6, 2023 04:50

leuyentran requested changes Oct 10, 2023

View reviewed changes

Kanister automation moved this from In Progress to Review Required Oct 10, 2023

leuyentran self-requested a review October 12, 2023 20:48

leuyentran reviewed Oct 12, 2023

View reviewed changes

mabhi force-pushed the update-azure-sdk-lib branch from 8a3e451 to 72c7a41 Compare October 13, 2023 17:57

mabhi commented Oct 16, 2023

View reviewed changes

leuyentran reviewed Oct 16, 2023

View reviewed changes

mabhi force-pushed the update-azure-sdk-lib branch from f5371f9 to 069c08f Compare October 17, 2023 14:09

viveksinghggits reviewed Oct 17, 2023

View reviewed changes

pkg/blockstorage/azure/client.go Show resolved Hide resolved

mabhi force-pushed the update-azure-sdk-lib branch from ba7e030 to e5978a4 Compare October 25, 2023 08:01

mabhi force-pushed the update-azure-sdk-lib branch from e5978a4 to 4fe7b46 Compare November 1, 2023 14:56

julio-lopez reviewed Nov 3, 2023

View reviewed changes

pkg/blockstorage/helpers.go Outdated Show resolved Hide resolved

pkg/blockstorage/helpers.go Outdated Show resolved Hide resolved

leuyentran reviewed Nov 7, 2023

View reviewed changes

mabhi force-pushed the update-azure-sdk-lib branch from 4fe7b46 to d6ae3ba Compare November 8, 2023 14:18

leuyentran reviewed Nov 8, 2023

View reviewed changes

mabhi force-pushed the update-azure-sdk-lib branch from d6ae3ba to 78aa9e3 Compare November 14, 2023 06:52

leuyentran approved these changes Nov 20, 2023

View reviewed changes

pavannd1 reviewed Nov 22, 2023

View reviewed changes

pkg/blockstorage/azure/azuredisk.go Show resolved Hide resolved

pkg/blockstorage/helper_test.go Show resolved Hide resolved

pkg/blockstorage/azure/environments.go Show resolved Hide resolved

pavannd1 mentioned this pull request Nov 22, 2023

Kanister docs update to version 0.100.0 #2488

Merged

mabhi added 18 commits November 22, 2023 16:28

Upgrade to new azure-go-sdk and refactor old go-autorest references

8c96516

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Updated test cases

eaeb587

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Added environment struct for azure soverign cloud

ebc0818

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Removed unused code comments

cedfb07

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Lint error fix

a7bbd43

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Refactor the azure credential, make it explicit

15cc66d

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Addressed review changes

144666b

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Removed explicit token fetch

5a1585c

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Added test cases for helper methods

be7d906

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Use deferenced disk pointer during volume list

b09e17c

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Update snapshot deference during listing

8e6af5f

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Fix nil pointer exception during snapshot-id fetch

4f1fa7c

Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>

Reverted azure-default-credential handling

67e1035

Signed-off-by: Abhijit Mukherjee <abhijit.mukherjee@infracloud.io>

Updated changes from master

91ee666

Signed-off-by: Abhijit Mukherjee <abhijit.mukherjee@infracloud.io>

Addressing review comments

c4323c5

Signed-off-by: Abhijit Mukherjee <abhijit.mukherjee@infracloud.io>

Addressed review comments on nil pointers

6b2bdc6

Signed-off-by: Abhijit Mukherjee <abhijit.mukherjee@infracloud.io>

Addressed import grouping

45c9249

Signed-off-by: Abhijit Mukherjee <abhijit.mukherjee@infracloud.io>

Added license information

0ad1e5f

Signed-off-by: Abhijit Mukherjee <abhijit.mukherjee@infracloud.io>

mabhi force-pushed the update-azure-sdk-lib branch from cb5e7ba to 0ad1e5f Compare November 22, 2023 11:04

mabhi added the kueue label Nov 22, 2023

mergify bot merged commit 94ac01a into master Nov 22, 2023
14 checks passed

Kanister automation moved this from Reviewer approved to Done Nov 22, 2023

mergify bot deleted the update-azure-sdk-lib branch November 22, 2023 11:52

mabhi restored the update-azure-sdk-lib branch November 22, 2023 13:17

PrasadG193 added a commit that referenced this pull request Nov 22, 2023

Revert "Upgrade to new azure-go-sdk and refactor old go-autorest refe…

4f09514

…rences (#2368)" This reverts commit 94ac01a.

PrasadG193 mentioned this pull request Nov 22, 2023

Revert "Upgrade to new azure-go-sdk and refactor old go-autorest references" #2490

Merged

mergify bot pushed a commit that referenced this pull request Nov 22, 2023

Revert "Upgrade to new azure-go-sdk and refactor old go-autorest refe…

2931c5b

…rences (#2368)" (#2490) This reverts commit 94ac01a.

leuyentran mentioned this pull request Dec 6, 2023

Update azure sdk lib #2507

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to new azure-go-sdk and refactor old go-autorest references #2368

Upgrade to new azure-go-sdk and refactor old go-autorest references #2368

mabhi commented Sep 30, 2023

viveksinghggits commented Oct 5, 2023

leuyentran left a comment •

edited

Loading

leuyentran left a comment •

edited

Loading

leuyentran left a comment

mabhi left a comment

leuyentran left a comment

leuyentran left a comment

leuyentran left a comment

Upgrade to new azure-go-sdk and refactor old go-autorest references #2368

Upgrade to new azure-go-sdk and refactor old go-autorest references #2368

Conversation

mabhi commented Sep 30, 2023

Change Overview

Pull request type

Issues

Test Plan

viveksinghggits commented Oct 5, 2023

leuyentran left a comment • edited Loading

Choose a reason for hiding this comment

leuyentran left a comment • edited Loading

Choose a reason for hiding this comment

leuyentran left a comment

Choose a reason for hiding this comment

mabhi left a comment

Choose a reason for hiding this comment

leuyentran left a comment

Choose a reason for hiding this comment

leuyentran left a comment

Choose a reason for hiding this comment

leuyentran left a comment

Choose a reason for hiding this comment

leuyentran left a comment •

edited

Loading

leuyentran left a comment •

edited

Loading