fix(upgrade-service): allow no preflights and no config #4942

JGAntunes · 2024-10-10T13:24:15Z

What this PR does / why we need it:

This PR fixes the upgrade flow for when an upgrade doesn't have config or preflight checks (the shortcut story has a video which illustrates the whole flow).

Which issue(s) this PR fixes:

https://app.shortcut.com/replicated/story/113259/new-upgrade-workflow-doesn-t-work-if-there-is-no-config
https://app.shortcut.com/replicated/story/113787/upgrade-workflow-doesn-t-work-if-you-don-t-have-preflights

Does this PR require a test?

Yes, I'll be looking into adding a playwright test to the EC repo.

Does this PR require a release note?

Fix for the upgrade flow in embedded cluster when there's no config and/or preflight checks available.

Does this PR require documentation?

NONE

CLAassistant · 2024-10-10T13:24:21Z

All committers have signed the CLA.

miaawong

just two little linter errors! Great work Joao!

web/src/components/upgrade_service/ConfirmAndDeploy.tsx

web/src/components/upgrade_service/UpgradeService.tsx

laverya · 2024-10-11T19:54:24Z

lint-web has a few failures - any reason those can't be fixed?

pkg/upgradeservice/handlers/config.go

sgalsaleh

I see a unit test for info API call, but not one for validating that the UI shows what's expected based on the values of hasPreflight and isConfigurable.

web/config/jest-setup.ts

JGAntunes · 2024-10-14T19:14:23Z

web/package.json

+    "testEnvironmentOptions": {
+      "customExportConditions": [
+        ""
+      ]
+    },


https://mswjs.io/docs/migrations/1.x-to-2.x#cannot-find-module-mswnode-jsdom

JGAntunes · 2024-10-14T19:21:09Z

web/src/jest-setup.ts

+import "intersection-observer";
+import "@testing-library/jest-dom";


There's a whole set of things which aren't pollyfilled by jsdom which is what the @testing-library/jest-dom is for. We also need to pollyfill the intersection-observer for the AppConfig component to render.

JGAntunes · 2024-10-14T19:25:34Z

Ok, after fighting a lot with jest, react router and its related friends I think this is ready for review. Could I get a 👀 again @sgalsaleh and @miaawong 🙏 There's a bit of noise in the tests because some of the components we're testing end up logging a lot but the tests are 🟢 if y'all want I can eventually remove the console.* calls we have spread out, or potentially silence them in some way in jest.

sgalsaleh · 2024-10-14T19:34:04Z

web/src/components/upgrade_service/UpgradeService.test.tsx

+        </MemoryRouter>
+      );
+
+      await findByTestId("preflight-check-area");


is this right? should this be available if preflights are not available?

Ah nice catch, thanks! This doesn't make sense though, there's something in the tests that's leading to this behaviour because if I it.only this test it fails as expected, if I run the whole file it passes 🙃 need to debug this further tomorrow 😩 I wonder if it's something to do with the http interception.

Managed to reproduce the failure locally while running the full harness of tests, turns out the state of the interceptors wasn't being correctly reset. I've also created different slugs per test to ensure we have fully decoupled setup processes (and prevent stuff like this in the future), fixed via - 84eb952

JGAntunes added type::bug Something isn't working bug::normal labels Oct 11, 2024

JGAntunes requested a review from sgalsaleh October 11, 2024 10:44

JGAntunes marked this pull request as ready for review October 11, 2024 10:44

JGAntunes requested a review from miaawong October 11, 2024 14:02

miaawong previously approved these changes Oct 11, 2024

View reviewed changes

web/src/components/upgrade_service/ConfirmAndDeploy.tsx Outdated Show resolved Hide resolved

web/src/components/upgrade_service/UpgradeService.tsx Show resolved Hide resolved

JGAntunes dismissed miaawong’s stale review via 8000c68 October 11, 2024 14:38

laverya reviewed Oct 11, 2024

View reviewed changes

pkg/upgradeservice/handlers/config.go Show resolved Hide resolved

sgalsaleh reviewed Oct 11, 2024

View reviewed changes

JGAntunes and others added 8 commits October 14, 2024 20:07

fix(upgrade-service): allow no preflights and no config

e7cb903

chore: refactor the get info request a bit

899944b

chore: conditional components for the deploy screen

a8e49c4

fix: back buttons

054c6df

chore: make linter happy

693aa5f

fix(upgrade_service): stop running preflights on save config

b717b9b

chore: add tests to the get upgrade info hook

c59d3c0

chore: add more tests

d37f4ed

JGAntunes force-pushed the jooantunes/sc-113259/new-upgrade-workflow-doesn-t-work-if-there branch from 9f7b297 to d37f4ed Compare October 14, 2024 19:08

JGAntunes commented Oct 14, 2024

View reviewed changes

web/config/jest-setup.ts Outdated Show resolved Hide resolved

JGAntunes commented Oct 14, 2024

View reviewed changes

JGAntunes added 2 commits October 14, 2024 20:18

chore: make the linter happy

4562729

chore: wtv happened to the jest config I had

675c9e6

JGAntunes commented Oct 14, 2024

View reviewed changes

JGAntunes requested review from laverya, miaawong and sgalsaleh October 14, 2024 19:25

sgalsaleh reviewed Oct 14, 2024

View reviewed changes

sgalsaleh previously approved these changes Oct 14, 2024

View reviewed changes

fix: test assertion

c26e0ae

JGAntunes dismissed sgalsaleh’s stale review via c26e0ae October 14, 2024 20:34

fix: test state

84eb952

JGAntunes requested a review from sgalsaleh October 15, 2024 09:46

sgalsaleh approved these changes Oct 15, 2024

View reviewed changes

sgalsaleh merged commit d20730a into main Oct 15, 2024
122 checks passed

sgalsaleh deleted the jooantunes/sc-113259/new-upgrade-workflow-doesn-t-work-if-there branch October 15, 2024 14:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(upgrade-service): allow no preflights and no config #4942

fix(upgrade-service): allow no preflights and no config #4942

JGAntunes commented Oct 10, 2024 •

edited

Loading

CLAassistant commented Oct 10, 2024 •

edited

Loading

miaawong left a comment

laverya commented Oct 11, 2024

sgalsaleh left a comment

JGAntunes Oct 14, 2024

JGAntunes Oct 14, 2024

JGAntunes commented Oct 14, 2024

sgalsaleh Oct 14, 2024

JGAntunes Oct 14, 2024 •

edited

Loading

JGAntunes Oct 15, 2024

		import "intersection-observer";
		import "@testing-library/jest-dom";

fix(upgrade-service): allow no preflights and no config #4942

fix(upgrade-service): allow no preflights and no config #4942

Conversation

JGAntunes commented Oct 10, 2024 • edited Loading

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR require a test?

Does this PR require a release note?

Does this PR require documentation?

CLAassistant commented Oct 10, 2024 • edited Loading

miaawong left a comment

Choose a reason for hiding this comment

laverya commented Oct 11, 2024

sgalsaleh left a comment

Choose a reason for hiding this comment

JGAntunes Oct 14, 2024

Choose a reason for hiding this comment

JGAntunes Oct 14, 2024

Choose a reason for hiding this comment

JGAntunes commented Oct 14, 2024

sgalsaleh Oct 14, 2024

Choose a reason for hiding this comment

JGAntunes Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

JGAntunes Oct 15, 2024

Choose a reason for hiding this comment

JGAntunes commented Oct 10, 2024 •

edited

Loading

CLAassistant commented Oct 10, 2024 •

edited

Loading

JGAntunes Oct 14, 2024 •

edited

Loading