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-8839] - Designate LKE-E clusters with 'Enterprise' chip #11442

Merged

Conversation

mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Dec 19, 2024

Description 📝

For listing and viewing cluster information, customers should be able to have explicit indication whether the cluster is an LKE vs LKE-E cluster.

To do this, we will add add an 'ENTERPRISE' chip on the LKE landing page and the cluster details page.

Changes 🔄

  • Create a new ClusterChip component to display both chip types when applicable
    • Provide unit test coverage for new component
  • Remove the Kubernetes Dashboard button for enterprise clusters (not related to this ticket, but same component, so made the change)
  • Fix a spacing issue in KubeSpecs due to enterprise versions

Note: Yes, the total price of the LKE-E cluster is inaccurate. This will be addressed by M3-9042.

Target release date 🗓️

1/14 (dev)

Preview 📷

Location Image
LKE Landing Screenshot 2024-12-19 at 12 33 45 PM
LKE Details Screenshot 2024-12-19 at 12 33 54 PM
LKE Details Responsive Design
Screen.Recording.2024-12-19.at.12.35.20.PM.mov

How to test 🧪

Prerequisites

(How to setup test environment)

  • Have the LKE-E customer tag on your account (see Project Tracker)
  • Have the feature flag on locally
  • Have created an LKE-E cluster through the create flow

Verification steps

(How to verify changes)

  • With the feature flag on, observe the LKE-E cluster chips for your enterprise cluster on the LKE landing page and LKE details page.
  • Resize the screen to tablet and mobile nd observe that the chips and kube specs display appropriately.
  • Observe that the Kubernetes Dashboard link is not visible at the top of the summary - this is not supported for LKE-E so it was always disabled for these clusters.
  • Observe no Enterprise chip when the feature flag is off.
  • Confirm tests pass:
yarn test ClusterChips
Author Checklists

As an Author, to speed up the review process, I 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 self-assigned this Dec 19, 2024
@@ -134,7 +139,13 @@ export const KubeClusterSpecs = React.memo((props: Props) => {
};

return (
<Grid container direction="row" lg={3} xs={12}>
<Grid
columnGap={matchesColGapBreakpointDown ? 2 : 0}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to the intended changes in this PR, but fixes the issue where the kube versions (which are longer because they include patch+lke version) was too close to the data in the right column at certain screen sizes.

Develop This Branch
Screenshot 2024-12-19 at 9 52 14 AM Screenshot 2024-12-19 at 10 05 18 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New component to render both HA and Enterprise chips, since they are use on both landing and details pages.

Comment on lines +124 to +130
[theme.breakpoints.down('sm')]: {
'& .MuiChip-root': {
marginRight: 0,
},
flexDirection: 'column',
},
})}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Styling to prevent the absolutely positioned chips from overlapping with specs text at smaller viewports.

@mjac0bs mjac0bs marked this pull request as ready for review December 19, 2024 23:39
@mjac0bs mjac0bs requested a review from a team as a code owner December 19, 2024 23:39
@mjac0bs mjac0bs requested review from bnussman-akamai and cpathipa and removed request for a team December 19, 2024 23:39
@mjac0bs mjac0bs force-pushed the M3-8839-add-enterprise-chip-to-lke-e-clusters branch from 8d2d506 to ebe5d1b Compare December 19, 2024 23:41
Copy link

github-actions bot commented Dec 20, 2024

Coverage Report:
Base Coverage: 86.97%
Current Coverage: 86.97%

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Looks good pending some test clean up 🧹

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Jan 2, 2025
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.

Looks good! Left some minor non-blocking comments

@hana-akamai hana-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! Ready for Review labels Jan 2, 2025
…ips.test.tsx

Co-authored-by: Hana Xu <115299789+hana-akamai@users.noreply.github.com>
…il/KubeClusterSpecs.tsx

Co-authored-by: Hana Xu <115299789+hana-akamai@users.noreply.github.com>
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 1 failing test on test run #7 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
1 Failing468 Passing2 Skipped107m 29s

Details

Failing Tests
SpecTest
lke-update.spec.tsLKE cluster updates » can add and delete node pool tags

Troubleshooting

Use this command to re-run the failing tests:

yarn cy:run -s "cypress/e2e/core/kubernetes/lke-update.spec.ts"

@mjac0bs
Copy link
Contributor Author

mjac0bs commented Jan 2, 2025

Test failure is fixed in develop (#11458) and unrelated to the changes in this PR.

@mjac0bs mjac0bs merged commit de24b40 into linode:develop Jan 2, 2025
22 of 23 checks passed
dmcintyr-akamai pushed a commit to dmcintyr-akamai/manager that referenced this pull request Jan 9, 2025
…linode#11442)

* Create component for rendering cluster chips

* Add test coverage for new component

* Replace current chip with ClusterChips component

* Fix spacing in column specs due to long LKE-E version

* Remove Kube dashboard button for LKE-E clusters

* Fix the fix for specs spacing

* Add changeset

* Fix a conditional before I forget

* Address feedback - clean up flag mocks in test

* Update packages/manager/src/features/Kubernetes/ClusterList/ClusterChips.test.tsx

Co-authored-by: Hana Xu <115299789+hana-akamai@users.noreply.github.com>

* Update packages/manager/src/features/Kubernetes/KubernetesClusterDetail/KubeClusterSpecs.tsx

Co-authored-by: Hana Xu <115299789+hana-akamai@users.noreply.github.com>

---------

Co-authored-by: Hana Xu <115299789+hana-akamai@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! LKE-Enterprise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants