Skip to content

upcoming: [M3-9495] - Disable APL for LKE-E clusters #11809

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.

Sign up for GitHub

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

Merged
mjac0bs merged 11 commits into from
Mar 14, 2025
Merged

upcoming: [M3-9495] - Disable APL for LKE-E clusters #11809

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e5dff3b
Show the APL section only for standard cluster tiers
Mar 7, 2025
0cd2df1
Added changeset: Disable APL for LKE-E clusters on create flow
Mar 7, 2025
ebfb57b
WIP - poc for what disabled, rather than hidden, section would look like
Mar 10, 2025
98942b8
Merge branch 'develop' into M3-9495-disable-apl-for-lke-e
Mar 11, 2025
b0626b2
Update with proposed UX changes
Mar 12, 2025
4ec7024
Update test coverage with mocked endpoints, UX changes
Mar 12, 2025
1000956
Tweak changeset
Mar 12, 2025
1fd39f0
Merge branch 'develop' into M3-9495-disable-apl-for-lke-e
Mar 12, 2025
ad5823f
Merge branch 'develop' into M3-9495-disable-apl-for-lke-e
Mar 12, 2025
b5362b1
Update chip text size to small for consistency with other chips
Mar 12, 2025
cfe66c9
Fix bug preventing 'no' button from being checked
Mar 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-11809-upcoming-features-1741383768710.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Disable APL for LKE-E clusters on create flow ([#11809](https://github.com/linode/manager/pull/11809))
6 changes: 6 additions & 0 deletions packages/manager/cypress/e2e/core/kubernetes/lke-create.spec.ts
View file
Open in desktop
Comment thread
mjac0bs marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,7 @@ describe('LKE Cluster Creation with LKE-E', () => {
* - Confirms that HA is enabled by default with LKE-E selection
* - Confirms an LKE-E supported region can be selected
* - Confirms an LKE-E supported k8 version can be selected
* - Confirms the APL section is not shown while it remains unsupported
* - Confirms at least one IP must be provided for ACL
* - Confirms the checkout bar displays the correct LKE-E info
* - Confirms an enterprise cluster can be created with the correct chip, version, and price
Expand Down Expand Up @@ -1215,6 +1216,11 @@ describe('LKE Cluster Creation with LKE-E', () => {
.should('be.enabled')
.click();

// Confirm the APL section is not shown.
cy.findByTestId('apl-label').should('not.exist');
cy.findByTestId('apl-radio-button-yes').should('not.exist');
cy.findByTestId('apl-radio-button-no').should('not.exist');

Copy link
Copy Markdown
Contributor

@jdamore-linode jdamore-linode Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Related to the above, can we add assertions to confirm that the APL section is present when LKE-E isn't selected?

Copy link
Copy Markdown
Contributor Author

@mjac0bs mjac0bs Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I believe this test is already accomplishing that, right? Do you mean toggling to an LKE standard tier and checking that it exists?

Copy link
Copy Markdown
Contributor

@jdamore-linode jdamore-linode Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Oh yeah, you're absolutely right. There is one distinction I'll call out which is that adding the assertions here gives us explicit coverage that the APL section is present when the LKE-E feature is enabled but not selected during cluster creation, whereas the other test doesn't take the state of LKE-E into account. So if there were ever a logic issue (or mocking issue in the test) involving APL/LKE-E that resulted in APL being hidden, it would get caught by this test but may not get caught by the other.

(I have no idea what the logic for the cluster create page is like, so if this comes across as a farfetched hypothetical we can definitely leave it alone, just wanted to point it out in case it's an important distinction from a dev's POV)

Copy link
Copy Markdown
Contributor Author

@mjac0bs mjac0bs Mar 12, 2025

Choose a reason for hiding this comment

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

I left the case of 'LKE-E feature enabled but LKE (standard) cluster tier selected' uncovered in tests because it is essentially the same flow as 'LKE-E feature disabled'. The code sets a default of standard for the selectedTier. The account beta endpoint determines whether or not the APL section is visible to users, and the section visible + the selectedTier determine whether the section is enabled or disabled.

Basically, if there were an issue with the LKE tier selection in the LKE-E flow, the first test could catch it.

// Confirm the expected available plans display.
validEnterprisePlanTabs.forEach((tab) => {
ui.tabList.findTabByTitle(tab).should('be.visible');
Expand Down
11 changes: 7 additions & 4 deletions packages/manager/src/features/Kubernetes/CreateCluster/CreateCluster.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ export const CreateCluster = () => {
isLoading: isLoadingKubernetesTypes,
} = useKubernetesTypesQuery(selectedTier === 'enterprise');

// LKE-E does not support APL at this time.
const isAPLEnabled = showAPL && selectedTier === 'standard';

const handleClusterTierSelection = (tier: KubernetesTier) => {
setSelectedTier(tier);

Expand Down Expand Up @@ -242,7 +245,7 @@ export const CreateCluster = () => {
region: selectedRegion?.id,
};

if (showAPL) {
if (isAPLEnabled) {
payload = { ...payload, apl_enabled };
}

Expand All @@ -251,7 +254,7 @@ export const CreateCluster = () => {
}

const createClusterFn =
showAPL || isLkeEnterpriseLAFeatureEnabled
isAPLEnabled || isLkeEnterpriseLAFeatureEnabled
? createKubernetesClusterBeta
: createKubernetesCluster;

Expand Down Expand Up @@ -447,7 +450,7 @@ export const CreateCluster = () => {
/>
</StyledDocsLinkContainer>
</StyledStackWithTabletBreakpoint>
{showAPL && (
{isAPLEnabled && (
<>
<Divider sx={{ marginTop: 4 }} />
<StyledStackWithTabletBreakpoint>
Expand All @@ -460,7 +463,7 @@ export const CreateCluster = () => {
</StyledStackWithTabletBreakpoint>
</>
)}
<Divider sx={{ marginTop: showAPL ? 1 : 4 }} />
<Divider sx={{ marginTop: isAPLEnabled ? 1 : 4 }} />
{showHighAvailability && selectedTier !== 'enterprise' && (
<Box data-testid="ha-control-plane">
<HAControlPlane
Expand Down