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

feature: automatically select image based on image architecture inspect #351

Merged
merged 1 commit into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
118 changes: 117 additions & 1 deletion packages/frontend/src/Build.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { vi, test, expect } from 'vitest';
import { screen, render } from '@testing-library/svelte';
import Build from './Build.svelte';
import type { BootcBuildInfo } from '/@shared/src/models/bootc';
import type { ImageInfo } from '@podman-desktop/api';
import type { ImageInfo, ImageInspectInfo } from '@podman-desktop/api';
import { bootcClient } from './api/client';

const mockHistoryInfo: BootcBuildInfo[] = [
Expand Down Expand Up @@ -85,6 +85,7 @@ vi.mock('./api/client', async () => {
buildExists: vi.fn(),
listHistoryInfo: vi.fn(),
listBootcImages: vi.fn(),
inspectImage: vi.fn(),
},
rpcBrowser: {
subscribe: () => {
Expand Down Expand Up @@ -225,3 +226,118 @@ test('Check that overwriting an existing build works', async () => {
const validation2 = screen.queryByLabelText('validation');
expect(validation2).toBeNull();
});

const fakedImageInspect: ImageInspectInfo = {
Architecture: '',
Author: '',
Comment: '',
Config: {
ArgsEscaped: false,
AttachStderr: false,
AttachStdin: false,
AttachStdout: false,
Cmd: [],
Domainname: '',
Entrypoint: [],
Env: [],
ExposedPorts: {},
Hostname: '',
Image: '',
Labels: {},
OnBuild: [],
OpenStdin: false,
StdinOnce: false,
Tty: false,
User: '',
Volumes: {},
WorkingDir: '',
},
Container: '',
ContainerConfig: {
ArgsEscaped: false,
AttachStderr: false,
AttachStdin: false,
AttachStdout: false,
Cmd: [],
Domainname: '',
Env: [],
ExposedPorts: {},
Hostname: '',
Image: '',
Labels: {},
OpenStdin: false,
StdinOnce: false,
Tty: false,
User: '',
Volumes: {},
WorkingDir: '',
},
Created: '',
DockerVersion: '',
GraphDriver: { Data: { DeviceId: '', DeviceName: '', DeviceSize: '' }, Name: '' },
Id: '',
Os: '',
Parent: '',
RepoDigests: [],
RepoTags: [],
RootFS: {
Type: '',
},
Size: 0,
VirtualSize: 0,
engineId: 'engineid',
engineName: 'engineName',
};

test('Test that arm64 is disabled in form if inspectImage returns no arm64', async () => {
vi.mocked(bootcClient.listHistoryInfo).mockResolvedValue(mockHistoryInfo);
vi.mocked(bootcClient.listBootcImages).mockResolvedValue(mockBootcImages);
vi.mocked(bootcClient.checkPrereqs).mockResolvedValue(undefined);
vi.mocked(bootcClient.buildExists).mockResolvedValue(false);
vi.mocked(bootcClient.inspectImage).mockResolvedValue(fakedImageInspect);

await waitRender({ imageName: 'image2', imageTag: 'latest' });

// Wait until children length is 2 meaning it's fully rendered / propagated the changes
while (screen.getByLabelText('image-select')?.children.length !== 2) {
await new Promise(resolve => setTimeout(resolve, 100));
}

const arm64 = screen.getByLabelText('arm64-select');
expect(arm64).toBeDefined();
// Expect it to be "disabled" (opacity-50)
expect(arm64.classList.contains('opacity-50'));

// Expect input amd64 to be selected (it would have bg-purple-500 class)
const x86_64 = screen.getByLabelText('amd64-select');
expect(x86_64).toBeDefined();
// Expect it to be "selected"
expect(x86_64.classList.contains('bg-purple-500'));
});

test('In the rare case that Architecture from inspectImage is blank, do not select either', async () => {
const fakeImageNoArchitecture = fakedImageInspect;
fakeImageNoArchitecture.Architecture = '';

vi.mocked(bootcClient.listHistoryInfo).mockResolvedValue(mockHistoryInfo);
vi.mocked(bootcClient.listBootcImages).mockResolvedValue(mockBootcImages);
vi.mocked(bootcClient.checkPrereqs).mockResolvedValue(undefined);
vi.mocked(bootcClient.buildExists).mockResolvedValue(false);
vi.mocked(bootcClient.inspectImage).mockResolvedValue(fakeImageNoArchitecture);

await waitRender({ imageName: 'image2', imageTag: 'latest' });

// Wait until children length is 2 meaning it's fully rendered / propagated the changes
while (screen.getByLabelText('image-select')?.children.length !== 2) {
await new Promise(resolve => setTimeout(resolve, 100));
}

const arm64 = screen.getByLabelText('arm64-select');
expect(arm64).toBeDefined();
// Expect it to be "disabled" (opacity-50)
expect(arm64.classList.contains('opacity-50'));

const x86_64 = screen.getByLabelText('amd64-select');
expect(x86_64).toBeDefined();
expect(x86_64.classList.contains('opacity-50'));
});
91 changes: 81 additions & 10 deletions packages/frontend/src/Build.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ export let imageTag: string | undefined = undefined;
let selectedImage: string | undefined;
let existingBuild: boolean = false;

// Architecture variable
// This is an array as we will be support manifests
// the array contains the architectures (ex. arm64, amd64, etc.) of the image
// we will ONLY enable the architectures that are available in the image / manifest within the form.
// this is to prevent the user from selecting an architecture that is not available in the image.
// Will either be 'arm64' or 'amd64' as that is all we support for now.
let availableArchitectures: string[] = [];

// Build options
let buildFolder: string;
let buildType: BuildType[];
Expand All @@ -39,9 +47,8 @@ function findImage(repoTag: string): ImageInfo | undefined {
}
// Function that will use listHistoryInfo, if there is anything in the list, pick the first one in the list (as it's the most recent)
// and fill buildFolder, buildType and buildArch with the values from the selected image.
async function fillBuildOptions() {
async function fillBuildOptions(historyInfo: BootcBuildInfo[] = []) {
// Fill the build options from history
const historyInfo = await bootcClient.listHistoryInfo();
if (historyInfo.length > 0) {
const latestBuild = historyInfo[0];
buildFolder = latestBuild.folder;
Expand All @@ -66,6 +73,24 @@ async function fillBuildOptions() {
}
}

async function fillArchitectures(historyInfo: BootcBuildInfo[]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a lot of overlap with fillBuildOptions, which even sets buildArch in one case. It's fine as-is, but I think they should probably be merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've opened up #358 that we could work on post-summit.

Within #356 I did add some changes for the buildArch section which would help improve it.

// If there is only one available architecture, select it automatically.
if (availableArchitectures.length === 1) {
buildArch = availableArchitectures[0];
return;
}

// If none are propagated yet, go through the history, update available architectures and select the latest one
if (selectedImage && historyInfo.length > 0) {
const latestArch = historyInfo[0].arch;
await updateAvailableArchitectures(selectedImage);
// Only set buildArch if it's available in availableArchitectures
if (latestArch && availableArchitectures.includes(latestArch)) {
buildArch = latestArch;
}
}
}

async function validate() {
let prereqs = await bootcClient.checkPrereqs();
if (prereqs) {
Expand Down Expand Up @@ -192,15 +217,50 @@ onMount(async () => {
bootcAvailableImages = imageInfos.filter(image => image.RepoTags && image.RepoTags.length > 0);

// Fills the build options with the last options
await fillBuildOptions();
const historyInfo = await bootcClient.listHistoryInfo();
await fillBuildOptions(historyInfo);
await fillArchitectures(historyInfo);

validate();
});

// validate every time a selection changes in the form
/// Find the selected image and update availableArchitectures with the architecture of the image
async function updateAvailableArchitectures(selectedImage: string) {
const image = findImage(selectedImage);
if (image) {
try {
const imageInspect = await bootcClient.inspectImage(image);
if (imageInspect?.Architecture) {
availableArchitectures = [imageInspect.Architecture];
}
} catch (error) {
console.error('Error inspecting image:', error);
}
}
}

// validate every time a selection changes in the form or available architectures
$: if (selectedImage || buildFolder || buildType || buildArch || overwrite) {
validate();
}

// Each time an image is selected, we need to update the available architectures
// to do that, inspect the image and get the architecture.
$: if (selectedImage) {
(async () => {
await updateAvailableArchitectures(selectedImage);
})();
}

$: if (availableArchitectures) {
// If there is only ONE available architecture, select it automatically.
if (availableArchitectures.length === 1) {
buildArch = availableArchitectures[0];
// If none, disable buildArch selection regardless of what was selected before in history, etc.
} else if (availableArchitectures.length === 0) {
buildArch = undefined;
}
}
</script>

<FormPage title="Build Disk Image" inProgress="{buildInProgress}" showBreadcrumb="{true}">
Expand Down Expand Up @@ -385,10 +445,15 @@ $: if (selectedImage || buildFolder || buildType || buildArch || overwrite) {
name="arch"
value="arm64"
class="sr-only peer"
aria-label="arm64-select" />
aria-label="arm64-select"
disabled="{!availableArchitectures.includes('arm64')}" />
<label
for="arm64"
class="h-full flex items-center p-5 cursor-pointer rounded-lg bg-zinc-700 border border-transparent hover:border-violet-500 focus:outline-none peer-checked:ring-2 peer-checked:ring-violet-500 peer-checked:border-transparent">
class="h-full flex items-center p-5 cursor-pointer rounded-lg bg-zinc-700 border border-transparent focus:outline-none peer-checked:ring-2 peer-checked:ring-violet-500 peer-checked:border-transparent {availableArchitectures.includes(
'arm64',
)
? 'cursor-pointer hover:border-violet-500'
: 'ring-0 opacity-50'}">
<i class="fab fa-linux fa-2x"></i>
<br />
<span class="ml-2 text-sm">ARM® aarch64 systems</span>
Expand All @@ -402,19 +467,25 @@ $: if (selectedImage || buildFolder || buildType || buildArch || overwrite) {
name="arch"
value="amd64"
class="sr-only peer"
aria-label="amd64-select" />
aria-label="amd64-select"
disabled="{!availableArchitectures.includes('amd64')}" />
<label
for="amd64"
class="h-full flex items-center p-5 cursor-pointer rounded-lg bg-zinc-700 border border-transparent hover:border-violet-500 focus:outline-none peer-checked:ring-2 peer-checked:ring-violet-500 peer-checked:border-transparent">
class="h-full flex items-center p-5 cursor-pointer rounded-lg bg-zinc-700 border border-transparent focus:outline-none peer-checked:ring-2 peer-checked:ring-violet-500 peer-checked:border-transparent {availableArchitectures.includes(
'amd64',
)
? 'cursor-pointer hover:border-violet-500'
: 'ring-0 opacity-50'}">
<i class="fab fa-linux fa-2x"></i>
<br />
<span class="ml-2 text-sm">Intel and AMD x86_64 systems</span>
</label>
</li>
</ul>
<p class="text-gray-300 text-xs pt-1">
Note: Architecture being built must match the architecture of the selected image. For example, you must
have an ARM container image to build an ARM disk image.
Note: Disk image architecture must match the architecture of the original image. For example, you must
have an ARM container image to build an ARM disk image. You can only select the architecture that is
detectable within the image or manifest.
</p>
</div>
</div>
Expand Down