fix: [M3-8726] - Users unable to upgrade Kubernetes version from landing page #11056

bnussman-akamai · 2024-10-07T19:56:24Z

Description 📝

Fixes a bug 🐛 preventing users from upgrading their Kubernetes cluster from the Kubernetes landing page ⬆️
I believe the bug was caused by feat: [M3-7685] - Show empty Kubernetes landing page with disabled create button for restricted users #10756
- Specifically switching isLoading ➡️ isFetching - see comment for explanation
Adds Cypress test that would have caught this bug 🧪
- (the test uses mocked API data)

Preview 📷

Before	After
Screen.Recording.2024-10-07.at.3.49.26.PM.mov	Screen.Recording.2024-10-07.at.3.55.16.PM.mov

How to test 🧪

Reproduction steps

Spin up a Kubernetes cluster that is not on the latest version
Attempt the upgrade flow on the Kubernetes landing page
Observe that when clicking "Upgrade Version", the dialog will not proceed to step 2

Verification steps

Spin up a Kubernetes cluster that is not on the latest version
Test the upgrade flow on the Kubernetes landing page
Verify you see step 1 and step 2

As an Author I have considered 🤔

github-actions · 2024-10-07T20:21:44Z

Coverage Report: ✅
Base Coverage: 86.95%
Current Coverage: 86.95%

bnussman-akamai · 2024-10-07T20:43:31Z

packages/manager/src/features/Kubernetes/KubernetesLanding/KubernetesLanding.tsx

