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

upcoming: [M3-8840] - Add Cluster Type selection to LKE Create Cluster flow #11322

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Nov 25, 2024

Description 📝

We want to allow users the ability to create an LKE-Enterprise cluster in the create flow.

Changes 🔄

  • A new Cluster Type section exists in the Create Cluster form.
  • Copy describes the section and a docs link with placeholder content.
  • The cards handle enabled/disabled state as follows:
    • When the feature flag is disabled, do not display the Cluster Type Panel.
    • When the feature flag is enabled but the user lacks the LKE-Enterprise account capability, the Enterprise panel is disabled and displays a tooltip.
    • When the feature flag is enabled and the user has the LKE-Enterprise account capability, the Enterprise panel is enabled and LKE (standard) is selected by default.
  • When the Enterprise cluster type is enabled, the HA Control Plane section is not visible.
  • Test coverage confirms the above.

Preview 📷

Flag ON with Capability Flag ON without Capability
Screenshot 2024-11-25 at 1 52 27 PM Screenshot 2024-11-25 at 1 52 34 PM Screenshot 2024-11-25 at 1 34 28 PM Screenshot 2024-11-25 at 1 34 44 PM

How to test 🧪

Prerequisites

(How to setup test environment)

  • Have LKE-E customer tag on your account (see Project Tracker)

Verification steps

(How to verify changes)

  • Go to http://localhost:3000/kubernetes/create
  • Confirm the Cluster Type section shows as expected:
    • Feature flag in dev tools ON, customer tag ON: Cluster Type is visible; HA section is not visible when Enterprise is selected
    • Feature flag in dev tools OFF, customer tag ON: Cluster Type is not visible; HA section is always visible
    • Feature flag in dev tools ON, customer tag OFF: Cluster Type is visible; Enterprise selection is disabled; HA section is always visible
  • Confirm test coverage passes:
yarn cy:run -s "cypress/e2e/core/kubernetes/lke-create.spec.ts"

As an Author, I have considered 🤔

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@mjac0bs mjac0bs marked this pull request as ready for review November 25, 2024 21:58
@mjac0bs mjac0bs requested review from a team as code owners November 25, 2024 21:58
@mjac0bs mjac0bs requested review from AzureLatte, carrillo-erik, cpathipa and hana-akamai and removed request for a team November 25, 2024 21:58
@@ -87,6 +88,11 @@ export interface SelectionCardProps {
* Optional text to set in a tooltip when hovering over the card.
*/
tooltip?: JSX.Element | string;
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this prop to be able to control the placement of the tooltip - it was covering the helper text and we could avoid that by placing elsewhere.

@@ -51,11 +51,10 @@ export const StyledFieldWithDocsStack = styled(Stack, {
}));

export const StyledDocsLinkContainer = styled(Box, {
label: 'StyledRegionSelectStack',
label: 'StyledDocsLinkContainer',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes to use the same styling for both the cluster type and DC pricing docs links.

@@ -191,29 +191,34 @@ export const getLatestVersion = (
* Hook to determine if the LKE-Enterprise feature should be visible to the user.
* Based on the user's account capability and the feature flag.
*
* @returns {boolean, boolean} - Whether the LKE-Enterprise feature is enabled for the current user in LA and GA, respectively.
* @returns {boolean, boolean, boolean, boolean} - Whether the LKE-Enterprise flags are enabled for LA/GA and whether feature is enabled for LA/GA (flags + account capability).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes were to account for the fact that the Create flow shows the Cluster Type section as long as the LD feature flag is on; it does not depend on account capability. The reason for this is to show all customers what is potentially available, even if not currently on their contract.

Copy link

github-actions bot commented Nov 25, 2024

Coverage Report:
Base Coverage: 86.93%
Current Coverage: 86.93%

Copy link
Contributor

@hana-akamai hana-akamai left a comment

Choose a reason for hiding this comment

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

I think we'll need to explicitly clear the HA values when LKE Enterprise is selected

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 3 failing tests on test run #6 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
3 Failing455 Passing2 Skipped111m 38s

Details

Failing Tests
SpecTest
machine-image-upload.spec.tsmachine image » uploads machine image, mock upload canceled failed event
machine-image-upload.spec.tsmachine image » uploads machine image, mock failed to decompress failed event
machine-image-upload.spec.tsmachine image » uploads machine image, mock expired upload event

Troubleshooting

Use this command to re-run the failing tests:

yarn cy:run -s "cypress/e2e/core/images/machine-image-upload.spec.ts,cypress/e2e/core/images/machine-image-upload.spec.ts,cypress/e2e/core/images/machine-image-upload.spec.ts"

@hana-akamai hana-akamai added the Add'tl Approval Needed Waiting on another approval! label Nov 27, 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