upcoming: [M3-10708] - Display maintenance type and config in linode_migrate event messages#13084
Conversation
…aintenance Type and Config Information
jaalah-akamai
left a comment
jaalah-akamai
left a comment
There was a problem hiding this comment.
Personal review comments
| const getMaintenanceDescription = ( | ||
| description: null | string | undefined | ||
| ): MaintenanceDescription => { | ||
| if (description === 'emergency' || description === 'scheduled') { | ||
| return description; | ||
| } | ||
| return 'maintenance'; | ||
| }; |
There was a problem hiding this comment.
Added getMaintenanceDescription helper function with type-safe validation using AccountMaintenance['description'] type
| {e.secondary_entity ? ( | ||
| <> | ||
| {' '} | ||
| with config <EventLink event={e} to="secondaryEntity" /> | ||
| </> | ||
| ) : ( |
There was a problem hiding this comment.
Added config information display when secondary_entity is present in migration events
| <> | ||
| <strong>{maintenanceType}</strong> maintenance | ||
| </> |
There was a problem hiding this comment.
Added linode_migrate event message factory to include maintenance type (emergency/scheduled/maintenance) with proper bold formatting
| const truncatedReason = reason ? truncate(reason, 93) : ''; | ||
|
|
||
| const isTruncated = reason !== truncatedReason; | ||
| const isTruncated = reason ? reason !== truncatedReason : false; |
There was a problem hiding this comment.
Fixed null handling for reason field in MaintenanceTableRow to prevent runtime errors
| type: 'migrate', | ||
| description: 'scheduled', | ||
| reason: 'Scheduled maintenance migration', | ||
| }), |
There was a problem hiding this comment.
Added dev tools templates for testing in-progress migrations with emergency/scheduled descriptions
| // Handle empty/whitespace text as null | ||
| if (!text.trim()) { | ||
| const event = { | ||
| currentTarget: { | ||
| name, | ||
| value: null, | ||
| }, | ||
| target: { | ||
| name, | ||
| value: null, | ||
| }, | ||
| } as unknown as React.ChangeEvent<HTMLTextAreaElement>; | ||
|
|
||
| onChange(event); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Fixed JsonTextArea component to properly detect and save null values when clearing fields
| completed: ['complete_time', 'End Date'], | ||
| 'in progress': ['start_time', 'Start Date'], | ||
| upcoming: ['start_time', 'Start Date'], | ||
| upcoming: ['when', 'Start Date'], |
There was a problem hiding this comment.
For upcoming maintenance events, start_time is null, but we can get this value from the when field which uses the same date. The "When" column uses a human readable label, whereas the "Start Date" column will display the date value
|
Looking at the logs, most of the failed tests appear flaky and unrelated to these changes cc: @dmcintyr-akamai |
|
i would guess that the failures in create-database.spec.ts are due to server-side resource contraints.
i am pushing another fix for the create-stackscripts. there was an additional cause of random failure that my PR will hopefully fix. and i don't think the failures in create-database.spec.ts are related to the fix for these tests that i just pushed. the new error is "timed out waiting |
|
fwiw i'm only getting failures on 2 tests now, linodes/linode-storage.spec.ts and firewalls/migrate-linode-with-firewall.spec.ts so i don't think you broke the others. |
dwiley-akamai
left a comment
dwiley-akamai
left a comment
There was a problem hiding this comment.
Verification steps ✅
Code review ✅
packages/manager/src/features/Account/Maintenance/MaintenanceTableRow.tsx
Outdated
Show resolved
Hide resolved
Cloud Manager UI test results🔺 17 failing tests on test run #9 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: pnpm cy:run -s "cypress/e2e/core/images/smoke-create-image.spec.ts,cypress/e2e/core/linodes/linode-config.spec.ts,cypress/e2e/core/volumes/delete-volume.spec.ts,cypress/e2e/core/linodes/linode-storage.spec.ts,cypress/e2e/core/firewalls/migrate-linode-with-firewall.spec.ts,cypress/e2e/core/linodes/resize-linode.spec.ts,cypress/e2e/core/databases/create-database.spec.ts,cypress/e2e/core/volumes/upgrade-volume.spec.ts,cypress/e2e/core/volumes/attach-volume.spec.ts,cypress/e2e/core/images/machine-image-upload.spec.ts" |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
5 participants
Description 📝
This PR enhances
linode_migrateevent messages to display maintenance type (emergency/scheduled) and config information when available. Previously, migration events only showed "Linode linode-1 is being migrated" without context about the maintenance type or which config was being used.Changes 🔄
The changes ensure that:
secondary_entityis present in the eventdescriptionfields by defaulting to "maintenance"whenfield as opposed to thestart_timewhich are essentially the same.Additionally, this PR includes bug fixes for null handling in maintenance-related components and improvements to dev tools for testing these scenarios.
Scope 🚢
Upon production release, changes in this PR will be visible to:
Target release date 🗓️
December 2nd
Preview 📷
Before:
After:
recording.mov
How to test 🧪
Prerequisites
vmHostMaintenancefeature flagReproduction steps
Verification steps
description: 'emergency'description: 'scheduled'secondary_entityis present: "with config [config name]"secondary_entityis nullsecondary_entityfield in dev tools and verify it saves as nullreasonfield is null in maintenance recordsAuthor 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
As an Author, before moving this PR from Draft to Open, I confirmed ✅