Skip to content

feat: support rhel bib #449

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 6, 2024
Merged

feat: support rhel bib #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b174a70
feat: support rhel bib
deboer-tim May 2, 2024
11a8cc2
chore: remove image label detection for now
deboer-tim May 6, 2024
e7c7101
chore: fix CentOS, pref description
deboer-tim May 6, 2024
099e841
chore: don't switch default bib
deboer-tim May 6, 2024
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
9 changes: 9 additions & 0 deletions packages/backend/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@
"default": 30,
"maximum": 120,
"description": "Build timeout (in minutes)"
},
"bootc.builder": {
"type": "string",
"default": "CentOS",
"enum": [
"CentOS",
"RHEL"
],
"description": "Builder image used to create disk images"
}
}
},
Expand Down
51 changes: 46 additions & 5 deletions packages/backend/src/build-disk-image.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -33,6 +41,9 @@ vi.mock('@podman-desktop/api', async () => {
containerEngine: {
listContainers: vi.fn().mockReturnValue([]),
},
configuration: {
getConfiguration: () => config,
},
};
});

Expand All @@ -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/');
Expand Down Expand Up @@ -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/');
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -230,3 +253,21 @@ 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 builder = await getBuilder();

expect(builder).toBeDefined();
expect(builder).toEqual(bootcImageBuilderRHEL);
});

test('check uses Centos builder', async () => {
configurationGetConfigurationMock.mockReturnValue('CentOS');

const builder = await getBuilder();

expect(builder).toBeDefined();
expect(builder).toEqual(bootcImageBuilderCentos);
});
31 changes: 25 additions & 6 deletions packages/backend/src/build-disk-image.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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';
Expand All @@ -118,11 +118,14 @@ export async function buildDiskImage(build: BootcBuildInfo, history: History, ov
fs.unlinkSync(logPath);
}

// 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
// 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 {
Expand Down Expand Up @@ -304,8 +307,24 @@ export async function getUnusedName(name: string): Promise<string> {
return unusedName;
}

export async function getBuilder(): Promise<string> {
Copy link
Contributor

@cgwalters cgwalters May 16, 2025

Choose a reason for hiding this comment

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

Hmm the commit message mentions the bootc.diskimage-builder label but I don't see it mentioned or used in the code anywhere?

Oh except...argh, we may have lost that label in some reworking of the base image builds.

Copy link
Contributor Author

@deboer-tim deboer-tim May 16, 2025

Choose a reason for hiding this comment

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

We didn't see it used consistently enough in the images (and sounds like it isn't going to be?), so the extension still uses a simple builder preference (under Settings > Preferences > Bootable Containers in the UI).

// use the preference to decide which builder to use
const buildProp = await getConfigurationValue<string>('builder');

if (buildProp === 'RHEL') {
return bootcImageBuilderRHEL;
}

// always default to centos bib
return bootcImageBuilderCentos;
Copy link
Contributor

@cdrage cdrage May 6, 2024

Choose a reason for hiding this comment

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

should we fail at all if the config setting is not centos or rhel? i see in our tests it still passed.

example you can pass in anything 'foobar' as the configuration setting and it'll always pick centos.

Copy link
Contributor Author

@deboer-tim deboer-tim May 6, 2024

Choose a reason for hiding this comment

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

I wanted to play it safe, i.e. even if config corrupted => we ignore and use CentOS anyway.

}

// 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));
Expand All @@ -323,7 +342,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,
Expand Down
5 changes: 3 additions & 2 deletions packages/backend/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@
***********************************************************************/

// 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-1714633180';
export const bootcImageBuilderRHEL = 'registry.redhat.io/rhel9/bootc-image-builder:9.4';
Copy link
Contributor

@cgwalters cgwalters May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

There's more than one RHEL version. The 9.4 image is Tech Preview even and isn't going to be updated more and we will definitely (for rhel9.6) want to use registry.redhat.io/rhel9/bootc-image-builder:9.6 etc.

I think in general we can just use registry.redhat.io/rhel$MAJOR/bootc-image-builder:latest reasonably safely.

To determine $MAJOR though requires inspecting the container image; like podman run <image> bash -c '. /usr/lib/os-release && echo $REDHAT_SUPPORT_PRODUCT_VERSION or so perhaps.

(In theory we could use $VERSION_ID except that may be overridden by downstream builds)

Copy link
Contributor Author

@deboer-tim deboer-tim May 16, 2025

Choose a reason for hiding this comment

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

The current value of that const is registry.redhat.io/rhel9/bootc-image-builder:9.5. Adding other builders to the list is simple if someone can confirm the list, but would prefer not to use :latest unless we know there is someone who understands the implications of that and agrees to support it.

Is using REDHAT_SUPPORT_PRODUCT_VERSION the normal way to determine which builder you should use? i.e. if we see 9.6 we should always use the 9.6 builder? I'd be inclined to go back to the support we were planning for bootc.diskimage-builder - the default builder preference would be something like 'based on image' and we'd use the correct builder if we found the version, and default to Centos builder if it didn't exist.