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

VM Maintenance Strategy #1060

Merged
merged 8 commits into from
Jul 9, 2024
Merged

Conversation

torchiaf
Copy link
Collaborator

@torchiaf torchiaf commented Jun 28, 2024

Summary

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#5888

Occurred changes and/or fixed issues

Technical notes summary

  • Instance Labels and Advanced Options tabs have been rearranged to a different position; Advanced tab is now at the bottom of the list of VM tabs.
  • It adds the dropdown element to select the Maintenance Strategy label.
  • The label is excluded from Instance Labels tab

Areas or cases that should be tested

Areas which could experience regressions

Screenshot/Video

image

torchiaf added 3 commits June 28, 2024 10:48
Signed-off-by: Francesco Torchia <[email protected]>
Signed-off-by: Francesco Torchia <[email protected]>
export const RunStrategys = ['Always', 'RerunOnFailure', 'Manual', 'Halted'];
export const runStrategies = ['Always', 'RerunOnFailure', 'Manual', 'Halted'];

export const maintenanceStrategies = ['Migrate', 'RestartOnEnable', 'RestartOnDisable', 'StayOff'];
Copy link
Member

Choose a reason for hiding this comment

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

Please check the HEP for more information about how the strategies are named.

Copy link
Member

@votdev votdev left a comment

Choose a reason for hiding this comment

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

There are several issues that need to be adapted.

@torchiaf torchiaf requested a review from votdev July 1, 2024 16:32
@torchiaf
Copy link
Collaborator Author

torchiaf commented Jul 1, 2024

Thanks @votdev ; I've updated the label's values and changed the YAML field where it is saved -> metadata.labels .

Copy link
Member

@votdev votdev left a comment

Choose a reason for hiding this comment

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

LGTM in general. The solution to add a drop-down in the Advances Options tab is a nice idea.

I would suggest to add a comment to the commit + PR message that the Instance Labels tab has been rearranged to a different position and that the Advanced Options is now at the last position. This makes it easier to review the code.

torchiaf and others added 2 commits July 2, 2024 12:31
@torchiaf torchiaf merged commit f409bfb into harvester:master Jul 9, 2024
8 checks passed
@torchiaf
Copy link
Collaborator Author

@mergify backport release-harvester-v1.4

Copy link

mergify bot commented Jul 18, 2024

backport release-harvester-v1.4

✅ Backports have been created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants