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

Import EKS clusters #13517

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mantis-toboggan-md
Copy link
Member

@mantis-toboggan-md mantis-toboggan-md commented Feb 26, 2025

Summary

Fixes #12409

Occurred changes and/or fixed issues

Similar to #13447

Areas or cases that should be tested

For testing purposes you should be to use my existing eks cluster nb-eks inn the us-west-2 region

  1. Navigate to the cluster management list view and click 'import existing'
    2 Select EKS
  2. Enter credentials: if the rancher instance already has at least on aws credential defined, you should see a dropdown to pick it and an input for the project id. If you don't already have a aws credential you should see the form to add one
  3. Click 'authenticate' - the remainder of the form should load (smaller form than EKS creation)
    5.Try selecting different regions - the cluster dropdown should reload (you should see aws api calls in the network tab)
  4. Name your cluster - verify that name form validators are used
    7.Add users, labels and annotations as well
  5. Click save
  6. Wait for the cluster to become available
  7. Edit the cluster and verify that:
  • you see the same form used to edit EKS clusters provisioned through Rancher
  • labels, annotations, and membership configured during creation appears as expected

Areas which could experience regressions

The existing EKS cluster creation flow should be tested.

Screenshot/Video

Screenshot 2025-02-26 at 6 43 49 AM

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

@@ -596,7 +635,7 @@ export default defineComponent({
<LabeledInput
required
label-key="eks.clusterName.label"
:value="config.displayName"
:value="normanCluster.name"
Copy link
Member Author

@mantis-toboggan-md mantis-toboggan-md Feb 26, 2025

Choose a reason for hiding this comment

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

config.displayName = eks cluster name in aws (set by the Import component)
normanCluster.name = rancher cluster object's name

}

const clusterNameRequired = (ctx: CruEKSContext) => {
return (): string | null => {
return !ctx.config.displayName ? ctx.t('validation.required', { key: ctx.t('nameNsDescription.name.label') }) : null;
return !ctx.normanCluster.name ? ctx.t('validation.required', { key: ctx.t('nameNsDescription.name.label') }) : null;
Copy link
Member Author

Choose a reason for hiding this comment

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

validate the name field that the user types instead of the displayName, which may come from eks API

@mantis-toboggan-md mantis-toboggan-md changed the title Eks import Port EKS import page to vue Feb 26, 2025
@mantis-toboggan-md mantis-toboggan-md changed the title Port EKS import page to vue Improt EKS clusters Feb 26, 2025
@mantis-toboggan-md mantis-toboggan-md changed the title Improt EKS clusters Import EKS clusters Feb 26, 2025
@mantis-toboggan-md mantis-toboggan-md added this to the v2.11.0 milestone Feb 26, 2025
<template>
<div class="row mb-10 align-center">
<div class="col span-6">
<LabeledSelect
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to make this a required field, it can't think of a scenario where we wouldn't want it.

Without it we'll get an error:
image

Copy link
Contributor

@codyrancher codyrancher left a comment

Choose a reason for hiding this comment

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

Other than the one change request it lgtm.

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.

Port EKS import cluster page from Ember to Vue
2 participants