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-8420] – Add conditionally-displayed reboot notice on Volume Create page #10868

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Add conditional client library update required reboot notice to Volume Create page ([#10868](https://github.com/linode/manager/pull/10868))
120 changes: 118 additions & 2 deletions packages/manager/cypress/e2e/core/volumes/create-volume.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { Linode, Region } from '@linode/api-v4';
import { createTestLinode } from 'support/util/linodes';
import { createLinodeRequestFactory } from 'src/factories/linodes';
import {
createLinodeRequestFactory,
linodeFactory,
} from 'src/factories/linodes';
import { authenticate } from 'support/api/authentication';
import { cleanUp } from 'support/util/cleanup';
import {
Expand All @@ -15,6 +18,10 @@ import { mockAppendFeatureFlags } from 'support/intercepts/feature-flags';
import { accountFactory, regionFactory, volumeFactory } from 'src/factories';
import { mockGetAccount } from 'support/intercepts/account';
import { mockGetRegions } from 'support/intercepts/regions';
import {
mockGetLinodeDetails,
mockGetLinodes,
} from 'support/intercepts/linodes';

// Local storage override to force volume table to list up to 100 items.
// This is a workaround while we wait to get stuck volumes removed.
Expand Down Expand Up @@ -132,6 +139,10 @@ describe('volume create flow', () => {
.should('be.visible')
.click();

// @TODO BSE: once BSE is fully rolled out, check for the notice (selected linode doesn't have
// blockstorage_encryption capability + user checked "Encrypt Volume" checkbox) instead of the absence of it
cy.findByText(CLIENT_LIBRARY_UPDATE_COPY).should('not.exist');
Copy link
Contributor

Choose a reason for hiding this comment

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

When manually testing, rebooting a linode without the bse capability didn't result in an upgraded qemu version/bse capability for me - not sure if that's expected in dev. I might have missed it, but wasn't seeing anywhere in tests where we were confirming that a user with a bse capability on the linode would no longer see the checkbox. Did we want to confirm this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Raised the reboot --> upgrade question in the project channel, not sure if that is fully in place yet.

Added test coverage for the case where the linode supports BSE


cy.findByText('Create Volume').click();
cy.wait('@createVolume');

Expand Down Expand Up @@ -162,7 +173,112 @@ describe('volume create flow', () => {
});

/*
* - Checks for Block Storage Encryption notices in the Create/Attach Volume drawer from the
* - Checks for Block Storage Encryption client library update notice on the Volume Create page.
*/
it('displays a warning notice on Volume Create page re: rebooting for client library updates under the appropriate conditions', () => {
// Conditions: Block Storage encryption feature flag is on; user has Block Storage Encryption capability; volume being created is encrypted and the
// selected Linode does not support Block Storage Encryption

// Mock feature flag -- @TODO BSE: Remove feature flag once BSE is fully rolled out
mockAppendFeatureFlags({
blockStorageEncryption: true,
}).as('getFeatureFlags');

// Mock account response
const mockAccount = accountFactory.build({
capabilities: ['Linodes', 'Block Storage Encryption'],
});

mockGetAccount(mockAccount).as('getAccount');
mockGetRegions(mockRegions).as('getRegions');

const linodeRequest = createLinodeRequestFactory.build({
label: randomLabel(),
root_pass: randomString(16),
region: mockRegions[0].id,
booted: false,
});

cy.defer(() => createTestLinode(linodeRequest), 'creating Linode').then(
(linode: Linode) => {
cy.visitWithLogin('/volumes/create');
cy.wait(['@getFeatureFlags', '@getAccount']);

// Select a linode without the BSE capability
cy.findByLabelText('Linode')
.should('be.visible')
.click()
.type(linode.label);

ui.autocompletePopper
.findByTitle(linode.label)
.should('be.visible')
.click();

// Check the "Encrypt Volume" checkbox
cy.get('[data-qa-checked]').should('be.visible').click();
// });

// Ensure warning notice is displayed
cy.findByText(CLIENT_LIBRARY_UPDATE_COPY).should('be.visible');
}
);
});

/*
* - Checks for absence of Block Storage Encryption client library update notice on the Volume Create page
* when selected linode supports BSE
*/
it('does not display a warning notice on Volume Create page re: rebooting for client library updates when selected linode supports BSE', () => {
// Conditions: Block Storage encryption feature flag is on; user has Block Storage Encryption capability; volume being created is encrypted and the
// selected Linode supports Block Storage Encryption

// Mock feature flag -- @TODO BSE: Remove feature flag once BSE is fully rolled out
mockAppendFeatureFlags({
blockStorageEncryption: true,
}).as('getFeatureFlags');

// Mock account response
const mockAccount = accountFactory.build({
capabilities: ['Linodes', 'Block Storage Encryption'],
});

// Mock linode
const mockLinode = linodeFactory.build({
region: mockRegions[0].id,
id: 123456,
capabilities: ['blockstorage_encryption'],
});

mockGetAccount(mockAccount).as('getAccount');
mockGetRegions(mockRegions).as('getRegions');
mockGetLinodes([mockLinode]).as('getLinodes');
mockGetLinodeDetails(mockLinode.id, mockLinode);

cy.visitWithLogin(`/volumes/create`);
cy.wait(['@getAccount', '@getRegions', '@getLinodes']);

// Select a linode without the BSE capability
cy.findByLabelText('Linode')
.should('be.visible')
.click()
.type(mockLinode.label);

ui.autocompletePopper
.findByTitle(mockLinode.label)
.should('be.visible')
.click();

// Check the "Encrypt Volume" checkbox
cy.get('[data-qa-checked]').should('be.visible').click();
// });

// Ensure warning notice is not displayed
cy.findByText(CLIENT_LIBRARY_UPDATE_COPY).should('not.exist');
});

/*
* - Checks for Block Storage Encryption client library update notice in the Create/Attach Volume drawer from the
'Storage' details page of an existing Linode.
*/
it('displays a warning notice re: rebooting for client library updates under the appropriate conditions', () => {
Expand Down
110 changes: 71 additions & 39 deletions packages/manager/src/features/Volumes/VolumeCreate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Button } from 'src/components/Button/Button';
import { DocumentTitleSegment } from 'src/components/DocumentTitle';
import {
BLOCK_STORAGE_CHOOSE_REGION_COPY,
BLOCK_STORAGE_CLIENT_LIBRARY_UPDATE_REQUIRED_COPY,
BLOCK_STORAGE_ENCRYPTION_GENERAL_DESCRIPTION,
BLOCK_STORAGE_ENCRYPTION_OVERHEAD_CAVEAT,
BLOCK_STORAGE_ENCRYPTION_UNAVAILABLE_IN_REGION_COPY,
Expand All @@ -23,6 +24,7 @@ import { LandingHeader } from 'src/components/LandingHeader';
import { Notice } from 'src/components/Notice/Notice';
import { Paper } from 'src/components/Paper';
import { RegionSelect } from 'src/components/RegionSelect/RegionSelect';
import { Stack } from 'src/components/Stack';
import { TextField } from 'src/components/TextField';
import { TooltipIcon } from 'src/components/TooltipIcon';
import { Typography } from 'src/components/Typography';
Expand All @@ -35,6 +37,7 @@ import {
useAccountAgreements,
useMutateAccountAgreements,
} from 'src/queries/account/agreements';
import { useLinodeQuery } from 'src/queries/linodes/linodes';
import { useGrants, useProfile } from 'src/queries/profile/profile';
import { useRegionsQuery } from 'src/queries/regions/regions';
import {
Expand Down Expand Up @@ -240,6 +243,15 @@ export const VolumeCreate = () => {

const { config_id, linode_id } = values;

const { data: linode } = useLinodeQuery(
linode_id ?? -1,
isBlockStorageEncryptionFeatureEnabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Late to this PR, but noticed a minor issue that can be resolved in a later change:

To prevent an API call for linodes/instances/-1

Suggested change
isBlockStorageEncryptionFeatureEnabled
isBlockStorageEncryptionFeatureEnabled && linode_id !== null

);

const linodeSupportsBlockStorageEncryption = Boolean(
linode?.capabilities?.includes('blockstorage_encryption')
);

const linodeError = touched.linode_id ? errors.linode_id : undefined;

const { showGDPRCheckbox } = getGDPRDetails({
Expand Down Expand Up @@ -283,6 +295,11 @@ export const VolumeCreate = () => {
}
};

const shouldDisplayClientLibraryCopy =
isBlockStorageEncryptionFeatureEnabled &&
values.linode_id !== null &&
!linodeSupportsBlockStorageEncryption;

return (
<>
<DocumentTitleSegment segment="Create a Volume" />
Expand Down Expand Up @@ -363,47 +380,57 @@ export const VolumeCreate = () => {
)}
</Box>
<Box
alignItems="flex-end"
alignItems="baseline"
className={classes.linodeConfigSelectWrapper}
display="flex"
>
<Box
alignItems="flex-end"
className={classes.linodeSelect}
display="flex"
>
<LinodeSelect
optionsFilter={(linode: Linode) => {
const linodeRegion = linode.region;
const valuesRegion = values.region;

/** When values.region is empty, all Linodes with
* block storage support will be displayed, regardless
* of their region. However, if a region is selected,
* only Linodes from the chosen region with block storage
* support will be shown. */
return isNilOrEmpty(valuesRegion)
? regionsWithBlockStorage.includes(linodeRegion)
: regionsWithBlockStorage.includes(linodeRegion) &&
linodeRegion === valuesRegion;
}}
sx={{
[theme.breakpoints.down('sm')]: {
width: 320,
},
width: '400px',
}}
clearable
disabled={doesNotHavePermission}
errorText={linodeError}
onBlur={handleBlur}
onSelectionChange={handleLinodeChange}
value={values.linode_id}
/>
{renderSelectTooltip(
'If you select a Linode, the Volume will be automatically created in that Linode’s region and attached upon creation.'
)}
</Box>
<Stack>
<Box
alignItems="flex-end"
className={classes.linodeSelect}
display="flex"
>
<LinodeSelect
optionsFilter={(linode: Linode) => {
const linodeRegion = linode.region;
const valuesRegion = values.region;

/** When values.region is empty, all Linodes with
* block storage support will be displayed, regardless
* of their region. However, if a region is selected,
* only Linodes from the chosen region with block storage
* support will be shown. */
return isNilOrEmpty(valuesRegion)
? regionsWithBlockStorage.includes(linodeRegion)
: regionsWithBlockStorage.includes(linodeRegion) &&
linodeRegion === valuesRegion;
}}
sx={{
[theme.breakpoints.down('sm')]: {
width: 320,
},
width: '400px',
}}
clearable
disabled={doesNotHavePermission}
errorText={linodeError}
onBlur={handleBlur}
onSelectionChange={handleLinodeChange}
value={values.linode_id}
/>
{renderSelectTooltip(
'If you select a Linode, the Volume will be automatically created in that Linode’s region and attached upon creation.'
)}
</Box>
{shouldDisplayClientLibraryCopy &&
values.encryption === 'enabled' && (
<Notice spacingBottom={0} spacingTop={16} variant="warning">
<Typography maxWidth="416px">
{BLOCK_STORAGE_CLIENT_LIBRARY_UPDATE_REQUIRED_COPY}
</Typography>
</Notice>
)}
</Stack>
<ConfigSelect
disabled={doesNotHavePermission || config_id === null}
error={touched.config_id ? errors.config_id : undefined}
Expand Down Expand Up @@ -473,6 +500,12 @@ export const VolumeCreate = () => {
</Paper>
<Box display="flex" justifyContent="flex-end">
<Button
disabled={
disabled ||
(isBlockStorageEncryptionFeatureEnabled &&
!linodeSupportsBlockStorageEncryption &&
values.encryption === 'enabled')
}
tooltipText={
!isLoading && isInvalidPrice
? PRICES_RELOAD_ERROR_NOTICE_TEXT
Expand All @@ -481,7 +514,6 @@ export const VolumeCreate = () => {
buttonType="primary"
className={classes.button}
data-qa-deploy-linode
disabled={disabled}
loading={isSubmitting}
style={{ marginLeft: 12 }}
type="submit"
Expand Down
Loading