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

feat: [WD-17984] Add Pure Storage #1047

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Conversation

Kxiru
Copy link
Contributor

@Kxiru Kxiru commented Jan 8, 2025

Done

  • Added Pure Storage functionality

QA

  1. Run the LXD-UI:
  2. Perform the following QA steps:
    • Check out the branch and in the env.local file (One must be created if it does not already exist) add this line of code LXD_UI_BACKEND_IP=10.239.7.201 to have LXD point to the Pure-Storage-supported server. One must also have the Canonical vpn running to access this.
    • Please message @Kxiru for API Token and Gateway Credentials.
    • Navigate to Create a new storage pool and attempt to create a pool with Driver set to "Pure Storage" and "Verify Gateway" set to False (If set to True, this will fail).
    • Navigate to storage volumes and attempt to create a sotrage volume and assign it to the "Pure storage" pool just created.

Screenshots

image

@webteam-app
Copy link

@Kxiru Kxiru force-pushed the add-pure-storage branch 2 times, most recently from 61b34f6 to 9fcda0c Compare January 8, 2025 14:25
@Kxiru Kxiru marked this pull request as ready for review January 8, 2025 14:27
@Kxiru Kxiru force-pushed the add-pure-storage branch 3 times, most recently from 7d228b7 to 663e85f Compare January 8, 2025 14:40
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

Some small suggestions on the copy. Code seems all right.

src/pages/storage/forms/StoragePoolFormMain.tsx Outdated Show resolved Hide resolved
src/pages/storage/forms/StoragePoolFormMain.tsx Outdated Show resolved Hide resolved
src/pages/storage/forms/StoragePoolFormMain.tsx Outdated Show resolved Hide resolved
src/pages/storage/forms/StoragePoolFormMain.tsx Outdated Show resolved Hide resolved
src/pages/storage/forms/StoragePoolFormMain.tsx Outdated Show resolved Hide resolved
src/util/permissions.spec.ts Show resolved Hide resolved
src/types/config.d.ts Show resolved Hide resolved
@edlerd
Copy link
Collaborator

edlerd commented Jan 8, 2025

To resolve the test failures, please rebase on latest state of the main branch.

@Kxiru Kxiru force-pushed the add-pure-storage branch from 663e85f to 4f88663 Compare January 8, 2025 15:32
@Kxiru Kxiru requested a review from edlerd January 8, 2025 19:13
@Kxiru Kxiru force-pushed the add-pure-storage branch from 4f88663 to c62cd36 Compare January 9, 2025 16:51
@Kxiru Kxiru requested a review from edlerd January 9, 2025 16:51
edlerd
edlerd previously approved these changes Jan 9, 2025
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

Code LGTM, @mas-who I think you also have a token, can you please do QA?

@mas-who
Copy link
Collaborator

mas-who commented Jan 10, 2025

Some QA issues:

  1. After creating a pure storage pool, navigating to the config page for that pool, some of the config fields are not displaying existing values e.g. api token, api gateway etc
    Screenshot from 2025-01-10 18-09-46

  2. On the edit page for pure storage pool, updates made to api token, api gateway, verify gateway and mode fields does not persist

  3. After creating a pure storage volume, navigating to its config page for the first time, update a config without saving. The save button shows "Save 3 changes", but only one config has been altered. I checked the same for a zfs storage volume, the problem didn't occur, so this seems to be specific to pure storage volume updates.
    Screencast from 10-01-2025 18:25:13.webm

@Kxiru
Copy link
Contributor Author

Kxiru commented Jan 13, 2025

Some QA issues:

  1. After creating a pure storage pool, navigating to the config page for that pool, some of the config fields are not displaying existing values e.g. api token, api gateway etc
    ...

Thanks for this QA, Mason, that would have been a serious oversight. I think I have identified and addressed the problem now, and it actually related to all 3 of your concerns. Please verify from your end.

@mas-who
Copy link
Collaborator

mas-who commented Jan 14, 2025

Some QA issues:

  1. After creating a pure storage pool, navigating to the config page for that pool, some of the config fields are not displaying existing values e.g. api token, api gateway etc
    ...

Thanks for this QA, Mason, that would have been a serious oversight. I think I have identified and addressed the problem now, and it actually related to all 3 of your concerns. Please verify from your end.

Can confirm 1 and 2 are resolved now. It seems that 3 is still happening (see screenshot below), changing any volume config results in the save button to show "3" changes.
Screenshot from 2025-01-14 09-25-00

@Kxiru
Copy link
Contributor Author

Kxiru commented Jan 14, 2025

Some QA issues:

  1. After creating a pure storage pool, navigating to the config page for that pool, some of the config fields are not displaying existing values e.g. api token, api gateway etc
    ...

Thanks for this QA, Mason, that would have been a serious oversight. I think I have identified and addressed the problem now, and it actually related to all 3 of your concerns. Please verify from your end.

Can confirm 1 and 2 are resolved now. It seems that 3 is still happening (see screenshot below), changing any volume config results in the save button to show "3" changes. ...

Ah, apologies, I thought you had been seeing the error on the storage pool page, not the storage volume page. I'll push a fix for this shortly.

@mas-who
Copy link
Collaborator

mas-who commented Jan 17, 2025

LGTM 👍

@Kxiru Kxiru merged commit 3661fbe into canonical:main Jan 17, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants