Skip to content
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

refactor: Dockerfile and CI #69

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

pratikbin
Copy link

@pratikbin pratikbin commented Dec 18, 2021

Overview

  • Prod image from scratch base
  • Use docker-bake and buildx for building parallel multi stage docker image with better caching support with gha
  • Use -s -w ldflags and trimpath more here
  • Use UPX binary compression, more here
  • Create /tmp dir as its necessaryfor rover
  • Copy certs for terraform
  • Add npm build as one testing workflow

go build = 24MB
-rwxr-xr-x 1 pratik pratik 17596884 Dec 18 19:28 rover

go build -trimpath -ldflags "-s -w" = 19M
-rwxr-xr-x 1 pratik pratik 19542016 Jan 14 19:43 rover

go build -trimpath -ldflags "-s -w" + UPX = 4.8M
-rwxr-xr-x 1 pratik pratik 4999224 Jan 14 19:43 rover

Terraform + UPX = ~13 MB from

                       Ultimate Packer for eXecutables
                          Copyright (C) 1996 - 2020
UPX 3.96        Markus Oberhumer, Laszlo Molnar & John Reiser   Jan 23rd 2020

        File size         Ratio      Format      Name
   --------------------   ------   -----------   -----------
  19542016 ->   4999224   25.58%   linux/amd64   rover                         

Packed 1 file.

Terraform + UPX = 60M to 13MB (part of slim container image)

                       Ultimate Packer for eXecutables
                          Copyright (C) 1996 - 2020
UPX 3.96        Markus Oberhumer, Laszlo Molnar & John Reiser   Jan 23rd 2020

        File size         Ratio      Format      Name
   --------------------   ------   -----------   -----------
  61956096 ->  12771252   20.61%   linux/amd64   terraform                     


May the source be with you

- Build image from scratch
- Copy terraform from its image
- Use `-s -w` ldflags with trimpath to build rover with least size (14M)
- Use UPX to compress the binray without any performance penalty that make binary size (4.3M)
- Change rover path as terraform's
- Create `/tmp` dir as its needed by rover
- Copy certs for terraform
@im2nguyen
Copy link
Owner

Thanks for submitting this PR!

I have spotty internet for this week and don't have access to my laptop, but I'll review and merge once I get the chance

Wanted to express how much I appreciate your work and I'm not just ignoring it 😄

@pratikbin
Copy link
Author

@im2nguyen Do you have any plans for multi platform container images? I can contribute in that

@im2nguyen
Copy link
Owner

Hey @pratikbalar, what do you mean by multiplatform container images?

@pratikbin
Copy link
Author

Hey @pratikbalar, what do you mean by multiplatform container images?

Okay i don't know if you know but as multiple architecture binaries we are generating in release, we can also create multiple architecture container images

You can see golang docker hub image

Screenshot_20211229-105026__01.jpg

E.g. let say if we pull golang:1.17, docker running in raspberry pi will automatically pull arm version of container tag and like so for other platforms available

More here

@im2nguyen
Copy link
Owner

Oh that's neat! Multi-arch builds would be a neat addition!

@pratikbin
Copy link
Author

pratikbin commented Jan 3, 2022

See, I'm using UPX for binary compression, so i would suggest maintaining UPXed version with slim postfix separately. What do you think?

@pratikbin
Copy link
Author

Do you want to try ? In your local ?

@im2nguyen
Copy link
Owner

Hello! sorry i got back to you so late, can you provide instructions on how I should test locally?

@pratikbin
Copy link
Author

I can see some changes in Dockerfile and all in main branch, so will update you here once I confirm everything

@pratikbin
Copy link
Author

@im2nguyen Do you think that image generation feature worth 340+MB in uncompressed image?

@im2nguyen
Copy link
Owner

That's a good question. I think we can make it optional?

One with image gen (chromium), the other one with it?

What do you think @pratikbalar?

@pratikbin
Copy link
Author

One with image gen (chromium), the other one with it?

