From f016b0b226b36a822d9ca9933f7ea79aa9d0bb20 Mon Sep 17 00:00:00 2001 From: Charlie Drage Date: Thu, 25 Apr 2024 11:19:46 -0400 Subject: [PATCH] feat: add manifest / multi-arch selection support ### What does this PR do? * Manifests are now listed, inspected and propagated within the Build page * Simply select a manifest, select the arch and it will build ### Screenshot / video of UI ### What issues does this PR fix or reference? Closes https://github.com/containers/podman-desktop-extension-bootc/issues/324 ### How to test this PR? Follow the video above, or do the following tests: 1. Build a manifest within Podman Desktop (select two architectures, and build a bootc image). Must use the latest Podman Desktop 2. Select the manifest within the extension and see it build. Signed-off-by: Charlie Drage --- packages/backend/src/api-impl.ts | 28 ++- packages/backend/src/constants.ts | 6 +- packages/backend/src/container-utils.ts | 32 +++ packages/frontend/src/Build.spec.ts | 250 +++++++++++++++++++++++- packages/frontend/src/Build.svelte | 37 +++- 5 files changed, 336 insertions(+), 17 deletions(-) diff --git a/packages/backend/src/api-impl.ts b/packages/backend/src/api-impl.ts index 1deea010..383f8dea 100644 --- a/packages/backend/src/api-impl.ts +++ b/packages/backend/src/api-impl.ts @@ -100,12 +100,30 @@ export class BootcApiImpl implements BootcApi { async listBootcImages(): Promise { let images: ImageInfo[] = []; try { - const retrieveImages = await podmanDesktopApi.containerEngine.listImages(); - images = retrieveImages.filter(image => { - if (image.Labels) { - return image.Labels['bootc'] ?? image.Labels['containers.bootc']; + const retrievedImages = await podmanDesktopApi.containerEngine.listImages(); + const filteredImages: ImageInfo[] = []; + for (const image of retrievedImages) { + let includeImage = false; + + // The image must have RepoTags and Labels to be considered a bootc image + if (image.RepoTags && image.Labels) { + // Convert to boolean by checking the string is non-empty + includeImage = !!(image.Labels['bootc'] ?? image.Labels['containers.bootc']); + } else if (image?.isManifest) { + // Manifests **usually** do not have any labels. If this is the case, we must find the images associated to the + // manifest in order to determine if we are going to return the manifest or not. + const manifestImages = await containerUtils.getImagesFromManifest(image, retrievedImages); + // Checking if any associated image has a non-empty label + includeImage = manifestImages.some( + manifestImage => !!(manifestImage.Labels['bootc'] ?? manifestImage.Labels['containers.bootc']), + ); } - }); + + if (includeImage) { + filteredImages.push(image); + } + } + images = filteredImages; } catch (err) { await podmanDesktopApi.window.showErrorMessage(`Error listing images: ${err}`); console.error('Error listing images: ', err); diff --git a/packages/backend/src/constants.ts b/packages/backend/src/constants.ts index 51f35be3..e0576206 100644 --- a/packages/backend/src/constants.ts +++ b/packages/backend/src/constants.ts @@ -18,4 +18,8 @@ // Image related export const bootcImageBuilderContainerName = '-bootc-image-builder'; -export const bootcImageBuilderName = 'quay.io/centos-bootc/bootc-image-builder:latest-1713548482'; + +// CHANGE IN THE FUTURE... MUST BE THE ONE WITH MANIFEST IMPLEMENTATION +// export const bootcImageBuilderName = 'quay.io/centos-bootc/bootc-image-builder:latest-1712917745'; +// Below only works for arm64 M1 testing +export const bootcImageBuilderName = 'quay.io/bootc-extension/test-arm64-bib-image:latest'; diff --git a/packages/backend/src/container-utils.ts b/packages/backend/src/container-utils.ts index 5ae65def..c0499103 100644 --- a/packages/backend/src/container-utils.ts +++ b/packages/backend/src/container-utils.ts @@ -317,3 +317,35 @@ export async function removeContainerAndVolumes(engineId: string, container: str throw new Error('There was an error removing the container and volumes: ' + e); } } + +// Image retrieval for manifest +// Params: Pass in ImageInfo +// 1. Check if isManifest for ImageInfo is true +// 2. If true, perform a inspectManifest to get the manifest object +// 3. Go through the array of manifests and get the digest values's (sha256) +// 4. do listImages and filter out the images that have the same digest value +// 5. Return the imageInfo of all matching images (these are the images that are part of the manifest) +export async function getImagesFromManifest( + image: extensionApi.ImageInfo, + images: extensionApi.ImageInfo[], +): Promise { + if (!image.isManifest) { + throw new Error('Image is not a manifest'); + } + + // Get the manifest + const manifest = await inspectManifest(image.engineId, image.Id); + + if (manifest.manifests === undefined) { + return []; + } + + // Get the digest values, if there are no digests, make sure array is [] so we don't run into + // issues with the filter + const digestValues = manifest.manifests + .map(manifest => manifest.digest) + .filter(digest => digest !== undefined); + + // Filter out the images that have the same digest value + return images.filter(image => image.Digest && digestValues.includes(image.Digest)); +} diff --git a/packages/frontend/src/Build.spec.ts b/packages/frontend/src/Build.spec.ts index 22bb8a48..ad45f743 100644 --- a/packages/frontend/src/Build.spec.ts +++ b/packages/frontend/src/Build.spec.ts @@ -17,10 +17,10 @@ ***********************************************************************/ import { vi, test, expect } from 'vitest'; -import { screen, render } from '@testing-library/svelte'; +import { screen, render, waitFor } from '@testing-library/svelte'; import Build from './Build.svelte'; import type { BootcBuildInfo } from '/@shared/src/models/bootc'; -import type { ImageInfo, ImageInspectInfo } from '@podman-desktop/api'; +import type { ImageInfo, ImageInspectInfo, ManifestInspectInfo } from '@podman-desktop/api'; import { bootcClient } from './api/client'; const mockHistoryInfo: BootcBuildInfo[] = [ @@ -80,6 +80,79 @@ const mockBootcImages: ImageInfo[] = [ }, ]; +const mockImageInspect: ImageInspectInfo = { + Architecture: 'amd64', + engineId: '', + engineName: '', + Id: '', + RepoTags: [], + RepoDigests: [], + Parent: '', + Comment: '', + Created: '', + Container: '', + ContainerConfig: { + Hostname: '', + Domainname: '', + User: '', + AttachStdin: false, + AttachStdout: false, + AttachStderr: false, + ExposedPorts: {}, + Tty: false, + OpenStdin: false, + StdinOnce: false, + Env: [], + Cmd: [], + ArgsEscaped: false, + Image: '', + Volumes: {}, + WorkingDir: '', + Entrypoint: undefined, + OnBuild: undefined, + Labels: {}, + }, + DockerVersion: '', + Author: '', + Config: { + Hostname: '', + Domainname: '', + User: '', + AttachStdin: false, + AttachStdout: false, + AttachStderr: false, + ExposedPorts: {}, + Tty: false, + OpenStdin: false, + StdinOnce: false, + Env: [], + Cmd: [], + ArgsEscaped: false, + Image: '', + Volumes: {}, + WorkingDir: '', + Entrypoint: undefined, + OnBuild: [], + Labels: {}, + }, + Os: '', + Size: 0, + VirtualSize: 0, + GraphDriver: { + Name: '', + Data: { + DeviceId: '', + DeviceName: '', + DeviceSize: '', + }, + }, + RootFS: { + Type: '', + Layers: undefined, + BaseLayer: undefined, + }, +}; + vi.mock('./api/client', async () => { return { bootcClient: { @@ -88,6 +161,7 @@ vi.mock('./api/client', async () => { listHistoryInfo: vi.fn(), listBootcImages: vi.fn(), inspectImage: vi.fn(), + inspectManifest: vi.fn(), }, rpcBrowser: { subscribe: () => { @@ -108,6 +182,7 @@ async function waitRender(customProperties?: object): Promise { } test('Render shows correct images and history', async () => { + vi.mocked(bootcClient.inspectImage).mockResolvedValue(mockImageInspect); vi.mocked(bootcClient.listHistoryInfo).mockResolvedValue(mockHistoryInfo); vi.mocked(bootcClient.listBootcImages).mockResolvedValue(mockBootcImages); vi.mocked(bootcClient.buildExists).mockResolvedValue(false); @@ -205,6 +280,9 @@ test('Check that overwriting an existing build works', async () => { vi.mocked(bootcClient.checkPrereqs).mockResolvedValue(undefined); vi.mocked(bootcClient.buildExists).mockResolvedValue(true); + // Mock the inspectImage to return 'amd64' as the architecture so it's selected / we can test the override function + vi.mocked(bootcClient.inspectImage).mockResolvedValue(mockImageInspect); + await waitRender({ imageName: 'image2', imageTag: 'latest' }); // Wait until children length is 2 meaning it's fully rendered / propagated the changes @@ -343,3 +421,171 @@ test('In the rare case that Architecture from inspectImage is blank, do not sele expect(x86_64).toBeDefined(); expect(x86_64.classList.contains('opacity-50')); }); + +test('Do not show an image if it has no repotags and has isManifest as false', async () => { + const mockedImages: ImageInfo[] = [ + { + Id: 'image1', + RepoTags: [], + Labels: { + bootc: 'true', + }, + engineId: 'engine1', + engineName: 'engine1', + ParentId: 'parent1', + Created: 0, + VirtualSize: 0, + Size: 0, + Containers: 0, + SharedSize: 0, + Digest: 'sha256:image1', + isManifest: false, + }, + ]; + + vi.mocked(bootcClient.listHistoryInfo).mockResolvedValue(mockHistoryInfo); + vi.mocked(bootcClient.listBootcImages).mockResolvedValue(mockedImages); + vi.mocked(bootcClient.buildExists).mockResolvedValue(false); + vi.mocked(bootcClient.checkPrereqs).mockResolvedValue(undefined); + await waitRender(); + + // Wait until children length is 1 + while (screen.getByLabelText('image-select')?.children.length !== 1) { + await new Promise(resolve => setTimeout(resolve, 100)); + } + + const select = screen.getByLabelText('image-select'); + expect(select).toBeDefined(); + expect(select.children.length).toEqual(1); + expect(select.children[0].textContent).toEqual('Select an image'); + + // Find the

