Skip to content

feat(frontend): frontend docker improving #175

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
Sma1lboy merged 5 commits into from
Mar 9, 2025
Merged

feat(frontend): frontend docker improving #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1e9f182
Update docker to use only one base image
ZHallen122 Mar 8, 2025
01f4097
add constrained
ZHallen122 Mar 9, 2025
e23cd55
[autofix.ci] apply automated fixes
autofix-ci[bot] Mar 9, 2025
a3d2888
improve webview only show page when service is ready
ZHallen122 Mar 9, 2025
4c79376
[autofix.ci] apply automated fixes
autofix-ci[bot] Mar 9, 2025
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
19 changes: 19 additions & 0 deletions docker/project-base-image/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
FROM node:20

WORKDIR /app

# Pre-install common frontend dependencies to speed up project startup
RUN npm install -g npm@latest vite@latest
Comment on lines +5 to +6
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use pinned versions for npm and Vite.

Installing npm@latest and vite@latest globally can lead to unpredictable builds if these packages publish breaking updates. Consider pinning specific versions or referencing a lockfile for more consistent and reproducible builds.

-RUN npm install -g npm@latest vite@latest
+RUN npm install -g npm@9.8.1 vite@4.3.9
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Pre-install common frontend dependencies to speed up project startup
RUN npm install -g npm@latest vite@latest
# Pre-install common frontend dependencies to speed up project startup
RUN npm install -g npm@9.8.1 vite@4.3.9


# Create a non-root user to run the app
RUN groupadd -r appuser && useradd -r -g appuser -m appuser
RUN chown -R appuser:appuser /app

# Switch to non-root user for security
USER appuser

EXPOSE 5173

# The actual project code will be mounted as a volume
# The CMD will be provided when running the container
CMD ["sh", "-c", "npm install --include=dev && npm run dev -- --host 0.0.0.0"]
126 changes: 79 additions & 47 deletions frontend/src/app/api/runProject/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import { URL_PROTOCOL_PREFIX } from '@/utils/const';
const CONTAINER_STATE_FILE = path.join(process.cwd(), 'container-state.json');
const PORT_STATE_FILE = path.join(process.cwd(), 'port-state.json');

// Base image name - this is the single image we'll use for all containers
const BASE_IMAGE_NAME = 'frontend-base-image';

// In-memory container and port state
let runningContainers = new Map<
string,
Expand All @@ -23,6 +26,15 @@ const processingRequests = new Set<string>();
// State lock to prevent concurrent reads/writes to state files
let isUpdatingState = false;

// Flag to track if base image has been built
let baseImageBuilt = false;
Comment on lines +29 to +30
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Address potential concurrency issue with baseImageBuilt.

Because baseImageBuilt is a simple boolean, multiple requests arriving simultaneously may each try to build the image before it’s set to true. This can cause redundant builds or race conditions.

Also applies to: 91-92


// limit memory usage for a container
const memoryLimit = '400m';

// limit cpu usage for a container
const cpusLimit = 1;

/**
* Initialize function, loads persisted state when service starts
*/
Expand Down Expand Up @@ -75,6 +87,9 @@ async function initializeState() {
// Save cleaned-up state
await saveState();

// Check if base image exists
baseImageBuilt = await checkBaseImageExists();

console.log(
'State initialization complete, cleaned up non-running containers and expired port allocations'
);
Expand Down Expand Up @@ -180,6 +195,21 @@ function checkContainerRunning(containerId: string): Promise<boolean> {
});
}

/**
* Check if base image exists
*/
function checkBaseImageExists(): Promise<boolean> {
return new Promise((resolve) => {
exec(`docker image inspect ${BASE_IMAGE_NAME}`, (err) => {
if (err) {
resolve(false);
} else {
resolve(true);
}
});
});
}

/**
* Check if there's already a container running with the specified label
*/
Expand All @@ -203,27 +233,42 @@ async function checkExistingContainer(
}

