refactor: Remove buildkit management from build-push-action

- Remove buildkitd startup and configuration logic
- Remove buildkitd shutdown and cleanup from both main and post actions
- Remove buildkitd-related imports and helper functions
- Update startBlacksmithBuilder to check for existing builder from setup-docker-builder
- Keep sticky disk setup and build reporting functionality intact

BREAKING CHANGE: This action now requires setup-docker-builder to be run first to manage the Docker builder lifecycle

plan.md

@@ -0,0 +1,78 @@
+# Build-Push-Action Refactoring Plan
+
+## Overview
+Split the current `useblacksmith/build-push-action` into two separate actions:
+1. `useblacksmith/setup-docker-builder` - Manages buildkitd lifecycle and stickydisk
+2. `useblacksmith/build-push-action` - Focuses on Docker builds and metrics reporting
+
+## Current Problems
+- The existing action supports two modes: "setup-only" and normal mode
+- Complex logic to manage buildkitd lifecycle across multiple invocations
+- Buildkitd must be shut down after each build to support multiple builds in one job
+- Post-action steps run in reverse order, complicating cleanup
+
+## Proposed Architecture
+
+### useblacksmith/setup-docker-builder
+**Responsibilities:**
+- Start buildkitd once per workflow job
+- Mount stickydisk at `/var/lib/buildkit` for shared Docker layer cache
+- Handle all cleanup, shutdown, and commit logic in post-action
+- Manage the entire buildkitd lifecycle
+
+**Key Features:**
+- Single buildkitd instance for entire job
+- All stickydisk logic centralized here
+- Post-action handles:
+  - Buildkitd shutdown
+  - Stickydisk commit (conditional based on build success)
+  - Cleanup
+
+### useblacksmith/build-push-action
+**Responsibilities:**
+- Execute Docker builds against running buildkitd
+- Report build metrics to control plane
+- No buildkitd lifecycle management
+
+**Key Features:**
+- Simplified logic - just build and push
+- Can be invoked multiple times in same job
+- Focuses on Docker operations and telemetry
+
+## Usage Patterns
+
+### Multiple Dockerfiles
+```yaml
+- uses: useblacksmith/setup-docker-builder
+- uses: useblacksmith/build-push-action  # dockerfile 1
+- uses: useblacksmith/build-push-action  # dockerfile 2
+- uses: useblacksmith/build-push-action  # dockerfile 3
+```
+
+### Custom Docker Commands
+```yaml
+- uses: useblacksmith/setup-docker-builder
+- run: docker bake
+- run: # other custom docker commands
+```
+
+## Open Questions
+1. How can the post-action of `setup-docker-builder` access build results from `build-push-action` invocations?
+   - Need to determine if stickydisk should be committed based on build success
+   - Possible solutions:
+     - Environment variables
+     - File-based communication
+     - GitHub Actions outputs/state
+
+## Benefits
+1. **Cleaner separation of concerns** - Setup vs build logic separated
+2. **Simpler maintenance** - Each action has focused responsibility
+3. **Better user experience** - One buildkitd instance regardless of build count
+4. **More flexible** - Users can mix our build action with custom Docker commands
+
+## Migration Path
+1. Create new `setup-docker-builder` repository/action
+2. Move buildkitd setup and stickydisk logic from current action
+3. Refactor `build-push-action` to remove setup logic
+4. Update documentation and examples
+5. Provide migration guide for existing users

src/main.ts

@@ -20,7 +20,7 @@ import * as context from './context';
 import {promisify} from 'util';
 import {exec} from 'child_process';
 import * as reporter from './reporter';
-import {setupStickyDisk, startAndConfigureBuildkitd, getNumCPUs, leaveTailnet, pruneBuildkitCache} from './setup_builder';
+import {setupStickyDisk, leaveTailnet} from './setup_builder';
 import {Metric_MetricType} from '@buf/blacksmith_vm-agent.bufbuild_es/stickydisk/v1/stickydisk_pb';
 
 const DEFAULT_BUILDX_VERSION = 'v0.23.0';
