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

Feat: extend Docker Support #1142

Open
wants to merge 7 commits into
base: v4
Choose a base branch
from

Conversation

almereyda
Copy link

@almereyda almereyda commented May 16, 2024

These commits package Quartz as a container here at the GitHub container registry when a new tag is pushed:

The CI workflow is prepared to also push to Docker Hub. There is also a workflow dispatch trigger to create manual builds. The configuration could be parametrised more in the future.

The Dockerfile has been minimally modified to conform to the OCI standard with regards to the entrypoint and its command, plus has been extended with the missing git dependency to be able to use the build command.

The documentation has been extended with some examples on how to use the image.

The documentation expects the image to be pushed to ghcr.io/jackyzha0/quartz.

@almereyda
Copy link
Author

An exemplary pipeline run and artifact were generated into:

@almereyda
Copy link
Author

almereyda commented May 16, 2024

This image also supports running in GitLab CI to push into GitLab Pages:

image:
  name: ghcr.io/allmende/quartz:v4
  entrypoint: [""]

pages:
  rules:
    - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH

  stage: deploy
  environment: live

  before_script:
    - cd /usr/src/app
    - mkdir -p public

  script:
    - cp $CI_PROJECT_DIR/quartz.*.ts .
    - npx quartz build -d $CI_PROJECT_DIR/content
    - mv public $CI_PROJECT_DIR

  artifacts:
    paths:
      - public

The minimal example for the file system layout of a content repository without including a whole fork of Quartz looks like this:

.
├── content
│   └── index.md
├── quartz.config.ts
└── quartz.layout.ts

@bachrc
Copy link
Contributor

bachrc commented May 19, 2024

there seem to be some issues with the linting, you can see the action logs

@almereyda
Copy link
Author

I've pushed a commit to fix the linter issues, which seem to be resolved.

While there is no definition of a content repository for quartz, we have updated our patterns.

.
├── .quartz
│   ├── quartz
│   │   └── components
│   │       ├── Backlinks.tsx
│   │       └── Footer.tsx
│   ├── quartz.config.ts
│   └── quartz.layout.ts
├── content
│   └── index.md
└── .gitlab-ci.yml

The CI needs to be adapted as well:

image:
  name: ghcr.io/allmende/quartz:v4
  entrypoint: [""]

pages:
  rules:
    - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH

  stage: deploy
  environment: live

  before_script:
    - cd /usr/src/app
    - mkdir -p public

  script:
    - cp -R $CI_PROJECT_DIR/.quartz/* .
    - npx quartz build -d $CI_PROJECT_DIR/content
    - mv public $CI_PROJECT_DIR

  artifacts:
    paths:
      - public

By using the dot pattern for the Quartz configuration, we can cleanly separate the concerns of content and configuration and can still apply a custom overlay, e.g. here also for certain components.

docker build --load -t ghcr.io/jackyzha0/quartz .
```

This example expects a `content/` directory with the `quartz.*.ts` configuration files and a (empty) `public/` directory to be present in the preesent working directory.
Copy link
Owner

Choose a reason for hiding this comment

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

can you illustrate the working directory structure here?

Copy link
Author

Choose a reason for hiding this comment

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

Good suggestion.

My exemplary way to use this container has also changed a bit, as illustrated in my previous comment. I will update the file.

Copy link
Author

Choose a reason for hiding this comment

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

I have now added 3b58b2f to address your comment.

From your perspective, does the explanation still lack suitable examples to work with this image?

docs/features/Docker Support.md Outdated Show resolved Hide resolved
uses: docker/build-push-action@v5
with:
push: true
platforms: linux/amd64
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can possibly build for multiple platform here, since quartz has been battle tested on Linux and Darwin.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks from looking into this. Should be a trivial change to add more targets here.

Which do you have in mind? linux/arm64 is the only other notable one I could think of for containers.

How would one build a container for Darwin?

Copy link
Collaborator

Choose a reason for hiding this comment

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

linux/arm64, darwin/arm64, maybe even most of the node covered architecture.

@@ -5,7 +5,13 @@ COPY package-lock.json* .
RUN npm ci

FROM node:20-slim

RUN apt update ; \
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should use mount cache here. see https://docs.docker.com/build/guide/mounts/

Copy link
Author

Choose a reason for hiding this comment

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

Let me investigate and add a patch based on what I can come up with.

@aarnphm
Copy link
Collaborator

aarnphm commented Oct 7, 2024

hi there, since #1192 we have something similar to what you propose here?

can you maybe update this PR to include docs update as well as Dockerfile improvement instead?

One thing is that with the main quartz container users can do something like this

docker run --rm --platform linux/amd64 -p 8080:8080 \
	-v /path/to/vault:/usr/src/app/content ghcr.io/jackyzha0/quartz:latest

@almereyda
Copy link
Author

I hadn't seen #1192, thanks for the update. Will need to look into it.

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.

4 participants