/**
* Remove node_modules and lock files
* Build base image if it doesn't exist
Comment on lines 235 to +236
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Serialize base image builds to avoid race conditions.

Multiple concurrent calls to ensureBaseImageExists could block or conflict. One approach is to maintain a building “lock” or queue:

-let baseImageBuilt = false;
+let baseImageBuilding = false;
+let baseImagePromise: Promise<void> | null = null;

async function ensureBaseImageExists(): Promise<void> {
  if (baseImageBuilt) return;

+  if (baseImageBuilding && baseImagePromise) {
+    // If a build is already in progress, wait for that promise
+    await baseImagePromise;
+    return;
+  }
+  baseImageBuilding = true;
+  baseImagePromise = (async () => {
    try {
      ...
      baseImageBuilt = true;
    } finally {
+      baseImageBuilding = false;
+    }
+  })();
+  await baseImagePromise;
}

Also applies to: 238-272

*/
async function removeNodeModulesAndLockFiles(directory: string) {
return new Promise<void>((resolve, reject) => {
const removeCmd = `rm -rf "${path.join(directory, 'node_modules')}" \
"${path.join(directory, 'yarn.lock')}" \
"${path.join(directory, 'package-lock.json')}" \
"${path.join(directory, 'pnpm-lock.yaml')}"`;

console.log(`Cleaning up node_modules and lock files in: ${directory}`);
exec(removeCmd, { timeout: 30000 }, (err, stdout, stderr) => {
if (err) {
console.error('Error removing node_modules or lock files:', stderr);
// Don't block the process, continue even if cleanup fails
resolve();
return;
}
console.log(`Cleanup done: ${stdout}`);
resolve();
});
});
async function ensureBaseImageExists(): Promise<void> {
if (baseImageBuilt) {
return;
}

try {
// Path to the base image Dockerfile
const dockerfilePath = path.join(
process.cwd(),
'../docker',
'project-base-image'
);

// Check if base Dockerfile exists
if (!fs.existsSync(path.join(dockerfilePath, 'Dockerfile'))) {
console.error('Base Dockerfile not found at:', dockerfilePath);
throw new Error('Base Dockerfile not found');
}

// Build the base image
console.log(
`Building base image ${BASE_IMAGE_NAME} from ${dockerfilePath}...`
);
await execWithTimeout(
`docker build -t ${BASE_IMAGE_NAME} ${dockerfilePath}`,
{ timeout: 300000, retries: 1 } // 5 minutes timeout, 1 retry
);

baseImageBuilt = true;
console.log(`Base image ${BASE_IMAGE_NAME} built successfully`);
} catch (error) {
console.error('Error building base image:', error);
throw new Error('Failed to build base image');
}
}

/**
Expand Down Expand Up @@ -265,9 +310,9 @@ function execWithTimeout(
}

/**
* Build and run Docker container
* Run Docker container using the base image
*/
async function buildAndRunDocker(
async function runDockerContainer(
projectPath: string
): Promise<{ domain: string; containerId: string; port: number }> {
const traefikDomain = process.env.TRAEFIK_DOMAIN || 'docker.localhost';
Expand Down Expand Up @@ -307,25 +352,17 @@ async function buildAndRunDocker(
}
}

// Ensure base image exists
await ensureBaseImageExists();

const directory = path.join(getProjectPath(projectPath), 'frontend');
const subdomain = projectPath.replace(/[^\w-]/g, '').toLowerCase();
const imageName = subdomain;
const containerName = `container-${subdomain}`;
const domain = `${subdomain}.${traefikDomain}`;

// Allocate port
const exposedPort = await findAvailablePort();

// Remove node_modules and lock files
try {
await removeNodeModulesAndLockFiles(directory);
} catch (error) {
console.error(
'Error during cleanup phase, but will continue with build:',
error
);
}

try {
// Check if a container with the same name already exists, remove it if found
try {
Expand All @@ -342,22 +379,15 @@ async function buildAndRunDocker(
// If container doesn't exist, this will error out which is expected
}

// Build Docker image
console.log(
`Starting Docker build for image: ${imageName} in directory: ${directory}`
);
await execWithTimeout(
`docker build -t ${imageName} ${directory}`,
{ timeout: 300000, retries: 1 } // 5 minutes timeout, 1 retry
);

// Determine whether to use TLS or non-TLS configuration
const TLS = process.env.TLS === 'true';

// Configure Docker run command
let runCommand;
if (TLS) {
runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \
--memory=${memoryLimit} --memory-swap=${memoryLimit} \
--cpus=${cpusLimit} \
-l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \
-l "traefik.http.routers.${subdomain}.entrypoints=websecure" \
-l "traefik.http.routers.${subdomain}.tls=true" \
Expand All @@ -368,9 +398,11 @@ async function buildAndRunDocker(
-l "traefik.http.routers.${subdomain}.middlewares=${subdomain}-cors" \
--network=docker_traefik_network -p ${exposedPort}:5173 \
-v "${directory}:/app" \
${imageName}`;
${BASE_IMAGE_NAME}`;
} else {
runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \
--memory=${memoryLimit} --memory-swap=${memoryLimit} \
--cpus=${cpusLimit} \
-l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \
-l "traefik.http.routers.${subdomain}.entrypoints=web" \
-l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \
Expand All @@ -380,7 +412,7 @@ async function buildAndRunDocker(
-l "traefik.http.routers.${subdomain}.middlewares=${subdomain}-cors" \
--network=docker_traefik_network -p ${exposedPort}:5173 \
-v "${directory}:/app" \
${imageName}`;
${BASE_IMAGE_NAME}`;
}

// Run container
Expand Down Expand Up @@ -414,7 +446,7 @@ async function buildAndRunDocker(
);
return { domain, containerId: containerActualId, port: exposedPort };
} catch (error: any) {
console.error(`Error building or running container:`, error);
console.error(`Error running container:`, error);

// Clean up allocated port
allocatedPorts.delete(exposedPort);
Expand Down Expand Up @@ -499,15 +531,15 @@ export async function GET(req: Request) {
// Prevent duplicate builds
if (processingRequests.has(projectPath)) {
return NextResponse.json({
message: 'Build in progress',
message: 'Container creation in progress',
status: 'pending',
});
}

processingRequests.add(projectPath);

try {
const { domain, containerId } = await buildAndRunDocker(projectPath);
const { domain, containerId } = await runDockerContainer(projectPath);

return NextResponse.json({
message: 'Docker container started',
Expand Down
Loading
Loading