chore: Remove storage and volume interface implementation

aayushrangwala commented 8 months ago

Changes Title (replace this with a logical title for your changes)

Remove unused and unsupported apis from equinix metal

Description

Remove implementation of storage APIs as they were removed from Packet years ago. there is no direct replacement. Equinix Metal offers Pure and NetApp storage devices today but there is not equivalent functionality as far as the Equinix Metal API is concerned with directly attaching storage to servers and equinix metal doesnot supports only 2 storage providers and

For more information on contributing, please see Contributing section of our documentation.

Status

Ready For Review

Checklist (tick everything that applies)

[x] Code linting (required, can be done after the PR checks)
[x] Documentation
[x] Tests
[x] ICLA (required for bigger changes)

displague commented 8 months ago

Reviewer note: The Elastic Block storage feature was removed from Equinix Metal in June 2021. https://web.archive.org/web/20210620005004/https://metal.equinix.com/developers/docs/storage/elastic-block-storage/#elastic-block-storage

displague commented 8 months ago

@Kami is this something that you can review and merge? There are three Equinix Metal PRs up. I have reviewed them on their Equinix Metal merits.

Kami commented 7 months ago

Thanks for the contribution.

Since this is a breaking change, can you please add an entry in upgrade notes file (docs/upgrade_notes.rst)? Thanks.

codecov-commenter commented 7 months ago

Codecov Report

Merging #1972 (86ff77e) into trunk (dfbb595) will increase coverage by 0.06%. Report is 10 commits behind head on trunk. The diff coverage is n/a.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## trunk #1972 +/- ## ========================================== + Coverage 83.20% 83.25% +0.06% ========================================== Files 353 353 Lines 81423 81252 -171 Branches 8591 8559 -32 ========================================== - Hits 67742 67645 -97 + Misses 10874 10820 -54 + Partials 2807 2787 -20 ``` | [Files](https://app.codecov.io/gh/apache/libcloud/pull/1972?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [libcloud/compute/drivers/equinixmetal.py](https://app.codecov.io/gh/apache/libcloud/pull/1972?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL2VxdWluaXhtZXRhbC5weQ==) | `70.54% <ø> (+6.18%)` | :arrow_up: | | [libcloud/test/compute/test\_equinixmetal.py](https://app.codecov.io/gh/apache/libcloud/pull/1972?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvdGVzdC9jb21wdXRlL3Rlc3RfZXF1aW5peG1ldGFsLnB5) | `92.89% <ø> (+2.76%)` | :arrow_up: |

Kami commented 7 months ago

@aayushrangwala Can you please sync up this branch with a latest trunk? I wanted to wrap it up myself locally, but I noticed more work is needed to make this change complete - it appears create_node() still calls create_volume() / attach_volume(), etc.

Please let me know when that has been addressed and when all checks are passing and I will have a look again.

On that note, it would also be good to update PR description with some context and why this change is needed (I know that context may be available in other PR, but we should also update this PR description).

Thanks.

aayushrangwala commented 6 months ago

@Kami can you please review again, Ive rebased and updated.

Kami commented 3 months ago

@antoinebourayne I had a look and it looks like this comment is still relevant - https://github.com/apache/libcloud/pull/1972#issuecomment-1837447866

I see create_node() method is still calling volume related methods which have been removed. create_node() should be updated to remove those calls and disk + disk_size argument should be removed as well (since it doesn't make sense / it's unused without corresponding volume management methods).

Kami commented 3 months ago

I made and pushed the following changes myself:

Remove disk and disk_size argument from create_node method - daddbae3fe699dcc708e2f7585a5d6865875ce7a
Add upgrade notes entry - 70a5f872c97d771dab2c5fe4e66a9b8a1d3aaf8a
Remove volumes from list_resources_async() - fc48cacde37b21498e4c7f1d597392df238c3c3f
Fix lint - 84b68be73045532110aa92eaa0b05873663ed994

With those changes, the code has now also been merged into trunk.

apache / libcloud