From b174a706c874cc49ec6140580cd18bf063918dd6 Mon Sep 17 00:00:00 2001 From: Tim deBoer Date: Thu, 2 May 2024 14:20:25 -0400 Subject: [PATCH 1/4] feat: support rhel bib Adds a new preference that allows you to switch between: - 'image' - the default, picks up builder from image label (or use centos if none is defined) - 'Centos' - always use Centos builder - 'RHEL' - always use RHEL builder getBuilder() is added to determine which builder to use, and createBuilderImageOptions() has an optional parameter to specify the builder to use. Signed-off-by: Tim deBoer --- packages/backend/package.json | 10 ++ packages/backend/src/build-disk-image.spec.ts | 107 +++++++++++++++++- packages/backend/src/build-disk-image.ts | 40 ++++++- packages/backend/src/constants.ts | 6 +- packages/backend/src/container-utils.spec.ts | 17 +++ packages/backend/src/container-utils.ts | 14 +++ 6 files changed, 181 insertions(+), 13 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index ba479fd0..199a95b2 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -20,6 +20,16 @@ "default": 30, "maximum": 120, "description": "Build timeout (in minutes)" + }, + "bootc.builder": { + "type": "string", + "default": "image", + "enum": [ + "image", + "Centos", + "RHEL" + ], + "description": "Builder image" } } }, diff --git a/packages/backend/src/build-disk-image.spec.ts b/packages/backend/src/build-disk-image.spec.ts index 0b0f51f6..d24ba2ef 100644 --- a/packages/backend/src/build-disk-image.spec.ts +++ b/packages/backend/src/build-disk-image.spec.ts @@ -17,14 +17,22 @@ ***********************************************************************/ import { beforeEach, expect, test, vi } from 'vitest'; -import { buildExists, createBuilderImageOptions, getUnusedName } from './build-disk-image'; -import { bootcImageBuilderName } from './constants'; -import type { ContainerInfo } from '@podman-desktop/api'; +import { buildExists, createBuilderImageOptions, getBuilder, getUnusedName } from './build-disk-image'; +import { bootcImageBuilderCentos, bootcImageBuilderRHEL } from './constants'; +import type { ContainerInfo, Configuration } from '@podman-desktop/api'; import { containerEngine } from '@podman-desktop/api'; import type { BootcBuildInfo } from '/@shared/src/models/bootc'; import * as fs from 'node:fs'; import { resolve } from 'node:path'; +const configurationGetConfigurationMock = vi.fn(); + +const config: Configuration = { + get: configurationGetConfigurationMock, + has: () => true, + update: vi.fn(), +}; + vi.mock('@podman-desktop/api', async () => { return { env: { @@ -33,6 +41,9 @@ vi.mock('@podman-desktop/api', async () => { containerEngine: { listContainers: vi.fn().mockReturnValue([]), }, + configuration: { + getConfiguration: () => config, + }, }; }); @@ -53,7 +64,7 @@ test('check image builder options', async () => { expect(options).toBeDefined(); expect(options.name).toEqual(name); - expect(options.Image).toEqual(bootcImageBuilderName); + expect(options.Image).toEqual(bootcImageBuilderCentos); expect(options.HostConfig).toBeDefined(); if (options.HostConfig?.Binds) { expect(options.HostConfig.Binds[0]).toEqual(build.folder + ':/output/'); @@ -84,7 +95,7 @@ test('check image builder with multiple types', async () => { expect(options).toBeDefined(); expect(options.name).toEqual(name); - expect(options.Image).toEqual(bootcImageBuilderName); + expect(options.Image).toEqual(bootcImageBuilderCentos); expect(options.HostConfig).toBeDefined(); if (options.HostConfig?.Binds) { expect(options.HostConfig.Binds[0]).toEqual(build.folder + ':/output/'); @@ -181,6 +192,18 @@ test('test if blank string is passed into filesystem, it is not included in the expect(options.Cmd).not.toContain('--rootfs'); }); +test('test specified builder is used', async () => { + const builder = 'foo-builder'; + const build = { + image: 'test-image', + type: ['vmdk'], + } as BootcBuildInfo; + const options = createBuilderImageOptions('my-image', build, builder); + + expect(options).toBeDefined(); + expect(options.Image).toEqual(builder); +}); + test('check we pick unused container name', async () => { const basename = 'test'; let name = await getUnusedName(basename); @@ -230,3 +253,77 @@ test('check build exists', async () => { exists = await buildExists(folder, ['iso', 'raw']); expect(exists).toEqual(false); }); + +test('check uses RHEL builder', async () => { + configurationGetConfigurationMock.mockReturnValue('RHEL'); + + const build = { + image: 'test-image', + type: ['ami'], + } as BootcBuildInfo; + const builder = await getBuilder(build); + + expect(builder).toBeDefined(); + expect(builder).toEqual(bootcImageBuilderRHEL); +}); + +test('check uses Centos builder', async () => { + configurationGetConfigurationMock.mockReturnValue('centos'); + + const build = { + image: 'test-image', + type: ['ami'], + } as BootcBuildInfo; + const builder = await getBuilder(build); + + expect(builder).toBeDefined(); + expect(builder).toEqual(bootcImageBuilderCentos); +}); + +test('check uses image preferred builder (RHEL)', async () => { + configurationGetConfigurationMock.mockReturnValue('image'); + + const listImagesMock = vi.fn(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (containerEngine as any).listImages = listImagesMock; + listImagesMock.mockResolvedValue([ + { + RepoTags: ['test-image:latest'], + Labels: { 'bootc.diskimage-builder': 'registry.redhat.io/rhel9/bootc-image-builder' }, + }, + ]); + + const build = { + image: 'test-image', + tag: 'latest', + type: ['iso'], + } as BootcBuildInfo; + const builder = await getBuilder(build); + + expect(builder).toBeDefined(); + expect(builder).toEqual(bootcImageBuilderRHEL); +}); + +test('check uses image preferred builder (Centos)', async () => { + configurationGetConfigurationMock.mockReturnValue('image'); + + const listImagesMock = vi.fn(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (containerEngine as any).listImages = listImagesMock; + listImagesMock.mockResolvedValue([ + { + RepoTags: ['test-image:latest'], + Labels: { 'bootc.diskimage-builder': 'quay.io/centos-bootc/bootc-image-builder' }, + }, + ]); + + const build = { + image: 'test-image', + tag: 'latest', + type: ['iso'], + } as BootcBuildInfo; + const builder = await getBuilder(build); + + expect(builder).toBeDefined(); + expect(builder).toEqual(bootcImageBuilderCentos); +}); diff --git a/packages/backend/src/build-disk-image.ts b/packages/backend/src/build-disk-image.ts index 6db4ceda..5b6ddddb 100644 --- a/packages/backend/src/build-disk-image.ts +++ b/packages/backend/src/build-disk-image.ts @@ -21,11 +21,11 @@ import * as extensionApi from '@podman-desktop/api'; import * as fs from 'node:fs'; import { resolve } from 'node:path'; import * as containerUtils from './container-utils'; -import { bootcImageBuilderContainerName, bootcImageBuilderName } from './constants'; +import { bootcImageBuilder, bootcImageBuilderCentos, bootcImageBuilderRHEL } from './constants'; import type { BootcBuildInfo, BuildType } from '/@shared/src/models/bootc'; import type { History } from './history'; import * as machineUtils from './machine-utils'; -import { telemetryLogger } from './extension'; +import { getConfigurationValue, telemetryLogger } from './extension'; export async function buildExists(folder: string, types: BuildType[]) { let exists = false; @@ -100,7 +100,7 @@ export async function buildDiskImage(build: BootcBuildInfo, history: History, ov .withProgress( { location: extensionApi.ProgressLocation.TASK_WIDGET, title: `Building disk image ${build.image}` }, async progress => { - const buildContainerName = build.image.split('/').pop() + bootcImageBuilderContainerName; + const buildContainerName = build.image.split('/').pop() + '-' + bootcImageBuilder; let successful: boolean = false; let logData: string = 'Build Image Log --------\n'; logData += 'ID: ' + build.id + '\n'; @@ -118,11 +118,15 @@ export async function buildDiskImage(build: BootcBuildInfo, history: History, ov fs.unlinkSync(logPath); } + // determine which bootc image builder to use based on the image + // being built and the current preferences + const builder = await getBuilder(build); + // Preliminary Step 0. Create the "bootc-image-builder" container // options that we will use to build the image. This will help with debugging // as well as making sure we delete the previous build, etc. const containerName = await getUnusedName(buildContainerName); - const buildImageContainer = createBuilderImageOptions(containerName, build); + const buildImageContainer = createBuilderImageOptions(containerName, build, builder); logData += JSON.stringify(buildImageContainer, undefined, 2); logData += '\n----------\n'; try { @@ -304,8 +308,32 @@ export async function getUnusedName(name: string): Promise { return unusedName; } +export async function getBuilder(build: BootcBuildInfo): Promise { + // check image for builder to use + const buildProp = await getConfigurationValue('builder'); + + if (buildProp === 'RHEL') { + // use to rhel if that's the preference + return bootcImageBuilderRHEL; + } else if (buildProp === 'image') { + // or use rhel if the preference comes from the image label + // AND we detect the rhel label + const image = `${build.image}:${build.tag}`; + const buildLabel = await containerUtils.getImageBuilderLabel(image); + if (buildLabel === 'registry.redhat.io/rhel9/bootc-image-builder') { + return bootcImageBuilderRHEL; + } + } + // otherwise, always use centos + return bootcImageBuilderCentos; +} + // Create builder options for the "bootc-image-builder" container -export function createBuilderImageOptions(name: string, build: BootcBuildInfo): ContainerCreateOptions { +export function createBuilderImageOptions( + name: string, + build: BootcBuildInfo, + builder?: string, +): ContainerCreateOptions { const cmd = [`${build.image}:${build.tag}`, '--output', '/output/', '--local']; build.type.forEach(t => cmd.push('--type', t)); @@ -323,7 +351,7 @@ export function createBuilderImageOptions(name: string, build: BootcBuildInfo): // Create the image options for the "bootc-image-builder" container const options: ContainerCreateOptions = { name: name, - Image: bootcImageBuilderName, + Image: builder ?? bootcImageBuilderCentos, Tty: true, HostConfig: { Privileged: true, diff --git a/packages/backend/src/constants.ts b/packages/backend/src/constants.ts index 00d5d815..1ed656d8 100644 --- a/packages/backend/src/constants.ts +++ b/packages/backend/src/constants.ts @@ -17,5 +17,7 @@ ***********************************************************************/ // Image related -export const bootcImageBuilderContainerName = '-bootc-image-builder'; -export const bootcImageBuilderName = 'quay.io/centos-bootc/bootc-image-builder:latest-1714633180'; +export const bootcImageBuilder = 'bootc-image-builder'; +export const bootcImageBuilderCentos = 'quay.io/centos-bootc/bootc-image-builder:latest-1714474808'; +export const bootcImageBuilderRHEL = 'registry.redhat.io/rhel9/bootc-image-builder:9.4'; +export const bootcImageBuilderLabel = 'bootc.diskimage-builder'; diff --git a/packages/backend/src/container-utils.spec.ts b/packages/backend/src/container-utils.spec.ts index 0dd8a7ab..9e81fad8 100644 --- a/packages/backend/src/container-utils.spec.ts +++ b/packages/backend/src/container-utils.spec.ts @@ -28,6 +28,7 @@ import { deleteOldImages, inspectImage, inspectManifest, + getImageBuilderLabel, } from './container-utils'; const mocks = vi.hoisted(() => ({ @@ -265,3 +266,19 @@ test('test running inspectManifest', async () => { expect(result.engineName).toBe('podman'); expect(result.manifests).toBeDefined(); }); + +test('test image builder label', async () => { + const listImagesMock = vi.fn(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (extensionApi.containerEngine as any).listImages = listImagesMock; + listImagesMock.mockResolvedValue([ + { RepoTags: ['test.io/name:1'] }, + { RepoTags: ['test.io/name:2'], Labels: { 'bootc.diskimage-builder': 'foo' } }, + { RepoTags: ['test.io/name:3'] }, + { RepoTags: ['test.io/name:4', 'keep-me'] }, + ]); + + // Test that it'll find the right image and label + const result = await getImageBuilderLabel('test.io/name:2'); + expect(result).toBe('foo'); +}); diff --git a/packages/backend/src/container-utils.ts b/packages/backend/src/container-utils.ts index f2dcdedd..5c62aa10 100644 --- a/packages/backend/src/container-utils.ts +++ b/packages/backend/src/container-utils.ts @@ -18,6 +18,7 @@ import type { ContainerCreateOptions } from '@podman-desktop/api'; import * as extensionApi from '@podman-desktop/api'; import { getConfigurationValue, telemetryLogger } from './extension'; +import { bootcImageBuilderLabel } from './constants'; // Get the running container engine export async function getContainerEngine(): Promise { @@ -347,3 +348,16 @@ export async function getImagesFromManifest( // Filter out the images that have the same digest value return images.filter(image => image.Digest && digestValues.includes(image.Digest)); } + +// Return the image builder label from the given image +export async function getImageBuilderLabel(imageId: string): Promise { + try { + const images = await extensionApi.containerEngine.listImages(); + + const foundImage = images.find(i => i.RepoTags?.find(tag => tag === imageId)); + return foundImage?.Labels[bootcImageBuilderLabel]; + } catch (err) { + console.error('Error getting image label: ', err); + } + return undefined; +} From 11a8cc2105dc75292841aab69a78e6e3b9bd3780 Mon Sep 17 00:00:00 2001 From: Tim deBoer Date: Mon, 6 May 2024 09:42:23 -0400 Subject: [PATCH 2/4] chore: remove image label detection for now Remove the label detection code for now: doesn't seem to be used by many containers and listImages is a costly call. Signed-off-by: Tim deBoer --- packages/backend/package.json | 3 +- packages/backend/src/build-disk-image.spec.ts | 48 ------------------- packages/backend/src/build-disk-image.ts | 21 +++----- packages/backend/src/constants.ts | 1 - packages/backend/src/container-utils.spec.ts | 17 ------- packages/backend/src/container-utils.ts | 14 ------ 6 files changed, 7 insertions(+), 97 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index 199a95b2..beacce8d 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -23,9 +23,8 @@ }, "bootc.builder": { "type": "string", - "default": "image", + "default": "Centos", "enum": [ - "image", "Centos", "RHEL" ], diff --git a/packages/backend/src/build-disk-image.spec.ts b/packages/backend/src/build-disk-image.spec.ts index d24ba2ef..c894a8d1 100644 --- a/packages/backend/src/build-disk-image.spec.ts +++ b/packages/backend/src/build-disk-image.spec.ts @@ -279,51 +279,3 @@ test('check uses Centos builder', async () => { expect(builder).toBeDefined(); expect(builder).toEqual(bootcImageBuilderCentos); }); - -test('check uses image preferred builder (RHEL)', async () => { - configurationGetConfigurationMock.mockReturnValue('image'); - - const listImagesMock = vi.fn(); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (containerEngine as any).listImages = listImagesMock; - listImagesMock.mockResolvedValue([ - { - RepoTags: ['test-image:latest'], - Labels: { 'bootc.diskimage-builder': 'registry.redhat.io/rhel9/bootc-image-builder' }, - }, - ]); - - const build = { - image: 'test-image', - tag: 'latest', - type: ['iso'], - } as BootcBuildInfo; - const builder = await getBuilder(build); - - expect(builder).toBeDefined(); - expect(builder).toEqual(bootcImageBuilderRHEL); -}); - -test('check uses image preferred builder (Centos)', async () => { - configurationGetConfigurationMock.mockReturnValue('image'); - - const listImagesMock = vi.fn(); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (containerEngine as any).listImages = listImagesMock; - listImagesMock.mockResolvedValue([ - { - RepoTags: ['test-image:latest'], - Labels: { 'bootc.diskimage-builder': 'quay.io/centos-bootc/bootc-image-builder' }, - }, - ]); - - const build = { - image: 'test-image', - tag: 'latest', - type: ['iso'], - } as BootcBuildInfo; - const builder = await getBuilder(build); - - expect(builder).toBeDefined(); - expect(builder).toEqual(bootcImageBuilderCentos); -}); diff --git a/packages/backend/src/build-disk-image.ts b/packages/backend/src/build-disk-image.ts index 5b6ddddb..3bf0a7d8 100644 --- a/packages/backend/src/build-disk-image.ts +++ b/packages/backend/src/build-disk-image.ts @@ -118,9 +118,8 @@ export async function buildDiskImage(build: BootcBuildInfo, history: History, ov fs.unlinkSync(logPath); } - // determine which bootc image builder to use based on the image - // being built and the current preferences - const builder = await getBuilder(build); + // determine which bootc image builder to use + const builder = await getBuilder(); // Preliminary Step 0. Create the "bootc-image-builder" container // options that we will use to build the image. This will help with debugging @@ -308,23 +307,15 @@ export async function getUnusedName(name: string): Promise { return unusedName; } -export async function getBuilder(build: BootcBuildInfo): Promise { - // check image for builder to use +export async function getBuilder(): Promise { + // use the preference to decide which builder to use const buildProp = await getConfigurationValue('builder'); if (buildProp === 'RHEL') { - // use to rhel if that's the preference return bootcImageBuilderRHEL; - } else if (buildProp === 'image') { - // or use rhel if the preference comes from the image label - // AND we detect the rhel label - const image = `${build.image}:${build.tag}`; - const buildLabel = await containerUtils.getImageBuilderLabel(image); - if (buildLabel === 'registry.redhat.io/rhel9/bootc-image-builder') { - return bootcImageBuilderRHEL; - } } - // otherwise, always use centos + + // always default to centos bib return bootcImageBuilderCentos; } diff --git a/packages/backend/src/constants.ts b/packages/backend/src/constants.ts index 1ed656d8..8e6389b4 100644 --- a/packages/backend/src/constants.ts +++ b/packages/backend/src/constants.ts @@ -20,4 +20,3 @@ export const bootcImageBuilder = 'bootc-image-builder'; export const bootcImageBuilderCentos = 'quay.io/centos-bootc/bootc-image-builder:latest-1714474808'; export const bootcImageBuilderRHEL = 'registry.redhat.io/rhel9/bootc-image-builder:9.4'; -export const bootcImageBuilderLabel = 'bootc.diskimage-builder'; diff --git a/packages/backend/src/container-utils.spec.ts b/packages/backend/src/container-utils.spec.ts index 9e81fad8..0dd8a7ab 100644 --- a/packages/backend/src/container-utils.spec.ts +++ b/packages/backend/src/container-utils.spec.ts @@ -28,7 +28,6 @@ import { deleteOldImages, inspectImage, inspectManifest, - getImageBuilderLabel, } from './container-utils'; const mocks = vi.hoisted(() => ({ @@ -266,19 +265,3 @@ test('test running inspectManifest', async () => { expect(result.engineName).toBe('podman'); expect(result.manifests).toBeDefined(); }); - -test('test image builder label', async () => { - const listImagesMock = vi.fn(); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (extensionApi.containerEngine as any).listImages = listImagesMock; - listImagesMock.mockResolvedValue([ - { RepoTags: ['test.io/name:1'] }, - { RepoTags: ['test.io/name:2'], Labels: { 'bootc.diskimage-builder': 'foo' } }, - { RepoTags: ['test.io/name:3'] }, - { RepoTags: ['test.io/name:4', 'keep-me'] }, - ]); - - // Test that it'll find the right image and label - const result = await getImageBuilderLabel('test.io/name:2'); - expect(result).toBe('foo'); -}); diff --git a/packages/backend/src/container-utils.ts b/packages/backend/src/container-utils.ts index 5c62aa10..f2dcdedd 100644 --- a/packages/backend/src/container-utils.ts +++ b/packages/backend/src/container-utils.ts @@ -18,7 +18,6 @@ import type { ContainerCreateOptions } from '@podman-desktop/api'; import * as extensionApi from '@podman-desktop/api'; import { getConfigurationValue, telemetryLogger } from './extension'; -import { bootcImageBuilderLabel } from './constants'; // Get the running container engine export async function getContainerEngine(): Promise { @@ -348,16 +347,3 @@ export async function getImagesFromManifest( // Filter out the images that have the same digest value return images.filter(image => image.Digest && digestValues.includes(image.Digest)); } - -// Return the image builder label from the given image -export async function getImageBuilderLabel(imageId: string): Promise { - try { - const images = await extensionApi.containerEngine.listImages(); - - const foundImage = images.find(i => i.RepoTags?.find(tag => tag === imageId)); - return foundImage?.Labels[bootcImageBuilderLabel]; - } catch (err) { - console.error('Error getting image label: ', err); - } - return undefined; -} From e7c710183fb4e1e0e97faf52ab011d281979aee0 Mon Sep 17 00:00:00 2001 From: Tim deBoer Date: Mon, 6 May 2024 10:06:11 -0400 Subject: [PATCH 3/4] chore: fix CentOS, pref description Signed-off-by: Tim deBoer --- packages/backend/package.json | 6 +++--- packages/backend/src/build-disk-image.spec.ts | 14 +++----------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index beacce8d..6153407f 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -23,12 +23,12 @@ }, "bootc.builder": { "type": "string", - "default": "Centos", + "default": "CentOS", "enum": [ - "Centos", + "CentOS", "RHEL" ], - "description": "Builder image" + "description": "Builder image used to create disk images" } } }, diff --git a/packages/backend/src/build-disk-image.spec.ts b/packages/backend/src/build-disk-image.spec.ts index c894a8d1..cc3364cb 100644 --- a/packages/backend/src/build-disk-image.spec.ts +++ b/packages/backend/src/build-disk-image.spec.ts @@ -257,24 +257,16 @@ test('check build exists', async () => { test('check uses RHEL builder', async () => { configurationGetConfigurationMock.mockReturnValue('RHEL'); - const build = { - image: 'test-image', - type: ['ami'], - } as BootcBuildInfo; - const builder = await getBuilder(build); + const builder = await getBuilder(); expect(builder).toBeDefined(); expect(builder).toEqual(bootcImageBuilderRHEL); }); test('check uses Centos builder', async () => { - configurationGetConfigurationMock.mockReturnValue('centos'); + configurationGetConfigurationMock.mockReturnValue('CentOS'); - const build = { - image: 'test-image', - type: ['ami'], - } as BootcBuildInfo; - const builder = await getBuilder(build); + const builder = await getBuilder(); expect(builder).toBeDefined(); expect(builder).toEqual(bootcImageBuilderCentos); From 099e8411d066025b0dfd9b27adc440836a2da8cb Mon Sep 17 00:00:00 2001 From: Tim deBoer Date: Mon, 6 May 2024 10:16:19 -0400 Subject: [PATCH 4/4] chore: don't switch default bib Signed-off-by: Tim deBoer --- packages/backend/src/constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/constants.ts b/packages/backend/src/constants.ts index 8e6389b4..6ed9d48a 100644 --- a/packages/backend/src/constants.ts +++ b/packages/backend/src/constants.ts @@ -18,5 +18,5 @@ // Image related export const bootcImageBuilder = 'bootc-image-builder'; -export const bootcImageBuilderCentos = 'quay.io/centos-bootc/bootc-image-builder:latest-1714474808'; +export const bootcImageBuilderCentos = 'quay.io/centos-bootc/bootc-image-builder:latest-1714633180'; export const bootcImageBuilderRHEL = 'registry.redhat.io/rhel9/bootc-image-builder:9.4';