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-8751& M3-8610] - Image Service Gen 2 final GA tweaks #11115

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Image Service Gen 2 final GA tweaks ([#11115](https://github.com/linode/manager/pull/11115))
1 change: 1 addition & 0 deletions packages/manager/src/dev-tools/FeatureFlagTool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const options: { flag: keyof Flags; label: string }[] = [
{ flag: 'disableLargestGbPlans', label: 'Disable Largest GB Plans' },
{ flag: 'gecko2', label: 'Gecko' },
{ flag: 'imageServiceGen2', label: 'Image Service Gen2' },
{ flag: 'imageServiceGen2Ga', label: 'Image Service Gen2 GA' },
{ flag: 'linodeDiskEncryption', label: 'Linode Disk Encryption (LDE)' },
{ flag: 'objMultiCluster', label: 'OBJ Multi-Cluster' },
{ flag: 'objectStorageGen2', label: 'OBJ Gen2' },
Expand Down
1 change: 1 addition & 0 deletions packages/manager/src/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export interface Flags {
gecko2: GeckoFeatureFlag;
gpuv2: gpuV2;
imageServiceGen2: boolean;
imageServiceGen2Ga: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this probably could've the same flag as a JSON flag

ipv6Sharing: boolean;
linodeDiskEncryption: boolean;
mainContentBanner: MainContentBanner;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,40 @@ describe('CreateImageTab', () => {
);
});

it('should render a notice if the user selects a Linode in a region that does not support image storage and Image Service Gen 2 GA is enabled', async () => {
const region = regionFactory.build({ capabilities: [] });
const linode = linodeFactory.build({ region: region.id });

server.use(
http.get('*/v4/linode/instances', () => {
return HttpResponse.json(makeResourcePage([linode]));
}),
http.get('*/v4/linode/instances/:id', () => {
return HttpResponse.json(linode);
}),
http.get('*/v4/regions', () => {
return HttpResponse.json(makeResourcePage([region]));
})
);

const { findByText, getByLabelText } = renderWithTheme(<CreateImageTab />, {
flags: { imageServiceGen2: true, imageServiceGen2Ga: true },
});

const linodeSelect = getByLabelText('Linode');

await userEvent.click(linodeSelect);

const linodeOption = await findByText(linode.label);

await userEvent.click(linodeOption);

await findByText(
'This Linode’s region doesn’t support local image storage.',
{ exact: false }
);
});

it('should render an encryption notice if disk encryption is enabled and the Linode is not in a distributed compute region', async () => {
const region = regionFactory.build({ site_type: 'core' });
const linode = linodeFactory.build({ region: region.id });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { useIsDiskEncryptionFeatureEnabled } from 'src/components/Encryption/uti
import { Link } from 'src/components/Link';
import { Notice } from 'src/components/Notice/Notice';
import { Paper } from 'src/components/Paper';
import { getIsDistributedRegion } from 'src/components/RegionSelect/RegionSelect.utils';
import { Stack } from 'src/components/Stack';
import { TagsInput } from 'src/components/TagsInput/TagsInput';
import { TextField } from 'src/components/TextField';
Expand Down Expand Up @@ -140,11 +139,20 @@ export const CreateImageTab = () => {

const isRawDisk = selectedDisk?.filesystem === 'raw';

const { data: regionsData } = useRegionsQuery();
const { data: regions } = useRegionsQuery();

const linodeIsInDistributedRegion = getIsDistributedRegion(
regionsData ?? [],
selectedLinode?.region ?? ''
const selectedLinodeRegion = regions?.find(
(r) => r.id === selectedLinode?.region
);

const linodeIsInDistributedRegion =
selectedLinodeRegion?.site_type === 'distributed';

/**
* The 'Object Storage' capability indicates a region can store images
*/
const linodeRegionSupportsImageStorage = selectedLinodeRegion?.capabilities.includes(
'Object Storage'
);

/*
Expand Down Expand Up @@ -220,7 +228,24 @@ export const CreateImageTab = () => {
required
value={selectedLinodeId}
/>
{linodeIsInDistributedRegion && (
{selectedLinode &&
!linodeRegionSupportsImageStorage &&
flags.imageServiceGen2 &&
flags.imageServiceGen2Ga && (
<Notice variant="warning">
This Linode’s region doesn’t support local image storage. This
image will be stored in the core compute region that’s{' '}
<Link to="https://techdocs.akamai.com/cloud-computing/docs/images#regions-and-captured-custom-images">
geographically closest
</Link>
. After it’s stored, you can replicate it to other{' '}
abailly-akamai marked this conversation as resolved.
Show resolved Hide resolved
<Link to="https://www.linode.com/global-infrastructure/">
core compute regions
</Link>
.
</Notice>
)}
{linodeIsInDistributedRegion && !flags.imageServiceGen2Ga && (
<Notice variant="warning">
This Linode is in a distributed compute region. These regions
can't store images. The image is stored in the core compute
Expand Down
100 changes: 74 additions & 26 deletions packages/manager/src/features/Images/ImagesLanding/ImageRow.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import userEvent from '@testing-library/user-event';
import * as React from 'react';
import React from 'react';

import { imageFactory } from 'src/factories';
import {
Expand All @@ -15,16 +15,6 @@ import type { Handlers } from './ImagesActionMenu';
beforeAll(() => mockMatchMedia());

describe('Image Table Row', () => {
const image = imageFactory.build({
capabilities: ['cloud-init', 'distributed-sites'],
regions: [
{ region: 'us-east', status: 'available' },
{ region: 'us-southeast', status: 'pending' },
],
size: 300,
total_size: 600,
});

const handlers: Handlers = {
onCancelFailed: vi.fn(),
onDelete: vi.fn(),
Expand All @@ -35,36 +25,94 @@ describe('Image Table Row', () => {
onRetry: vi.fn(),
};

it('should render an image row', async () => {
const { getAllByText, getByLabelText, getByText } = renderWithTheme(
it('should render an image row with Image Service Gen2 enabled', async () => {
const image = imageFactory.build({
capabilities: ['cloud-init', 'distributed-sites'],
regions: [
{ region: 'us-east', status: 'available' },
{ region: 'us-southeast', status: 'pending' },
],
size: 300,
total_size: 600,
});

const { getByLabelText, getByText } = renderWithTheme(
wrapWithTableBody(
<ImageRow handlers={handlers} image={image} multiRegionsEnabled />
)
);

// Check to see if the row rendered some data

expect(getByText(image.label)).toBeVisible();
expect(getByText(image.id)).toBeVisible();
expect(getByText('Ready')).toBeVisible();
expect(getByText('Cloud-init, Distributed')).toBeVisible();
expect(getByText('2 Regions')).toBeVisible();
expect(getByText('0.29 GB')).toBeVisible(); // 300 / 1024 = 0.292
expect(getByText('0.59 GB')).toBeVisible(); // 600 / 1024 = 0.585

getByText(image.label);
getAllByText('Ready');
getAllByText('Cloud-init, Distributed');
getAllByText(image.id);
expect(getByText('0.29 GB')).toBeVisible(); // Size is converted from MB to GB - 300 / 1024 = 0.292
expect(getByText('0.59 GB')).toBeVisible(); // Size is converted from MB to GB - 600 / 1024 = 0.585

// Open action menu
const actionMenu = getByLabelText(`Action menu for Image ${image.label}`);
await userEvent.click(actionMenu);

getByText('Edit');
getByText('Manage Replicas');
getByText('Deploy to New Linode');
getByText('Rebuild an Existing Linode');
getByText('Delete');
expect(getByText('Edit')).toBeVisible();
expect(getByText('Manage Replicas')).toBeVisible();
expect(getByText('Deploy to New Linode')).toBeVisible();
expect(getByText('Rebuild an Existing Linode')).toBeVisible();
expect(getByText('Delete')).toBeVisible();
});

it('should show a cloud-init icon with a tooltip when Image Service Gen 2 GA is enabled and the image supports cloud-init', () => {
const image = imageFactory.build({
capabilities: ['cloud-init'],
regions: [{ region: 'us-east', status: 'available' }],
});

const { getByLabelText } = renderWithTheme(
wrapWithTableBody(
<ImageRow handlers={handlers} image={image} multiRegionsEnabled />,
{ flags: { imageServiceGen2: true, imageServiceGen2Ga: true } }
)
);

expect(
getByLabelText('This image is compatible with cloud-init.')
).toBeVisible();
});

it('does not show the compatibility column when Image Service Gen2 GA is enabled', () => {
const image = imageFactory.build({
capabilities: ['cloud-init', 'distributed-sites'],
});

const { queryByText } = renderWithTheme(
wrapWithTableBody(
<ImageRow handlers={handlers} image={image} multiRegionsEnabled />,
{ flags: { imageServiceGen2: true, imageServiceGen2Ga: true } }
)
);

expect(queryByText('Cloud-init, Distributed')).not.toBeInTheDocument();
});

it('should show N/A if multiRegionsEnabled is true, but the Image does not have any regions', () => {
const image = imageFactory.build({ regions: [] });

const { getByText } = renderWithTheme(
wrapWithTableBody(
<ImageRow handlers={handlers} image={image} multiRegionsEnabled />,
{ flags: { imageServiceGen2: true } }
)
);

expect(getByText('N/A')).toBeVisible();
});

it('calls handlers when performing actions', async () => {
const image = imageFactory.build({
regions: [{ region: 'us-east', status: 'available' }],
});

const { getByLabelText, getByText } = renderWithTheme(
wrapWithTableBody(
<ImageRow handlers={handlers} image={image} multiRegionsEnabled />
Expand Down
51 changes: 40 additions & 11 deletions packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import * as React from 'react';
import React from 'react';

import CloudInitIcon from 'src/assets/icons/cloud-init.svg';
import { Hidden } from 'src/components/Hidden';
import { LinkButton } from 'src/components/LinkButton';
import { Stack } from 'src/components/Stack';
import { TableCell } from 'src/components/TableCell';
import { TableRow } from 'src/components/TableRow';
import { Tooltip } from 'src/components/Tooltip';
import { Typography } from 'src/components/Typography';
import { useFlags } from 'src/hooks/useFlags';
import { useProfile } from 'src/queries/profile/profile';
import { capitalizeAllWords } from 'src/utilities/capitalize';
import { formatDate } from 'src/utilities/formatDate';
Expand Down Expand Up @@ -44,6 +48,7 @@ export const ImageRow = (props: Props) => {
} = image;

const { data: profile } = useProfile();
const flags = useFlags();

const isFailed = status === 'pending_upload' && event?.status === 'failed';

Expand Down Expand Up @@ -92,23 +97,47 @@ export const ImageRow = (props: Props) => {

return (
<TableRow data-qa-image-cell={id} key={id}>
<TableCell data-qa-image-label>{label}</TableCell>
<TableCell data-qa-image-label noWrap>
{capabilities.includes('cloud-init') &&
flags.imageServiceGen2 &&
flags.imageServiceGen2Ga ? (
<Stack
alignItems="center"
direction="row"
gap={1}
justifyContent="space-between"
>
{label}
<Tooltip title="This image is compatible with cloud-init.">
<div style={{ display: 'flex' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a <Stack /> or <Box /> rather than hard coded styles?

Copy link
Member Author

@bnussman-akamai bnussman-akamai Oct 17, 2024

Choose a reason for hiding this comment

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

We can't because we don't forward ref on our Box or Stack, which I believe is needed for the tooltip to work. I could throw a forward ref but I'm hoping React 19 will make that not necessary πŸ˜…

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yeah... annoying but makes sense πŸ‘

<CloudInitIcon />
</div>
</Tooltip>
</Stack>
) : (
label
)}
</TableCell>
<Hidden smDown>
<TableCell>{getStatusForImage(status)}</TableCell>
</Hidden>
{multiRegionsEnabled && (
<>
<Hidden smDown>
<TableCell>
<Hidden smDown>
<TableCell>
{regions.length > 0 ? (
<LinkButton onClick={() => handlers.onManageRegions?.(image)}>
{pluralize('Region', 'Regions', regions.length)}
</LinkButton>
</TableCell>
</Hidden>
<Hidden smDown>
<TableCell>{compatibilitiesList}</TableCell>
</Hidden>
</>
) : (
'N/A'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be the default copy?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you explain what you mean?

For context: This N/A here isn't likely to show to customers. It exists just in case a user has both legacy images (0 regions) and "Gen2" images (with regions).

Screenshot 2024-10-21 at 10 50 15β€―AM

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, that explains it well. Was just thinking N/A may not be very clear to the user in case this happens. Not a big deal

)}
</TableCell>
</Hidden>
)}
{multiRegionsEnabled && !flags.imageServiceGen2Ga && (
<Hidden smDown>
<TableCell>{compatibilitiesList}</TableCell>
</Hidden>
)}
<TableCell data-qa-image-size>
{getSizeForImage(size, status, event?.status)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,14 +450,14 @@ export const ImagesLanding = () => {
<TableCell>Status</TableCell>
</Hidden>
{multiRegionsEnabled && (
<>
<Hidden smDown>
<TableCell>Replicated in</TableCell>
</Hidden>
<Hidden smDown>
<TableCell>Compatibility</TableCell>
</Hidden>
</>
<Hidden smDown>
<TableCell>Replicated in</TableCell>
</Hidden>
)}
{multiRegionsEnabled && !flags.imageServiceGen2Ga && (
<Hidden smDown>
<TableCell>Compatibility</TableCell>
</Hidden>
)}
<TableSortCell
active={manualImagesOrderBy === 'size'}
Expand Down