-
Notifications
You must be signed in to change notification settings - Fork 11
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
Allow the default docker command that packages gems to be overriden #54
base: master
Are you sure you want to change the base?
Conversation
Using env variable and include more debug information.
Is there any way for the code to detect this situation and adapt for you? The proposed solution requires the user to have intimate details of the plugin's implementation details. By copying & pasting the current default command to make a couple tweaks, the user is "freezing" the command into their own codebase, and will miss out on any changes to the plugin. |
${dockerImage} \ | ||
'/var/task/node_modules/serverless-ruby-package/build-gems.sh'` | ||
const default_command = `docker run --rm \ | ||
--volume "${localPath}:/var/task" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A more targeted but less flexible change would be to allow ENV var to override the localPath
here. In our case:
- build host: a CI worker node. Mount is something like
/var/build/<project_checkout>:/app
- build container: the docker container launched by the host with all the dependencies for a serverless ruby project that has the project checkout mounted to
/app
and runs the serverlesspackage
command - native extension container: the docker container spawned by
serverless-ruby-package
inside the build container. Here,localPath
is evaluating to/app
but since the build container is sharing the docker socket with the build host,localPath
needs to be a path to where the project is on the build host. So the mount here should be/var/build/<project_checkout>:/var/task
and not/app:/var/task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to also include documentation for docker-in-docker in the readme. Many CI/CD systems will use a docker-in-docker approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a reasonable approach, if we are unable to detect the "docker in docker" situation and determine the volume mount automatically.
1942b9b
to
b18a29a
Compare
Using env variable and include more debug information. This was necessary for us at onramp as we run our builds inside docker containers. When we started having native dependencies, this resulted in the
serverless-ruby-package
plugin attempting to run docker containers inside docker containers.Docker in docker is a known and used operational pattern and can be enabled by mounting the docker socket from the host inside the container that will launch new containers. A side effect of this, however, is that mounts to the innermost container are not happening relative to its immediate parent container that spawned it. Rather, it needs to be against the filesystem on the outermost container host.
So the default command of:
Works if you are running on a top level container on a local development machine, but NOT on a nested container. This allows the above command to be overriden with the
SRP_DOCKER_COMMAND
env variable as we are here in our particular project:Unit Tests
Integration Tests