Umm, I'm not with it. Because that will start to make confusion, and then we'll add slim variant, Also chromium is only available for arm64 and amd64 architectures. So I guess we have to find some other way to get generate image(picture).

https://pkgs.alpinelinux.org/packages?name=chromium&branch=edge
image

@pratikbin
Copy link
Author

pratikbin commented Jan 12, 2022

@im2nguyen Well I talked to my friends, They haven't used these but based on their knowledge they suggested me ⬇️

https://github.com/chromedp/chromedp
https://github.com/sensepost/gowitness

let me know if you want me to involve more

Plus, one of my friend also suggested running ngrok and querying public API to create screenshot of it.

@im2nguyen
Copy link
Owner

It's already using chromedp to access Chrome headlessly. The problem is that it still needs access Chrome to exists in the environment.

This looks like a promising solution. https://hub.docker.com/r/chromedp/headless-shell/

Could we use this as a base and move the Terraform and rover binary?

@im2nguyen
Copy link
Owner

Always appreciate contributions 😄

Just let me know how I should test your changes. If it benefits the product for everyone (it does in this case), I'll be more than happy to merge

@pratikbin
Copy link
Author

Could we use this as a base and move the Terraform and rover binary?

Can you give me little time for this 😄

Just let me know how I should test your changes. If it benefits the product for everyone (it does in this case), I'll be more than happy to merge

I haven't merge chrome changes, that's why. Let me push current changes.

@pratikbin
Copy link
Author

Also, do you/we want to maintain Makefile for all ?

@pratikbin
Copy link
Author

pratikbin commented Jan 14, 2022

Moved to first comment

@im2nguyen
Copy link
Owner

Nice! Is there a way for me to test this locally?

@pratikbin
Copy link
Author

Well, I don't know if you noticed, but I've edited README Yesterday
see https://github.com/pratikbalar/rover/tree/feat/ci-and-dockerfile#container-image

@pratikbin
Copy link
Author

@im2nguyen should I separate all the changes in different PR ? And have you tried, -genImage I think it's working as expected without chrome drivers.

Copy link
Owner

@im2nguyen im2nguyen left a comment

Choose a reason for hiding this comment

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

Hello! Sorry, I was finally able to get to it. While testing it, I had trouble running the docker buildx bake command, can you help me through it?

Also, I included notes about moving the Docker build instruction to its own dedicated page!

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Comment on lines +170 to +171
## Create local image
docker buildx bake
Copy link
Owner

Choose a reason for hiding this comment

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

I received the following error while building. Can you look into it? Thank you!