@@ -110,23 +110,25 @@ export async function startBlacksmithBuilder(inputs: context.Inputs): Promise<{a
     const stickyDiskSetup = await setupStickyDisk(dockerfilePath || '', inputs.setupOnly);
     const stickyDiskDurationMs = Date.now() - stickyDiskStartTime;
     await reporter.reportMetric(Metric_MetricType.BPA_HOTLOAD_DURATION_MS, stickyDiskDurationMs);
-    const parallelism = await getNumCPUs();
+    
-
+    // For now, we'll check if a builder is already available from setup-docker-builder
-    const buildkitdStartTime = Date.now();
+    // by looking for the sentinel file
-    const buildkitdAddr = await startAndConfigureBuildkitd(parallelism, inputs.setupOnly, inputs.platforms);
+    const sentinelPath = path.join('/tmp', 'builder-setup-complete');
-    const buildkitdDurationMs = Date.now() - buildkitdStartTime;
+    const builderAvailable = fs.existsSync(sentinelPath);
-    await reporter.reportMetric(Metric_MetricType.BPA_BUILDKITD_READY_DURATION_MS, buildkitdDurationMs);
+    
+    if (!builderAvailable) {
+      throw new Error('Docker builder not available. Please use setup-docker-builder action first.');
+    }
+    
     stateHelper.setExposeId(stickyDiskSetup.exposeId);
-    return {addr: buildkitdAddr, buildId: stickyDiskSetup.buildId || null, exposeId: stickyDiskSetup.exposeId};
+    // Return null for addr since we're not managing the builder anymore
+    return {addr: null, buildId: stickyDiskSetup.buildId || null, exposeId: stickyDiskSetup.exposeId};
   } 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 (error.message.includes('buildkitd')) {
-      errorMessage = `Error during buildkitd setup: ${error.message}`;
-    }
     if (inputs.nofallback) {
       core.warning(`${errorMessage}. Failing the build because nofallback is set.`);
       throw error;
@@ -195,66 +197,25 @@ actionsToolkit.run(
     let buildDurationSeconds: string | undefined;
     let ref: string | undefined;
     try {
-      await core.group(`Starting Blacksmith builder`, async () => {
+      await core.group(`Setting up Blacksmith build tracking`, async () => {
         builderInfo = await startBlacksmithBuilder(inputs);
       });
 
-      if (builderInfo.addr) {
+      // Check that a builder is available (either from setup-docker-builder or existing)
-        await core.group(`Creating a builder instance`, async () => {
+      await core.group(`Checking for configured builder`, async () => {
-          const name = `blacksmith-${Date.now().toString(36)}`;
+        try {
-          const createCmd = await toolkit.buildx.getCommand(await context.getRemoteBuilderArgs(name, builderInfo.addr!, inputs.platforms));
+          const builder = await toolkit.builder.inspect();
-          core.info(`Creating builder with command: ${createCmd.command}`);
+          if (builder) {
-          await Exec.getExecOutput(createCmd.command, createCmd.args, {
+            core.info(`Found configured builder: ${builder.name}`);
-            ignoreReturnCode: true
+          } else {
-          }).then(res => {
+            core.setFailed(`No Docker builder found. Please use setup-docker-builder action or configure a builder before using build-push-action.`);
-            if (res.stderr.length > 0 && res.exitCode != 0) {
-              throw new Error(res.stderr.match(/(.*)\s*$/)?.[0]?.trim() ?? 'unknown error');
-            }
-          });
-          // Set this builder as the global default since future docker commands will use this builder.
-          if (inputs.setupOnly) {
-            const setDefaultCmd = await toolkit.buildx.getCommand(await context.getUseBuilderArgs(name));
-            core.info('Setting builder as global default');
-            await Exec.getExecOutput(setDefaultCmd.command, setDefaultCmd.args, {
-              ignoreReturnCode: true
-            }).then(res => {
-              if (res.stderr.length > 0 && res.exitCode != 0) {
-                throw new Error(res.stderr.match(/(.*)\s*$/)?.[0]?.trim() ?? 'unknown error');
-              }
-            });
           }
-        });
+        } catch (error) {
-      } else {
+          core.setFailed(`Error checking for builder: ${error.message}`);
-        core.warning('Failed to setup Blacksmith builder, falling back to default builder');
+        }
-        await core.group(`Checking for configured builder`, async () => {
+      });
-          try {
-            const builder = await toolkit.builder.inspect();
-            if (builder) {
-              core.info(`Found configured builder: ${builder.name}`);
-            } else {
-              // Create a local builder using the docker-container driver (which is the default driver in setup-buildx)
-              const createLocalBuilderCmd = 'docker buildx create --name local --driver docker-container --use';
-              try {
-                await Exec.exec(createLocalBuilderCmd);
-                core.info('Created and set a local builder for use');
-              } catch (error) {
-                core.setFailed(`Failed to create local builder: ${error.message}`);
-              }
-            }
-          } catch (error) {
-            core.setFailed(`Error configuring builder: ${error.message}`);
-          }
-        });
-      }
 
-      // Write a sentinel file to indicate builder setup is complete.
+      // The sentinel file should already exist from setup-docker-builder
-      const sentinelPath = path.join('/tmp', 'builder-setup-complete');
-      try {
-        fs.writeFileSync(sentinelPath, 'Builder setup completed successfully.');
-        core.debug(`Created builder setup sentinel file at ${sentinelPath}`);
-      } catch (error) {
-        core.warning(`Failed to create builder setup sentinel file: ${error.message}`);
-      }
 
       let builder: BuilderInfo;
       await core.group(`Builder info`, async () => {
@@ -400,34 +361,7 @@ actionsToolkit.run(
           });
         }
 
-        try {
+        // Buildkitd is now managed by setup-docker-builder, not here
-          const {stdout} = await execAsync('pgrep buildkitd');
-          if (stdout.trim()) {
-            try {
-              core.info('Pruning BuildKit cache');
-              await pruneBuildkitCache();
-              core.info('BuildKit cache pruned');
-            } catch (error) {
-              // Log warning but don't fail the cleanup
-              core.warning(`Error pruning BuildKit cache: ${error.message}`);
-            }
-
-            const buildkitdShutdownStartTime = Date.now();
-            await shutdownBuildkitd();
-            const buildkitdShutdownDurationMs = Date.now() - buildkitdShutdownStartTime;
-            await reporter.reportMetric(Metric_MetricType.BPA_BUILDKITD_SHUTDOWN_DURATION_MS, buildkitdShutdownDurationMs);
-            core.info('Shutdown buildkitd');
-          } else {
-            core.debug('No buildkitd process found running');
-          }
-        } catch (error) {
-          if (error.code === 1) {
-            // pgrep returns non-zero if no processes found, which is fine
-            core.debug('No buildkitd process found running');
-          } else {
-            core.warning(`Error checking for buildkitd processes: ${error.message}`);
-          }
-        }
 
         await leaveTailnet();
         try {
@@ -471,21 +405,7 @@ actionsToolkit.run(
         core.warning(`Error during Blacksmith builder shutdown: ${error.message}`);
         await reporter.reportBuildPushActionFailure(error, 'shutting down blacksmith builder');
       } finally {
-        if (buildError) {
+        // Buildkitd logs are managed by setup-docker-builder
-          try {
-            const buildkitdLog = fs.readFileSync('/tmp/buildkitd.log', 'utf8');
-            core.info('buildkitd.log contents:');
-            core.info(buildkitdLog);
-          } catch (error) {
-            // Only log warning if the file was expected to exist (builder setup completed)
-            const sentinelPath = path.join('/tmp', 'builder-setup-complete');
-            if (fs.existsSync(sentinelPath)) {
-              core.warning(`Failed to read buildkitd.log: ${error.message}`);
-            } else {
-              core.debug(`buildkitd.log not found (builder setup incomplete): ${error.message}`);
-            }
-          }
-        }
       }
     });
 
@@ -500,28 +420,7 @@ actionsToolkit.run(
       try {
         await leaveTailnet();
 
-        try {
+        // Buildkitd is now managed by setup-docker-builder, not here
-          const {stdout} = await execAsync('pgrep buildkitd');
-          if (stdout.trim()) {
-            try {
-              core.info('Pruning BuildKit cache');
-              await pruneBuildkitCache();
-              core.info('BuildKit cache pruned');
-            } catch (error) {
-              // Log warning but don't fail the cleanup
-              core.warning(`Error pruning BuildKit cache: ${error.message}`);
-            }
-
-            await shutdownBuildkitd();
-            core.info('Shutdown buildkitd');
-          }
-        } catch (error) {
-          if (error.code === 1) {
-            core.debug('No buildkitd process found running');
-          } else {
-            core.warning(`Error checking for buildkitd processes: ${error.message}`);
-          }
-        }
 
         try {
           // Run sync to flush any pending writes before unmounting.
@@ -612,34 +511,3 @@ function buildSummaryEnabled(): boolean {
   return true;
 }
 
-export async function shutdownBuildkitd(): Promise<void> {
-  const startTime = Date.now();
-  const timeout = 10000; // 10 seconds
-  const backoff = 300; // 300ms
-
-  try {
-    await execAsync(`sudo pkill -TERM buildkitd`);
-
-    // Wait for buildkitd to shutdown with backoff retry
-    while (Date.now() - startTime < timeout) {
-      try {
-        const {stdout} = await execAsync('pgrep buildkitd');
-        core.debug(`buildkitd process still running with PID: ${stdout.trim()}`);
-        await new Promise(resolve => setTimeout(resolve, backoff));
-      } catch (error) {
-        if (error.code === 1) {
-          // pgrep returns exit code 1 when no process is found, which means shutdown successful
-          core.debug('buildkitd successfully shutdown');
-          return;
-        }
-        // Some other error occurred
-        throw error;
-      }
-    }
-
-    throw new Error('Timed out waiting for buildkitd to shutdown after 10 seconds');
-  } catch (error) {
-    core.error('error shutting down buildkitd process:', error);
-    throw error;
-  }
-}