that CONTAINS "No bootable container compatible images found." + const noImages = screen.getByText(/No bootable container compatible images found./); + expect(noImages).toBeDefined(); +}); + +test('If inspectImage fails, do not select any architecture / make them available', 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).mockRejectedValue('Error'); + + 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')); + + // Expect Architecture must be selected to be shown + const validation = screen.getByLabelText('validation'); + expect(validation).toBeDefined(); + expect(validation.textContent).toEqual('Architecture must be selected'); +}); + +test('Show the image if isManifest: true and Labels is empty', async () => { + + + // spy on inspectManifest + const spyOnInspectManifest = vi.spyOn(bootcClient, 'inspectManifest'); + + const mockedImages: ImageInfo[] = [ + { + Id: 'image1', + RepoTags: ['testmanifest1:latest'], + Labels: {}, + engineId: 'engine1', + engineName: 'engine1', + ParentId: 'parent1', + Created: 0, + VirtualSize: 0, + Size: 0, + Containers: 0, + SharedSize: 0, + Digest: 'sha256:image1', + isManifest: true, + }, + // "children" images of a manifest that has the 'bootc' and 'containers.bootc' labels + // they have no repo tags, but have the labels / architecture + { + Id: 'image2', + RepoTags: [], + Labels: { + bootc: 'true', + }, + engineId: 'engine1', + engineName: 'engine1', + ParentId: 'parent1', + Created: 0, + VirtualSize: 0, + Size: 0, + Containers: 0, + SharedSize: 0, + Digest: 'sha256:image2', + isManifest: false, + }, + ]; + + // FIX BELOW TESTS + const mockedManifestInspect: ManifestInspectInfo = { + engineId: 'podman1', + engineName: 'podman', + manifests: [ + { + digest: 'sha256:image2', + mediaType: 'mediaType', + platform: { + architecture: 'amd64', + features: [], + os: 'os', + variant: 'variant', + }, + size: 100, + urls: ['url1', 'url2'], + }, + ], + mediaType: 'mediaType', + schemaVersion: 1, + }; + + + vi.mocked(bootcClient.inspectManifest).mockResolvedValue(mockedManifestInspect); + vi.mocked(bootcClient.listHistoryInfo).mockResolvedValue(mockHistoryInfo); + vi.mocked(bootcClient.listBootcImages).mockResolvedValue(mockedImages); + vi.mocked(bootcClient.buildExists).mockResolvedValue(false); + vi.mocked(bootcClient.checkPrereqs).mockResolvedValue(undefined); + await waitRender(); + + // Wait until children length is 2 + while (screen.getByLabelText('image-select')?.children.length !== 2) { + await new Promise(resolve => setTimeout(resolve, 100)); + } + + const select = screen.getByLabelText('image-select'); + expect(select).toBeDefined(); + expect(select.children.length).toEqual(2); + expect(select.children[1].textContent).toEqual('testmanifest1:latest'); + + // Expect input amd64 to be selected + const x86_64 = screen.getByLabelText('amd64-select'); + expect(x86_64).toBeDefined(); + // Expect it to be "selected" + expect(x86_64.classList.contains('bg-purple-500')); + + // arm64 should be disabled + const arm64 = screen.getByLabelText('arm64-select'); + expect(arm64).toBeDefined(); + expect(arm64.classList.contains('opacity-50')); + + +}); \ No newline at end of file diff --git a/packages/frontend/src/Build.svelte b/packages/frontend/src/Build.svelte index 3066fda2..3cab3737 100644 --- a/packages/frontend/src/Build.svelte +++ b/packages/frontend/src/Build.svelte @@ -119,7 +119,7 @@ async function validate() { } if (!buildArch) { - errorFormValidation = 'At least one architecture must be selected'; + errorFormValidation = 'Architecture must be selected'; existingBuild = false; return; } @@ -229,13 +229,30 @@ onMount(async () => { async function updateAvailableArchitectures(selectedImage: string) { const image = findImage(selectedImage); if (image) { - try { - const imageInspect = await bootcClient.inspectImage(image); - if (imageInspect?.Architecture) { - availableArchitectures = [imageInspect.Architecture]; + // If it is a manifest, we can just inspectManifest and get the architecture(s) from there + if (image?.isManifest) { + try { + const manifest = await bootcClient.inspectManifest(image); + // Go through each manifest.manifests and get the architecture from manifest.platform.architecture + availableArchitectures = manifest.manifests.map(manifest => manifest.platform.architecture); + } catch (error) { + console.error('Error inspecting manifest:', error); + } + } else { + try { + const imageInspect = await bootcClient.inspectImage(image); + // Architecture is a mandatory field in the image inspect and should **always** be there. + if (imageInspect?.Architecture) { + availableArchitectures = [imageInspect.Architecture]; + } else { + // If for SOME reason Architecture is missing (testing purposes, weird output, etc.) + // we will set availableArchitectures to an empty array to disable the architecture selection. + availableArchitectures = []; + console.error('Architecture not found in image inspect:', imageInspect); + } + } catch (error) { + console.error('Error inspecting image:', error); } - } catch (error) { - console.error('Error inspecting image:', error); } } } @@ -254,11 +271,13 @@ $: if (selectedImage) { } $: if (availableArchitectures) { - // If there is only ONE available architecture, select it automatically. if (availableArchitectures.length === 1) { + // If there is only ONE available architecture, select it automatically. buildArch = availableArchitectures[0]; - // If none, disable buildArch selection regardless of what was selected before in history, etc. + } else if (availableArchitectures.length > 1 && buildArch && !availableArchitectures.includes(buildArch)) { + buildArch = undefined; } else if (availableArchitectures.length === 0) { + // If none, disable buildArch selection regardless of what was selected before in history, etc. buildArch = undefined; } }