docker
/
build-push-action
mirror of https://github.com/docker/build-push-action.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

refactor: complete removal of buildkit and sticky disk management

Browse Source 
This commit completes the refactoring of build-push-action to focus solely on
Docker build reporting and metrics, with all infrastructure management moved
to the separate setup-docker-builder action.

Changes:
- Remove all setupOnly references from context.ts, main.ts, and state-helper.ts
- Rename startBlacksmithBuilder to reportBuildMetrics to better reflect its purpose
- Remove exposeId from all function signatures and state management
- Remove sticky disk commit logic from reporter.ts
- Update tests to match new function names and signatures
- Clean up unused imports and fix linting issues

The action now assumes that a Docker builder has already been configured
(either via setup-docker-builder or existing setup) and focuses only on:
- Running Docker builds with the configured builder
- Reporting build metrics and status to Blacksmith API
- Managing build outputs and metadata

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
pull/1394/head
Claude 5 months ago
parent 877a04de98
commit cc46a915dd
8 changed files with 50 additions and 231 deletions
Show Stats Download Patch File Download Diff File

1
babel.config.js
Unescape Escape View File

@ -1,3 +1,4 @@
// eslint-disable-next-line no-undef
module.exports = {
presets: [
['@babel/preset-env', { targets: { node: 'current' } }],

2
dist/index.js generated vendored
View File

File diff suppressed because one or more lines are too long

2
dist/index.js.map generated vendored
View File

File diff suppressed because one or more lines are too long

124
src/__tests__/blacksmith-builder.test.ts
Unescape Escape View File

@ -3,7 +3,6 @@ import * as main from '../main';
import * as reporter from '../reporter';
import {getDockerfilePath} from '../context';
import * as setupBuilder from '../setup_builder';
import {Metric_MetricType} from '@buf/blacksmith_vm-agent.bufbuild_es/stickydisk/v1/stickydisk_pb';
jest.mock('@actions/core', () => ({
debug: jest.fn(),
@ -25,21 +24,15 @@ jest.mock('../reporter', () => {
const actual = jest.requireActual('../reporter');
return {
...actual,
reportBuildPushActionFailure: jest.fn().mockResolvedValue(undefined),
reportMetric: jest.fn().mockImplementation((type: Metric_MetricType) => Promise.resolve())
reportBuildPushActionFailure: jest.fn().mockResolvedValue(undefined)
};
});
jest.mock('../setup_builder', () => ({
...jest.requireActual('../setup_builder'),
startAndConfigureBuildkitd: jest.fn(),
setupStickyDisk: jest.fn(),
getNumCPUs: jest.fn().mockResolvedValue(4),
leaveTailnet: jest.fn().mockResolvedValue(undefined),
getTailscaleIP: jest.fn()
reportBuildStart: jest.fn()
}));
describe('startBlacksmithBuilder', () => {
describe('reportBuildMetrics', () => {
let mockInputs;
beforeEach(() => {
@ -51,110 +44,47 @@ describe('startBlacksmithBuilder', () => {
};
});
test('should handle missing dockerfile path with nofallback=false', async () => {
test('should handle missing dockerfile path', async () => {
(getDockerfilePath as jest.Mock).mockReturnValue(null);
const result = await main.startBlacksmithBuilder(mockInputs);
const result = await main.reportBuildMetrics(mockInputs);
expect(result).toEqual({addr: null, buildId: null, exposeId: ''});
expect(core.warning).toHaveBeenCalledWith('Error during Blacksmith builder setup: Failed to resolve dockerfile path. Falling back to a local build.');
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(new Error('Failed to resolve dockerfile path'), 'starting blacksmith builder');
expect(result).toBeNull();
expect(core.warning).toHaveBeenCalledWith('Error during build metrics reporting: Failed to resolve dockerfile path');
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(new Error('Failed to resolve dockerfile path'), 'reporting build metrics');
});
test('should handle missing dockerfile path with nofallback=true', async () => {
(getDockerfilePath as jest.Mock).mockReturnValue(null);
mockInputs.nofallback = true;
await expect(main.startBlacksmithBuilder(mockInputs)).rejects.toThrow('Failed to resolve dockerfile path');
expect(core.warning).toHaveBeenCalledWith('Error during Blacksmith builder setup: Failed to resolve dockerfile path. Failing the build because nofallback is set.');
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(new Error('Failed to resolve dockerfile path'), 'starting blacksmith builder');
});
test('should handle error in setupStickyDisk with nofallback=false', async () => {
test('should successfully report build start', async () => {
const mockBuildId = 'test-build-id';
(getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile');
(setupBuilder.setupStickyDisk as jest.Mock).mockRejectedValue(new Error('Failed to obtain Blacksmith builder'));
(setupBuilder.reportBuildStart as jest.Mock).mockResolvedValue({ docker_build_id: mockBuildId });
mockInputs.nofallback = false;
const result = await main.startBlacksmithBuilder(mockInputs);
const result = await main.reportBuildMetrics(mockInputs);
expect(result).toEqual({addr: null, buildId: null, exposeId: ''});
expect(core.warning).toHaveBeenCalledWith('Error during Blacksmith builder setup: Failed to obtain Blacksmith builder. Falling back to a local build.');
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(new Error('Failed to obtain Blacksmith builder'), 'starting blacksmith builder');
expect(result).toBe(mockBuildId);
expect(setupBuilder.reportBuildStart).toHaveBeenCalledWith('/path/to/Dockerfile');
expect(reporter.reportBuildPushActionFailure).not.toHaveBeenCalled();
});
test('should handle error in setupStickyDisk with nofallback=true', async () => {
test('should handle reportBuildStart returning null', async () => {
(getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile');
const error = new Error('Failed to obtain Blacksmith builder');
(setupBuilder.setupStickyDisk as jest.Mock).mockRejectedValue(error);
mockInputs.nofallback = true;
(setupBuilder.reportBuildStart as jest.Mock).mockResolvedValue(null);
await expect(main.startBlacksmithBuilder(mockInputs)).rejects.toThrow(error);
expect(core.warning).toHaveBeenCalledWith('Error during Blacksmith builder setup: Failed to obtain Blacksmith builder. Failing the build because nofallback is set.');
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(error, 'starting blacksmith builder');
});
const result = await main.reportBuildMetrics(mockInputs);
test('should successfully start buildkitd when setup succeeds', async () => {
const mockBuildkitdAddr = 'unix:///run/buildkit/buildkitd.sock';
const mockExposeId = 'test-expose-id';
const mockBuildId = 'test-build-id';
const mockDevice = '/dev/vdb';
const mockParallelism = 4;
(getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile');
(setupBuilder.setupStickyDisk as jest.Mock).mockResolvedValue({
device: mockDevice,
buildId: mockBuildId,
exposeId: mockExposeId
});
(setupBuilder.getNumCPUs as jest.Mock).mockResolvedValue(mockParallelism);
(setupBuilder.startAndConfigureBuildkitd as jest.Mock).mockResolvedValue(mockBuildkitdAddr);
const result = await main.startBlacksmithBuilder(mockInputs);
expect(result).toEqual({
addr: mockBuildkitdAddr,
buildId: mockBuildId,
exposeId: mockExposeId
});
expect(setupBuilder.startAndConfigureBuildkitd).toHaveBeenCalledWith(mockParallelism, false, []);
expect(result).toBeNull();
expect(setupBuilder.reportBuildStart).toHaveBeenCalledWith('/path/to/Dockerfile');
expect(reporter.reportBuildPushActionFailure).not.toHaveBeenCalled();
});
test('should handle buildkitd startup failure with nofallback=false', async () => {
const mockDevice = '/dev/vdb';
const mockParallelism = 4;
test('should handle error in reportBuildStart', async () => {
(getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile');
(setupBuilder.setupStickyDisk as jest.Mock).mockResolvedValue({
device: mockDevice,
buildId: 'test-build-id',
exposeId: 'test-expose-id'
});
(setupBuilder.getNumCPUs as jest.Mock).mockResolvedValue(mockParallelism);
(setupBuilder.startAndConfigureBuildkitd as jest.Mock).mockRejectedValue(new Error('Failed to start buildkitd'));
mockInputs.nofallback = false;
const result = await main.startBlacksmithBuilder(mockInputs);
expect(result).toEqual({addr: null, buildId: null, exposeId: ''});
expect(core.warning).toHaveBeenCalledWith('Error during buildkitd setup: Failed to start buildkitd. Falling back to a local build.');
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalled();
});
(setupBuilder.reportBuildStart as jest.Mock).mockRejectedValue(new Error('API error'));
test('should throw error when buildkitd fails and nofallback is true', async () => {
const mockDevice = '/dev/vdb';
const mockParallelism = 4;
(getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile');
(setupBuilder.setupStickyDisk as jest.Mock).mockResolvedValue({
device: mockDevice,
buildId: 'test-build-id',
exposeId: 'test-expose-id'
});
(setupBuilder.getNumCPUs as jest.Mock).mockResolvedValue(mockParallelism);
(setupBuilder.startAndConfigureBuildkitd as jest.Mock).mockRejectedValue(new Error('Failed to start buildkitd'));
mockInputs.nofallback = true;
await expect(main.startBlacksmithBuilder(mockInputs)).rejects.toThrow('Failed to start buildkitd');
expect(core.warning).toHaveBeenCalledWith('Error during buildkitd setup: Failed to start buildkitd. Failing the build because nofallback is set.');
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalled();
const result = await main.reportBuildMetrics(mockInputs);
expect(result).toBeNull();
expect(core.warning).toHaveBeenCalledWith('Error during build metrics reporting: API error');
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(new Error('API error'), 'reporting build metrics');
});
});

2
src/context.ts
Unescape Escape View File

@ -58,7 +58,6 @@ export interface Inputs {
ulimit: string[];
'github-token': string;
nofallback: boolean;
setupOnly: boolean;
'buildx-version': string;
}
@ -98,7 +97,6 @@ export async function getInputs(): Promise<Inputs> {
ulimit: Util.getInputList('ulimit', {ignoreComma: true}),
'github-token': core.getInput('github-token'),
nofallback: core.getBooleanInput('nofallback'),
setupOnly: core.getBooleanInput('setup-only'),
'buildx-version': core.getInput('buildx-version')
};
}

92
src/main.ts
Unescape Escape View File

@ -17,16 +17,12 @@ import {BuilderInfo} from '@docker/actions-toolkit/lib/types/buildx/builder';
import {ConfigFile} from '@docker/actions-toolkit/lib/types/docker/docker';
import * as context from './context';
import {promisify} from 'util';
import {exec} from 'child_process';
import * as reporter from './reporter';
import {reportBuildStart, leaveTailnet} from './setup_builder';
import {reportBuildStart} from './setup_builder';
import {Metric_MetricType} from '@buf/blacksmith_vm-agent.bufbuild_es/stickydisk/v1/stickydisk_pb';
const DEFAULT_BUILDX_VERSION = 'v0.23.0';
const execAsync = promisify(exec);
async function retryWithBackoff<T>(operation: () => Promise<T>, maxRetries: number = 5, initialBackoffMs: number = 200): Promise<T> {
let lastError: Error = new Error('No error occurred');
for (let attempt = 0; attempt < maxRetries; attempt++) {
@ -80,65 +76,26 @@ function isValidBuildxVersion(version: string): boolean {
}
/**
* Attempts to set up a Blacksmith builder for Docker builds.
* Reports the build start to the backend and gets a build ID for tracking.
*
* @param inputs - Configuration inputs including the nofallback flag
* @returns {Object} Builder configuration
* @returns {string|null} addr - The buildkit socket address if setup succeeded, null if using local build
* @param inputs - Configuration inputs
* @returns {string|null} buildId - ID used to track build progress and report metrics
* @returns {string} exposeId - ID used to track and cleanup sticky disk resources
*
* The addr is used to configure the Docker buildx builder instance.
* The buildId is used for build progress tracking and metrics reporting.
* The exposeId is used during cleanup to ensure proper resource cleanup of sticky disks.
*
* Throws an error if setup fails and nofallback is false.
* Returns null values if setup fails and nofallback is true.
*/
export async function startBlacksmithBuilder(inputs: context.Inputs): Promise<{addr: string | null; buildId: string | null; exposeId: string}> {
export async function reportBuildMetrics(inputs: context.Inputs): Promise<string | null> {
try {
// We only use the dockerfile path to report the build to our control plane.
// If setup-only is true, we don't want to report the build to our control plane
// since we are only setting up the builder and therefore cannot expose any analytics
// about the build.
const dockerfilePath = inputs.setupOnly ? '' : context.getDockerfilePath(inputs);
if (!inputs.setupOnly && !dockerfilePath) {
// Get the dockerfile path to report the build to our control plane.
const dockerfilePath = context.getDockerfilePath(inputs);
if (!dockerfilePath) {
throw new Error('Failed to resolve dockerfile path');
}
// Report build start to get a build ID for tracking
let buildId: string | null = null;
if (!inputs.setupOnly && dockerfilePath) {
const buildInfo = await reportBuildStart(dockerfilePath);
buildId = buildInfo?.docker_build_id || null;
}
// For now, we'll check if a builder is already available from setup-docker-builder
// by looking for the sentinel file
const sentinelPath = path.join('/tmp', 'builder-setup-complete');
const builderAvailable = fs.existsSync(sentinelPath);
if (!builderAvailable) {
throw new Error('Docker builder not available. Please use setup-docker-builder action first.');
}
// We no longer manage expose IDs since sticky disk is handled by setup-docker-builder
return {addr: null, buildId: buildId, exposeId: ''};
const buildInfo = await reportBuildStart(dockerfilePath);
return buildInfo?.docker_build_id || null;
} catch (error) {
// If the builder setup fails for any reason, we check if we should fallback to a local build.
// If we should not fallback, we rethrow the error and fail the build.
await reporter.reportBuildPushActionFailure(error, 'starting blacksmith builder');
let errorMessage = `Error during Blacksmith builder setup: ${error.message}`;
if (inputs.nofallback) {
core.warning(`${errorMessage}. Failing the build because nofallback is set.`);
throw error;
}
core.warning(`${errorMessage}. Falling back to a local build.`);
return {addr: null, buildId: null, exposeId: ''};
} finally {
await leaveTailnet();
await reporter.reportBuildPushActionFailure(error, 'reporting build metrics');
core.warning(`Error during build metrics reporting: ${error.message}`);
return null;
}
}
@ -189,17 +146,13 @@ actionsToolkit.run(
}
});
let builderInfo = {
addr: null as string | null,
buildId: null as string | null,
exposeId: '' as string
};
let buildId: string | null = null;
let buildError: Error | undefined;
let buildDurationSeconds: string | undefined;
let ref: string | undefined;
try {
await core.group(`Setting up Blacksmith build tracking`, async () => {
builderInfo = await startBlacksmithBuilder(inputs);
await core.group(`Setting up build metrics tracking`, async () => {
buildId = await reportBuildMetrics(inputs);
});
// Check that a builder is available (either from setup-docker-builder or existing)
@ -224,13 +177,6 @@ actionsToolkit.run(
core.info(JSON.stringify(builder, null, 2));
});
// If setup-only is true, we don't want to continue configuring and running the build.
if (inputs.setupOnly) {
core.info('setup-only mode enabled, builder is ready for use by Docker');
stateHelper.setSetupOnly(true);
// Let's remove the default
process.exit(0);
}
await core.group(`Proxy configuration`, async () => {
let dockerConfig: ConfigFile | undefined;
@ -364,14 +310,13 @@ actionsToolkit.run(
// Buildkitd is now managed by setup-docker-builder, not here
await leaveTailnet();
// Sticky disk is now managed by setup-docker-builder, not here
if (builderInfo.buildId) {
if (buildId) {
if (!buildError) {
await reporter.reportBuildCompleted(exportRes, builderInfo.buildId, ref, buildDurationSeconds, '');
await reporter.reportBuildCompleted(exportRes, buildId, ref, buildDurationSeconds);
} else {
await reporter.reportBuildFailed(builderInfo.buildId, buildDurationSeconds, '');
await reporter.reportBuildFailed(buildId, buildDurationSeconds);
}
}
} catch (error) {
@ -391,7 +336,6 @@ actionsToolkit.run(
async () => {
await core.group('Final cleanup', async () => {
try {
await leaveTailnet();
// Buildkitd is now managed by setup-docker-builder, not here
// Sticky disk is also managed by setup-docker-builder, not here

42
src/reporter.ts
Unescape Escape View File

@ -60,24 +60,13 @@ export async function reportBuildPushActionFailure(error?: Error, event?: string
return response.data;
}
export async function reportBuildCompleted(exportRes?: ExportRecordResponse, blacksmithDockerBuildId?: string | null, buildRef?: string, dockerBuildDurationSeconds?: string, exposeId?: string): Promise<void> {
export async function reportBuildCompleted(exportRes?: ExportRecordResponse, blacksmithDockerBuildId?: string | null, buildRef?: string, dockerBuildDurationSeconds?: string): Promise<void> {
if (!blacksmithDockerBuildId) {
core.warning('No docker build ID found, skipping build completion report');
return;
}
try {
const agentClient = createBlacksmithAgentClient();
await agentClient.commitStickyDisk({
exposeId: exposeId || '',
stickyDiskKey: process.env.GITHUB_REPO_NAME || '',
vmId: process.env.BLACKSMITH_VM_ID || '',
shouldCommit: true,
repoName: process.env.GITHUB_REPO_NAME || '',
stickyDiskToken: process.env.BLACKSMITH_STICKYDISK_TOKEN || ''
});
// Report success to Blacksmith API
const requestOptions = {
docker_build_id: blacksmithDockerBuildId,
@ -111,23 +100,13 @@ export async function reportBuildCompleted(exportRes?: ExportRecordResponse, bla
}
}
export async function reportBuildFailed(dockerBuildId: string | null, dockerBuildDurationSeconds?: string, exposeId?: string | null): Promise<void> {
export async function reportBuildFailed(dockerBuildId: string | null, dockerBuildDurationSeconds?: string): Promise<void> {
if (!dockerBuildId) {
core.warning('No docker build ID found, skipping build completion report');
return;
}
try {
const blacksmithAgentClient = createBlacksmithAgentClient();
await blacksmithAgentClient.commitStickyDisk({
exposeId: exposeId || '',
stickyDiskKey: process.env.GITHUB_REPO_NAME || '',
vmId: process.env.BLACKSMITH_VM_ID || '',
shouldCommit: false,
repoName: process.env.GITHUB_REPO_NAME || '',
stickyDiskToken: process.env.BLACKSMITH_STICKYDISK_TOKEN || ''
});
// Report failure to Blacksmith API
const requestOptions = {
docker_build_id: dockerBuildId,
@ -196,20 +175,3 @@ export async function reportMetric(metricType: Metric_MetricType, value: number)
// core.warning('Error reporting metric to BlacksmithAgent:', error);
}
}
export async function commitStickyDisk(exposeId?: string, shouldCommit: boolean = true): Promise<void> {
try {
const agentClient = createBlacksmithAgentClient();
await agentClient.commitStickyDisk({
exposeId: exposeId || '',
stickyDiskKey: process.env.GITHUB_REPO_NAME || '',
vmId: process.env.BLACKSMITH_VM_ID || '',
shouldCommit,
repoName: process.env.GITHUB_REPO_NAME || '',
stickyDiskToken: process.env.BLACKSMITH_STICKYDISK_TOKEN || ''
});
} catch (error) {
core.warning('Error committing sticky disk:', error);
}
}

16
src/state-helper.ts
Unescape Escape View File

@ -58,19 +58,3 @@ export function setDockerBuildStatus(dockerBuildStatus: string) {
export function setDockerBuildDurationSeconds(dockerBuildDurationSeconds: string) {
core.saveState('dockerBuildDurationSeconds', dockerBuildDurationSeconds);
}
export function setExposeId(exposeId: string) {
core.saveState('exposeId', exposeId);
}
export function getExposeId(): string {
return core.getState('exposeId');
}
export function setSetupOnly(setupOnly: boolean) {
core.saveState('setupOnly', setupOnly.toString());
}
export function getSetupOnly(): boolean {
return core.getState('setupOnly') === 'true';
}

Write Preview
Loading…
Cancel
Save