-
Notifications
You must be signed in to change notification settings - Fork 2
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
initial commit for docker build #2
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: jasmingacic <[email protected]>
@@ -0,0 +1,4 @@ | |||
PLATFORM?=linux/amd64 | |||
|
|||
build: |
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.
possible solution would be to run npm install here
build: | |
build: | |
npm install |
|
||
COPY . . | ||
|
||
RUN npm install |
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.
if makefile runs npm install then there is no need for this line
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.
The full version of node
is quite heavy, if we can get away with the alpine version, it would mean a much smaller docker image size. The only caveat is we may need to install git on the alpine version, as it does't come with that.
.dockerignore
Outdated
@@ -0,0 +1 @@ | |||
node_modules |
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.
also node_modules wouldn't have to be ignored if npm install is ran with make build
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.
Building within the docker environment has proven to be a good approach in other projects I've worked in, as it means the host only needs Docker installed. Helps with Ci/CD pipelinles, etc.
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.
There are a few things we can do to make a production ready docker image of this. I've scattered some notes on the conversations. And can find another nodejs project that we can still the docker set up from.
Thanks for kicking this off!
.dockerignore
Outdated
@@ -0,0 +1 @@ | |||
node_modules |
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.
Building within the docker environment has proven to be a good approach in other projects I've worked in, as it means the host only needs Docker installed. Helps with Ci/CD pipelinles, etc.
|
||
COPY . . | ||
|
||
RUN npm install |
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.
The full version of node
is quite heavy, if we can get away with the alpine version, it would mean a much smaller docker image size. The only caveat is we may need to install git on the alpine version, as it does't come with that.
|
||
RUN npm install | ||
|
||
CMD cat overlay.yaml | node bin.js |
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 can set the command to be bin.js
, that way it's possible to make use of stdin and we may not need to mount files. Although mounting files may prove useful later on.
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 was more of a suggestion on how to do it. I'm happy with any way you think would work the best. For example when we use this we are running command:
docker run --rm -v=${PWD}/overlay.yaml:/overlay.yaml -v=${PWD}/test.yaml:/test.yaml jasmingacic/overlay-cli
We can adjust to run it any other way.
And you are absolutely right about the docker image I used here. Current size is about 140MB. I'm not well versed with Node so I took the first thing I found.
Do you want me to add those changes?
Signed-off-by: jasmingacic <[email protected]>
This build effectively will enable this sort of use of the CLI
The idea is to be able to easily use overlay-cli in multiple platforms (linux, macos, win) and multiple cpu arch (arm, amd).
Ideally we could use vercel/pkg but that means packaging up to 6 different binaries into a single CLI app.
NOTE: While trying to build for platforms linux/arm/v7 llinux/arm64/v8 I found out that the official node image doesn't really work. Build was resulting in
Signed-off-by: jasmingacic [email protected]