Skip to content

Build webapp assets during Docker build instead of checking them into git#611

Open
benoit-nexthop wants to merge 1 commit intocloudbase:mainfrom
nexthop-ai:remove-webapp-assets-from-git
Open

Build webapp assets during Docker build instead of checking them into git#611
benoit-nexthop wants to merge 1 commit intocloudbase:mainfrom
nexthop-ai:remove-webapp-assets-from-git

Conversation

@benoit-nexthop
Copy link
Contributor

This closes #609

  • Add webapp build stage to Dockerfile and Dockerfile.build-static using node:lts-alpine
  • Move build-webapp.sh to scripts/ directory
  • Add .dockerignore to exclude node_modules and reduce build context
  • Update .gitignore to exclude compiled webapp assets
  • Remove compiled webapp assets from repository, keeping it up to date was a constant source of conflict and doing this was bad practice generally anyway

… git

- Add webapp build stage to Dockerfile and Dockerfile.build-static using node:lts-alpine
- Move build-webapp.sh to scripts/ directory
- Add .dockerignore to exclude node_modules and reduce build context
- Update .gitignore to exclude compiled webapp assets
- Remove compiled webapp assets from repository
Copy link
Member

@gabriel-samfira gabriel-samfira left a comment

Choose a reason for hiding this comment

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

I've been meaning to address this for a while, but have had very little time lately. So thanks!

Just a few comments for the build-static bit. Let me know what you think

@@ -1,3 +1,13 @@
# Stage 1: Build webapp
Copy link
Member

Choose a reason for hiding this comment

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

This Dockerfile was initially written to be built once and reused many times by just creating a new temporary container from it and running the build-static.sh script. But given that we might need to also build and copy the webapp now, I think we can expand it a bit more and just run docker build to get the static binary. See bellow comments.

# Copy webapp build output to assets directory
COPY --from=webapp /build/webapp/build/ /build/garm/webapp/assets/

CMD ["/bin/sh"]
Copy link
Member

Choose a reason for hiding this comment

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

We can replace this with

Suggested change
CMD ["/bin/sh"]
ARG GARM_REF=""
ENV GARM_REF=${GARM_REF}
RUN /build-static.sh
# Stage 3: Export only the binaries
FROM scratch AS export
COPY --from=builder /build/output/ /
CMD ["/bin/sh"]

See the Makefile comment for the last bit.

##@ Build

.PHONY : build-static test install-lint-deps lint go-test fmt fmtcheck verify-vendor verify create-release-files release
build-static: ## Build garm statically
Copy link
Member

Choose a reason for hiding this comment

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

Then this section can be replaced with something like:

build-static: ## Build garm statically
	@echo Building garm
	mkdir -p build
	$(IMAGE_BUILDER) build $(EXTRA_ARGS) \
		--build-arg GARM_REF=$(GARM_REF) \
		--output type=local,dest=$(PWD)/build \
		--target=export \
		-f Dockerfile.build-static .
	@echo Binaries are available in $(PWD)/build

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.

Don't check in the generated UI code

2 participants