@@ -98,7 +98,7 @@ export const KubernetesLanding = () => {

  const isRestricted = profile?.restricted ?? false;

-  const { data, error, isFetching } = useKubernetesClustersQuery(


For context

isLoading is true whenever the first fetch for the query is in-flight
isFetching is true whenever any fetch for the query is in-flight

Explanation

In the upgrade dialog, useKubernetesClusterMutation is called, which invalidates the paginated list of Kubernets clusters, which causes useKubernetesClustersQuery's isFetching to become true, which unmounted the upgrade dialog because of the if statement below. Switching to isLoading allows the Upgrade dialog to stay mounted during the upgrade, which fixes the bug

There are probably other ways to fix this bug. For example, the UpgradeVersionModal component could probably be made more robust. I decided to go with this fix because I think isLoading is the desired loading state here rather than isFetching anyway

Thanks for the context. Agreed that the dialog is not in the best shape but that is out of scope.

coliu-akamai · 2024-10-07T21:45:07Z

The recent test addition was failing on my end when I ran it locally, but I saw it passed in this run here (and then I tried rerunning the tests but received a couple errors - not sure if I'm missing permissions or something?). Will look into some more!

jaalah-akamai

✅ Thanks for catching and will keep an eye on this detail for future

mjac0bs

Thanks for fixing this. Can we convert the internal PDI ticket to an M3 ticket, so we have it in our project, and update this PR title?

I don't know how Jaalah managed to get the tests to pass. I was seeing the same thing as Connie. Left a comment on the spec about what seems to fix it.

Other than that, this looks good for approving pending the test update!

mjac0bs · 2024-10-08T20:54:54Z

packages/manager/cypress/e2e/core/kubernetes/lke-update.spec.ts

+    ui.button
+      .findByTitle('Upgrade Version')
+      .should('be.visible')
+      .should('be.enabled')
+      .click();
+
+    mockGetClusters([updatedCluster]).as('getClusters');
+    cy.wait(['@updateCluster', '@getClusters']);


Suggested change

ui.button

.findByTitle('Upgrade Version')

.should('be.visible')

.should('be.enabled')

.click();

mockGetClusters([updatedCluster]).as('getClusters');

cy.wait(['@updateCluster', '@getClusters']);

mockGetClusters([updatedCluster]).as('getClusters');

ui.button

.findByTitle('Upgrade Version')

.should('be.visible')

.should('be.enabled')

.click();

cy.wait(['@updateCluster', '@getClusters']);

This appears to be what we actually need here to get the version to update to 1.26. I am not 100% clear on why the mock needs to go before the user action, but know that often when the right requests are being made, but the right data isn't showing, it's this kind of (timing?) issue.

glad I wasn't going crazy! I'm also curious to how this test managed to pass in the automatic test checks when it's been failing for us.. 😅

Also seeing this test fail locally.

Good catch @mjac0bs

I knew something felt weird when I wrote that

Fixed in 1d66657

hkhalil-akamai

Thanks for looking into this and adding relevant tests 🧪. Also seeing flake in the new test along with @mjac0bs. Approved pending test is fixed and fix changeset is added.

hkhalil-akamai · 2024-10-08T21:09:27Z

packages/manager/src/features/Kubernetes/KubernetesLanding/KubernetesLanding.tsx

@@ -98,7 +98,7 @@ export const KubernetesLanding = () => {

  const isRestricted = profile?.restricted ?? false;

-  const { data, error, isFetching } = useKubernetesClustersQuery(


Thanks for the context. Agreed that the dialog is not in the best shape but that is out of scope.

hkhalil-akamai · 2024-10-08T21:12:14Z

packages/manager/cypress/e2e/core/kubernetes/lke-update.spec.ts

+    ui.button
+      .findByTitle('Upgrade Version')
+      .should('be.visible')
+      .should('be.enabled')
+      .click();
+
+    mockGetClusters([updatedCluster]).as('getClusters');
+    cy.wait(['@updateCluster', '@getClusters']);


Also seeing this test fail locally.

cypress · 2024-10-09T00:04:24Z

Cloud Manager E2E Run #6644

Run Properties: Failed #6644 • 3e6d480c7c: fix: [M3-8726] - Users unable to upgrade Kubernetes version from landing page (#...

Project	`Cloud Manager E2E`
Run status	`Failed #6644`
Run duration	`27m 08s`
Commit	`3e6d480c7c: fix: [M3-8726] - Users unable to upgrade Kubernetes version from landing page (#...`
Committer	`Banks Nussman`
View all properties for this run ↗︎

Test results
Failures	`1`
Flaky	`3`
Pending	`2`
Skipped	`0`
Passing	`429`

Tests for review

cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts • 1 failed test

View Output Video

Test		Artifacts
Placement Group deletion > can delete with Linodes assigned when unexpected error show up and retry		`Screenshots` `Video`

switch-linode-state.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
switch linode state > reboots a linode from details page		`Screenshots` `Video`

linode-config.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
Linode Config management > End-to-End > Clones a config		`Screenshots` `Video`

rebuild-linode.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
rebuild linode > cannot rebuild a provisioning linode		`Screenshots` `Video`

…ing page (linode#11056) * fix dialog unmounting during upgrade and add cypress test * fix cypress test * add changeset --------- Co-authored-by: Banks Nussman <banks@nussman.us>

fix dialog unmounting during upgrade and add cypress test

edb4a12

bnussman-akamai added the LKE Related to Linode Kubernetes Engine offerings label Oct 7, 2024

bnussman-akamai self-assigned this Oct 7, 2024

bnussman-akamai requested review from a team as code owners October 7, 2024 19:56

bnussman-akamai requested review from cliu-akamai, hkhalil-akamai and jaalah-akamai and removed request for a team October 7, 2024 19:56

bnussman-akamai commented Oct 7, 2024

View reviewed changes

bnussman-akamai added the Bug Fixes for regressions or bugs label Oct 8, 2024

jaalah-akamai approved these changes Oct 8, 2024

View reviewed changes

mjac0bs self-requested a review October 8, 2024 18:01

mjac0bs approved these changes Oct 8, 2024

View reviewed changes

hkhalil-akamai approved these changes Oct 8, 2024

View reviewed changes

hkhalil-akamai added Approved Multiple approvals and ready to merge! Missing Changeset labels Oct 8, 2024

bnussman added 2 commits October 8, 2024 18:54

fix cypress test

1d66657

add changeset

bf99d87

bnussman-akamai removed the Missing Changeset label Oct 8, 2024

bnussman-akamai changed the title ~~fix: [PDI-3057] - Users unable to upgrade Kubernetes version from landing page~~ fix: [M3-8726] - Users unable to upgrade Kubernetes version from landing page Oct 8, 2024

bnussman-akamai merged commit 3e6d480 into linode:develop Oct 8, 2024
20 checks passed

This was referenced Oct 10, 2024

Release v1.130.0 - release → staging #11087

Merged

Release v1.130.0- staging → master #11095

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: [M3-8726] - Users unable to upgrade Kubernetes version from landing page #11056

fix: [M3-8726] - Users unable to upgrade Kubernetes version from landing page #11056

bnussman-akamai commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 7, 2024 •

edited

Loading

bnussman-akamai Oct 7, 2024

hkhalil-akamai Oct 8, 2024

coliu-akamai commented Oct 7, 2024

jaalah-akamai left a comment

mjac0bs left a comment

mjac0bs Oct 8, 2024

coliu-akamai Oct 8, 2024

hkhalil-akamai Oct 8, 2024

bnussman-akamai Oct 8, 2024

hkhalil-akamai left a comment

hkhalil-akamai Oct 8, 2024

hkhalil-akamai Oct 8, 2024

cypress bot commented Oct 9, 2024

		@@ -98,7 +98,7 @@ export const KubernetesLanding = () => {

		const isRestricted = profile?.restricted ?? false;

		const { data, error, isFetching } = useKubernetesClustersQuery(

fix: [M3-8726] - Users unable to upgrade Kubernetes version from landing page #11056

fix: [M3-8726] - Users unable to upgrade Kubernetes version from landing page #11056

Conversation

bnussman-akamai commented Oct 7, 2024 • edited Loading

Description 📝

Preview 📷

How to test 🧪

Reproduction steps

Verification steps

As an Author I have considered 🤔

github-actions bot commented Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

For context

Explanation

Choose a reason for hiding this comment

coliu-akamai commented Oct 7, 2024

jaalah-akamai left a comment

Choose a reason for hiding this comment

mjac0bs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hkhalil-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Oct 9, 2024

Tests for review

bnussman-akamai commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 7, 2024 •

edited

Loading