$ docker buildx bake
[+] Building 0.0s (0/0)                                                                                                                                       
docker-bake.hcl:27
--------------------
  25 |     variable "TAGS" {
  26 |       default = [
  27 | >>>     "${REPO}:latest",
  28 |         "${REPO}:${VERSION}",
  29 |         "${REPO}:${VERSION}-${GIT_SHA}"
--------------------
error: docker-bake.hcl:27,8-12: Variables not allowed; Variables may not be used here., and 14 other diagnostic(s)

Copy link
Owner

Choose a reason for hiding this comment

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

these are my current docker specs. might be good to include this as prereqs for folks that are interested in building this locally

$ docker version
Client:
 Cloud integration: 1.0.14
 Version:           20.10.6
 API version:       1.41
 Go version:        go1.16.3
 Git commit:        370c289
 Built:             Fri Apr  9 22:46:57 2021
 OS/Arch:           darwin/amd64
 Context:           default
 Experimental:      true

Server: Docker Engine - Community
 Engine:
  Version:          20.10.6
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.13.15
  Git commit:       8728dd2
  Built:            Fri Apr  9 22:44:56 2021
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.4.4
  GitCommit:        05f951a3781f4f2c1911b05e61c160e9c30eaa8e
 runc:
  Version:          1.0.0-rc93
  GitCommit:        12644e614e25b05da6fd08a38ffa0cfe1903fdec
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Copy link
Author

@pratikbin pratikbin Jan 25, 2022

Choose a reason for hiding this comment

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

Can you docker buildx version?
Mine is,
github.com/docker/buildx v0.7.1-docker 05846896d149da05f3d6fd1e7770da187b52a247

Copy link
Author

Choose a reason for hiding this comment

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

I think this is due to docker version. Mine is,

$ docker version        
Client:
 Version:           20.10.12
 API version:       1.41
 Go version:        go1.17.5
 Git commit:        e91ed5707e
 Built:             Mon Dec 13 22:31:40 2021
 OS/Arch:           linux/amd64
 Context:           builders
 Experimental:      true

Server:
 Engine:
  Version:          20.10.12
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.17.5
  Git commit:       459d0dfbbb
  Built:            Mon Dec 13 22:30:43 2021
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          v1.5.8
  GitCommit:        1e5ef943eb76627a6d3b6de8cd1ef6537f393a71.m
 runc:
  Version:          1.0.3
  GitCommit:        v1.0.3-0-gf46b6ba2
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Can you try upgrading your docker version if that won't break any of your existing projects.

pratikbin and others added 4 commits January 25, 2022 10:37
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
@pratikbin
Copy link
Author

And I don't know why, but now I'm getting this error on npm build

=> ERROR [ui 7/7] RUN npm run build                                                                                                                                                                           15.2s
------                                                                                                                                                                                                               
 > [ui 7/7] RUN npm run build:                                                                                                                                                                                       
#31 0.360                                                                                                                                                                                                            
#31 0.360 > ui@0.1.0 build
#31 0.360 > vue-cli-service build
#31 0.360 
#31 0.880 
#31 0.880 -  Building for production...
#31 15.16  ERROR  Failed to compile with 1 error5:14:12 AM
#31 15.16 
#31 15.16  error  in ./src/components/ResourceDetail.vue?vue&type=script&lang=js&
#31 15.16 
#31 15.16 Module parse failed: Unexpected token (182:46)
#31 15.16 File was processed with these loaders:
#31 15.16  * ./node_modules/cache-loader/dist/cjs.js
#31 15.16  * ./node_modules/thread-loader/dist/cjs.js
#31 15.16  * ./node_modules/babel-loader/lib/index.js
#31 15.16  * ./node_modules/cache-loader/dist/cjs.js
#31 15.16  * ./node_modules/vue-loader/lib/index.js
#31 15.16 You may need an additional loader to handle the result of these loaders.
#31 15.16 | 
#31 15.16 |     getResourceConfig(resourceID, model, isChild) {
#31 15.16 >       let configID = model.states[resourceID]?.config_id ? model.states[resourceID]?.config_id : resourceID.replace(/\[[^[\]]*\]/g, "");
#31 15.16 |       let config;
#31 15.16 |       if (isChild) return {
#31 15.16 
#31 15.16  @ ./src/components/ResourceDetail.vue?vue&type=script&lang=js& 1:0-328 1:344-347 1:349-674 1:349-674
#31 15.16  @ ./src/components/ResourceDetail.vue
#31 15.16  @ ./node_modules/cache-loader/dist/cjs.js??ref--13-0!./node_modules/thread-loader/dist/cjs.js!./node_modules/babel-loader/lib!./node_modules/cache-loader/dist/cjs.js??ref--1-0!./node_modules/vue-loader/lib??vue-loader-options!./src/App.vue?vue&type=script&lang=js&
#31 15.16  @ ./src/App.vue?vue&type=script&lang=js&
#31 15.16  @ ./src/App.vue
#31 15.16  @ ./src/main.js
#31 15.16  @ multi ./src/main.js
#31 15.16 
#31 15.16  ERROR  Build failed with errors.
------
Dockerfile:12
--------------------
  10 |     COPY ./ui/public ./public
  11 |     COPY ./ui/src ./src
  12 | >>> RUN npm run build
  13 |     
  14 |     FROM --platform=$BUILDPLATFORM alpine:3.15 as terraform
--------------------
error: failed to solve: process "/bin/sh -c npm run build" did not complete successfully: exit code: 1

@im2nguyen
Copy link
Owner

Hey @pratikbalar, that was due to recent changes that included optional chaining.

Did you pull + merge from the main branch? I added this file to babel to address a while back (

plugins: ['@babel/plugin-proposal-optional-chaining']
)

@pratikbin
Copy link
Author

pratikbin commented Jan 26, 2022

@im2nguyen Yeah, I've pull + merged main. Still getting same error.

For this kind of situations, I've created a npm build as a testing workflow for which will only triggers on changes in ui/ directory

@im2nguyen
Copy link
Owner

Was it able to resolve it?

@pratikbin
Copy link
Author

And I don't know why, but now I'm getting this error on npm build

=> ERROR [ui 7/7] RUN npm run build                                                                                                                                                                           15.2s
------                                                                                                                                                                                                               
 > [ui 7/7] RUN npm run build:                                                                                                                                                                                       
#31 0.360                                                                                                                                                                                                            
#31 0.360 > ui@0.1.0 build
#31 0.360 > vue-cli-service build
#31 0.360 
#31 0.880 
#31 0.880 -  Building for production...
#31 15.16  ERROR  Failed to compile with 1 error5:14:12 AM
#31 15.16 
#31 15.16  error  in ./src/components/ResourceDetail.vue?vue&type=script&lang=js&
#31 15.16 
#31 15.16 Module parse failed: Unexpected token (182:46)
#31 15.16 File was processed with these loaders:
#31 15.16  * ./node_modules/cache-loader/dist/cjs.js
#31 15.16  * ./node_modules/thread-loader/dist/cjs.js
#31 15.16  * ./node_modules/babel-loader/lib/index.js
#31 15.16  * ./node_modules/cache-loader/dist/cjs.js
#31 15.16  * ./node_modules/vue-loader/lib/index.js
#31 15.16 You may need an additional loader to handle the result of these loaders.
#31 15.16 | 
#31 15.16 |     getResourceConfig(resourceID, model, isChild) {
#31 15.16 >       let configID = model.states[resourceID]?.config_id ? model.states[resourceID]?.config_id : resourceID.replace(/\[[^[\]]*\]/g, "");
#31 15.16 |       let config;
#31 15.16 |       if (isChild) return {
#31 15.16 
#31 15.16  @ ./src/components/ResourceDetail.vue?vue&type=script&lang=js& 1:0-328 1:344-347 1:349-674 1:349-674
#31 15.16  @ ./src/components/ResourceDetail.vue
#31 15.16  @ ./node_modules/cache-loader/dist/cjs.js??ref--13-0!./node_modules/thread-loader/dist/cjs.js!./node_modules/babel-loader/lib!./node_modules/cache-loader/dist/cjs.js??ref--1-0!./node_modules/vue-loader/lib??vue-loader-options!./src/App.vue?vue&type=script&lang=js&
#31 15.16  @ ./src/App.vue?vue&type=script&lang=js&
#31 15.16  @ ./src/App.vue
#31 15.16  @ ./src/main.js
#31 15.16  @ multi ./src/main.js
#31 15.16 
#31 15.16  ERROR  Build failed with errors.
------
Dockerfile:12
--------------------
  10 |     COPY ./ui/public ./public
  11 |     COPY ./ui/src ./src
  12 | >>> RUN npm run build
  13 |     
  14 |     FROM --platform=$BUILDPLATFORM alpine:3.15 as terraform
--------------------
error: failed to solve: process "/bin/sh -c npm run build" did not complete successfully: exit code: 1

Still getting this same error on npm build

@GeorgeDavis-TriumphTech

This is great! Thank you @im2nguyen @pratikbin.

Looking forward to seeing this PR come live and building Docker images using GitHub Actions in a regular cadence. Happy to help with the testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants