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

Add longhorn support message in VM clone dialog #1223

Merged
merged 5 commits into from
Nov 15, 2024

Conversation

torchiaf
Copy link
Collaborator

@torchiaf torchiaf commented Nov 5, 2024

Summary

  • Re-enabling Longhorn V2 unsupported dialogs and Add a warning message in the dialog layouts; the actions continue to be disabled:
    • VM -> Clone, Snapshot (todo)
    • Volume -> Clone, Export, Snapshot

PR Checklist

  • Is this a multi-tenancy feature/bug?
    • Yes, the relevant RBAC changes are at:
  • Do we need to backport changes to the old Rancher UI, such as RKE1?
    • Yes, the relevant PR is at:
  • Are backend engineers aware of UI changes?
    • Yes, the backend owner is:

Related Issue harvester/harvester#6989

Occurred changes and/or fixed issues

Technical notes summary

Areas or cases that should be tested

Areas which could experience regressions

Screenshot/Video

image (4)

@torchiaf torchiaf requested a review from a110605 November 5, 2024 11:30
@a110605 a110605 added the Backport to v1.4 Backport PR target v1.4 label Nov 12, 2024
@torchiaf torchiaf force-pushed the fix/longhorn-support branch from 959baf7 to d6d1bcb Compare November 14, 2024 15:13
@torchiaf
Copy link
Collaborator Author

@tserong @Vicente-Cheng I also disabled the VM -> Migrate action, for those Vms that contain Longhorn v2 volumes. Is it correct ?

@torchiaf torchiaf marked this pull request as ready for review November 14, 2024 15:41
</template>
<template v-else>
<Banner color="warning">
<t k="harvester.modal.cloneVM.message.support.longhorn" :raw="true" />
Copy link
Collaborator

@a110605 a110605 Nov 15, 2024

Choose a reason for hiding this comment

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

nit: it's fine to hard code the doc url wthin en-us.yaml in harvester/dashboard as we will only maintain v1.4.x release in this repo.

But when we port the same change to harvester/harvester-ui-extension. The doc link should be varaible. Like

<t k="harvester.setting.storageNetwork.tip" :raw="true" :url="storageNetworkExampleLink" />

@tserong
Copy link

tserong commented Nov 15, 2024

@tserong @Vicente-Cheng I also disabled the VM -> Migrate action, for those Vms that contain Longhorn v2 volumes. Is it correct ?

I'm not sure about this, for two reasons:

  1. I don't think it's strictly necessary to disable in the GUI, as the admission webhook will notice when a VM can't be migrated and will prevent the migration anyway, e.g:
    migration-admission-webhook
  2. Right now, LHv2 volumes can't be migrated, but in future, it will be possible to migrate them. So it's not really about the fact that an LHv2 volume is attached, it's actually whether any attached volume has accessmode RWO or RWX. If it's RWO, it can't be migrated, and if it's RWX it probably can be.

Copy link
Collaborator

@a110605 a110605 left a comment

Choose a reason for hiding this comment

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

Test good to me.

Show the unsupported message to below actions

  • Longhorn v2 volume: clone, export image, take snapshot actions.
  • VM with longhorn v2 volume : clone, take VM snapshot, migrate actions
Screenshot 2024-11-15 at 3 11 31 PM Screenshot 2024-11-15 at 3 38 10 PM

@torchiaf
Copy link
Collaborator Author

@tserong @a110605 since we have the webhook validation for the Migrate action, I'd remove the limitation from the UI.

@torchiaf torchiaf force-pushed the fix/longhorn-support branch from 3a40a8e to 1f1d421 Compare November 15, 2024 08:56
@torchiaf torchiaf merged commit a8ed128 into harvester:master Nov 15, 2024
7 checks passed
@torchiaf
Copy link
Collaborator Author

@mergify backport release-harvester-v1.4

Copy link

mergify bot commented Nov 15, 2024

backport release-harvester-v1.4

✅ Backports have been created

torchiaf added a commit that referenced this pull request Nov 15, 2024
…v1.4/pr-1223

Add longhorn support message in VM clone dialog (backport #1223)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport to v1.4 Backport PR target v1.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants