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

Migrated editing imported cluster to Vue #13216

Merged
merged 9 commits into from
Feb 3, 2025

Conversation

eva-vashkevich
Copy link
Member

@eva-vashkevich eva-vashkevich commented Jan 27, 2025

Summary

Fixes #9476 #10432

Migrated editing imported cluster to Vue.

Occurred changes and/or fixed issues

  • Made name field uneditable to be consistent with RKE2/K3s clusters.
  • Removed Project Network Isolation for K3s cluster as it is not supported for RKE2/K3s imported clusters.
  • Authorized endpoint has been moved to a separate tab to accommodate upcoming features.

Technical notes summary

This PR only covers editing functionality. I will move creation over as part of #13151

Areas or cases that should be tested

  1. Import a generic cluster. This should not be changed.
  2. Before actually importing the cluster, go to edit. Make sure that only "Member roles" and "Labels and annotations" tabs are present.
  3. Finish importing an RKE2 cluster. Go to edit. Make sure now we also have "RKE2 Options" tab.
  4. Import a K3s cluster. Go to edit. Now we should have a "K3s Options" tab and "Advanced" tab. Advanced tab should allow you to add Environment variables
  5. Go to edit Local cluster. It should match either RKE2 or K3s experience (depending on which one you have). Additionally, you should be able to add cluster description.
  6. For each 3 of these cases:

Areas which could experience regressions

  1. Provisioning of non-imported clusters could be affected
  2. Provisioning of imported non-generic clusters could be affected

Screenshot/Video

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

Copy link
Member

@mantis-toboggan-md mantis-toboggan-md left a comment

Choose a reason for hiding this comment

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

I tested imported and local rke2 and k3s clusters and everything seemed functional, keeping in mind the changes you highlighted in your demo (showing networking while cluster is pending; not showing PNI for rke2/k3s)

I noticed some options have changed for imported and local rke1 (generic?) clusters, but I also see some weird behavior in the Ember UI, so it's not clear to me how much of the differences are just you fixing existing bugs:

  • In ember imported rke1 clusters in a 'pending' state have PNI and agent env var inputs that go away once the cluster is provisioned - is it a bug that they were shown at all?
  • In ember imported/local rke1 clusters do not have authorized endpoint inputs...I don't see errors when configuring them with this PR but wasn't sure exposing them was intentional
  • local rke1 cluster in ember has agent env var and PNI inputs. This form seems a little broken in general, though: by default I couldn't save and had to manually add an admin user

Aside from my rke1 confusion the PR generally looks good, I had only minor comments

pkg/imported/components/Basics.vue Outdated Show resolved Hide resolved
pkg/imported/components/CruImported.vue Outdated Show resolved Hide resolved
pkg/imported/provisioner.ts Outdated Show resolved Hide resolved
@eva-vashkevich
Copy link
Member Author

@mantis-toboggan-md

  • In ember imported rke1 clusters in a 'pending' state have PNI and agent env var inputs that go away once the cluster is provisioned - is it a bug that they were shown at all?
    I intentionally removed PNI because I don't see an option to configure network policy anywhere for RKE1 and it doesn't make sense to give a warning about configuring something we don't let users configure? (and it is not supported for rke2/k3s) so I don't think any case this wizard covers will actually have it.
  • Agent env and authorized endpoints should actually be supported by all 3 (k3s, rke1 and rke2), so I enabled it for them.

Copy link
Member

@mantis-toboggan-md mantis-toboggan-md left a comment

Choose a reason for hiding this comment

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

LGTM - reviewed updates with both k3s/rke2 and rke1 clusters

@eva-vashkevich eva-vashkevich merged commit 06c788e into rancher:master Feb 3, 2025
34 checks passed
@eva-vashkevich eva-vashkevich deleted the iss9476 branch February 3, 2025 21:44
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.

Migrate imported cluster edit pages from Ember to Vue
2 participants