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: provide UI backward compatibility for older lxd versions [WD-8671] #647

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

mas-who
Copy link
Collaborator

@mas-who mas-who commented Feb 14, 2024

Done

  • Hide iso import functionality for storage volumes if the lxd server does not support the feature. This includes creating an instance based on custom ISO or attaching custom ISO to a running instance.
  • Return empty server configs for lxd 5.0 instead of calling the /1.0/metadata/configuration endpoint
  • Return empty doc string for lxd 5.0 instead of calling the /documentation api endpoint
  • Remove features.networks.zones project config for lxd 5.0
  • Remove features.storage.buckets project config for lxd 5.0
  • Hide warnings page for lxd 5.0

QA

  1. Run the LXD-UI:
    • On the demo server via the link posted by @webteam-app below. This is only available for PRs created by collaborators of the repo. Ask @lorumic or @edlerd for access.
    • With a local copy of this branch, run as described here.
  2. Perform the following QA steps:
    • Downgrade lxd to 5.0/stable first
    • make sure you can't create an instance using a custom ISO, attach a custom ISO to a running instance via the Instance Console and the Custom ISOs tab is not visible on the storage page.
    • make sure the server settings page does not error out even it's mostly empty and that there is a notification asking users to upgrade to lxd 5.19 or later.
    • check the network tab in the browser inspect tool that on any configuration page, no network requests are made to the /documentation/objects.inv.txt endpoint
    • make sure that you can create project successfully, then try edit a project and can save it successfully. On the project details page, main configuration section, you should not see the check boxes for Network zones and storage bucket features.
    • make sure the warnings page is hidden from the side navigation

@webteam-app
Copy link

Demo starting at https://lxd-ui-647.demos.haus

@mas-who
Copy link
Collaborator Author

mas-who commented Feb 14, 2024

Things to think about

  1. I wonder if there is a better and more generic approach to manage feature support? Going to think on this one for a bit.
  2. Maybe we should start checking from lxd 4.0, then get a full list of features that doesn't work and that should inform which api extension to check for feature support. This is probably a good approach to establish a baseline assuming api extensions only have additions with lxd version increase from the perspective of the UI.

src/context/useSettings.tsx Outdated Show resolved Hide resolved
@edlerd
Copy link
Collaborator

edlerd commented Feb 14, 2024

2. Maybe we should start checking from lxd 4.0, then get a full list of features that doesn't work and that should inform which api extension to check for feature support. This is probably a good approach to establish a baseline assuming api extensions only have additions with lxd version increase from the perspective of the UI.

I am not sure how much 4.0 is still in use. I took this on our sync agenda for tomorrow, we can get more input from the core team on which lowest version to support. Likewise, I was previously under the impression 5.0 is fine.

@mas-who
Copy link
Collaborator Author

mas-who commented Feb 14, 2024

  1. Maybe we should start checking from lxd 4.0, then get a full list of features that doesn't work and that should inform which api extension to check for feature support. This is probably a good approach to establish a baseline assuming api extensions only have additions with lxd version increase from the perspective of the UI.

I am not sure how much 4.0 is still in use. I took this on our sync agenda for tomorrow, we can get more input from the core team on which lowest version to support. Likewise, I was previously under the impression 5.0 is fine.

Aah great, thanks I was thinking about asking that tomorrow as well! I think if I know what the baseline lxd version is, then I can streamline the process to make this happen

@mas-who mas-who force-pushed the backward-lxd-compatibility branch 3 times, most recently from e4144a7 to 73da0aa Compare February 15, 2024 17:13
src/api/server.tsx Outdated Show resolved Hide resolved
@mas-who mas-who marked this pull request as ready for review February 15, 2024 17:32
@mas-who
Copy link
Collaborator Author

mas-who commented Feb 15, 2024

Will update PR notes in the morning.

src/pages/settings/Settings.tsx Outdated Show resolved Hide resolved
src/context/useSupportedFeatures.tsx Outdated Show resolved Hide resolved
@mas-who mas-who force-pushed the backward-lxd-compatibility branch 2 times, most recently from 198cc5c to e46c319 Compare February 16, 2024 06:37
@mas-who mas-who changed the title feat: provide UI backward compatibility for older lxd versions feat: provide UI backward compatibility for older lxd versions [WD-8671] Feb 16, 2024
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.

QA looks good, tested with 5.0 and a recent backend.

I found one issue and some small optional comments on code simplifications below.

src/api/server.tsx Outdated Show resolved Hide resolved
src/context/useSupportedFeatures.tsx Outdated Show resolved Hide resolved
src/pages/projects/CreateProject.tsx Outdated Show resolved Hide resolved
src/api/server.tsx Outdated Show resolved Hide resolved
src/pages/projects/EditProject.tsx Outdated Show resolved Hide resolved
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.

LGTM, thanks for applying the fixes :)

@mas-who mas-who merged commit 8c966b7 into canonical:main Feb 16, 2024
8 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 16, 2024
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.

3 participants