-
-
Notifications
You must be signed in to change notification settings - Fork 194
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.
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
Build arguments not replaced #735
Comments
Hi @nicolabovolato,
The code link you shared is unrelated. That code parses a list of Docker image names which are later used for authentication. We need the build arguments there just in case the image name is a build arg. The build arguments relevant for you are sent directly to the Docker daemon here: testcontainers-node/packages/testcontainers/src/generic-container/generic-container-builder.ts Lines 65 to 74 in c514be0
Could you please share what exactly is going wrong with your implementation, perhaps share the logs as well? |
My bad @cristianrgreco, i should have looked a bit further into that function. Anyway, I'm running containerized benchmarks inside a monorepo. const workspaceName = `bench-${group}-${pkg}`;
const workspacePath = path.relative(
rootDir,
path.join(__dirname, group, pkg)
);
logger.info(
`Building dockerfile with workspace ${workspaceName} and path ${workspacePath}...`
);
const container = await GenericContainer.fromDockerfile(
rootDir,
dockerfilePath
)
.withBuildArgs({
WORKSPACE_NAME: workspaceName,
WORKSPACE_PATH: workspacePath,
})
.build(workspaceName)
.withResourcesQuota({
cpu: containerCpu,
memory: containerMemory,
});
const startedContainer = await container.start();
logger.info("Running bench for package " + pkg);
const results = await benchmark(group, startedContainer);
await startedContainer.stop(); Logs are as follows:
|
@nicolabovolato I don't necessarily see an issue with the build arguments here. What I do see is this error:
Address this issue first and then we'll see if there's an issue with the build arguments. |
@cristianrgreco building directly with the docker command works. You can see from the logs above that the same build arguments are passed to the testcontainer. |
Simpler reproduction https://stackblitz.com/edit/stackblitz-starters-ab6hdh?file=index.ts Looks like args in multi stage builds are not propagated correctly, perhaps this is something unrelated to testcontainers. |
Hey @nicolabovolato, docker documentation states that build args last until the end of the stage where they were declared - see here https://docs.docker.com/reference/dockerfile/#scope. You declare |
Looks like global build args are a docker build kit feature - https://medium.com/@sujaypillai/globally-scoped-platform-args-in-docker-buildkit-787b9010bbb8, so it's related to #571 |
Thanks for looking into this @silh, indeed lack of build kit support seems to be the issue. |
Expected Behaviour
Having a Dockerfile containing build arguments, I expected them to be replaced when using
withBuildArgs()
onGenericContainerBuilder
E.g. dockerfile for nodejs monorepos.
Actual Behaviour
Looking at the source code, build arguments are only getting replaced if present in the FROM clause.
Also that regex doesn't look right.
testcontainers-node/packages/testcontainers/src/utils/dockerfile-parser.ts
Lines 6 to 28 in 9a41b92
Steps to Reproduce
Try to programmatically build an image with a build arg placed everywhere but in the FROM clause.
Environment Information
The text was updated successfully, but these errors were encountered: