-
Notifications
You must be signed in to change notification settings - Fork 29
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
HDDS-9568. Add docusaurus build check in github actions #71
Conversation
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.
Thanks for working on this @adoroszlai
This implementation uses a mix of dependencies installed in the docker image and in the runner, which I think is a bit confusing (correct me if I'm reading this wrong):
docker build -t ozone-site:ci .
This installs all javascript dependencies into the image when building it.
docker run --rm -v $PWD:/ozone-site ozone-site:ci pnpm install
This mounts the current directory over the ozone-site
directory in the image, covering the previously installed dependencies in /ozone-site/node_modules
but using pnpm from the image. The dependencies here are installed into the runner directly so now we have two copies.
docker run --rm -v $PWD:/ozone-site ozone-site:ci pnpm build
This executes the build using pnpm from the image and dependencies from the runner. Dependencies installed into the image are not used.
I think we should use pnpm and the dependencies from the image to do the build, but leave the build output in the runner with the volume mount. We can do this with the existing compose file:
docker commpose build
: Optional, sincerun
will build the image anyways. Making this its own step might make for clearer CI messages though.docker compose run site pnpm build
: Builds the site using all dependencies in the image, but leaves the final build artifacts in the runner.- (Future PR) If this is a push to the
HDDS-9225-website-v2
branch, re-use the pre-built artifacts in the commit step to the publishing branch. - (Future PR) Liveness check for the build:
docker compose run --service-ports site pnpm serve
followed bycurl
to check that the homepage is returned. Yunikorn has a similar check
Thanks @errose28 for the explanation on how to use compose to build/run. I had a feeling that my steps are too complex. |
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.
Thanks for the updates @adoroszlai LGTM
* HDDS-9225-website-v2: HDDS-10255. Update Release Manager Guide after importing it to the new website. (apache#67) HDDS-9568. Add docusaurus build check in github actions (apache#71) HDDS-10266. Auto label PRs for the new website. (apache#68) HDDS-10267. Add PR template for the new website. (apache#70) HDDS-9603. Add PR title check (apache#69) HDDS-9564. Create Ozone Social Card. (apache#62) HDDS-10233. Apache Release Manager Guide (apache#66) HDDS-10203. Update Github actions to v4 since Node.js 16 actions are deprecated. (apache#60) HDDS-9612. Add CONTRIBUTING.md for the new Ozone website (apache#54) HDDS-9865. Add outline of docs sections and pages. (apache#53) HDDS-9926. Publish WIP website to staging branch (2/2) (apache#56) Conflicts: docs/01-overview.md docs/03-core-concepts/04-security/01-authentication/01-kerberos.md docs/05-administrator-guide/02-configuration/03-security/01-kerberos.md progress.sh
What changes were proposed in this pull request?
Add smoketest that builds the website in CI.
https://issues.apache.org/jira/browse/HDDS-9568
How was this patch tested?
Introduced broken link, verified the same steps fail with:
CI:
https://github.com/adoroszlai/ozone-site/actions/runs/7740784527/job/21106463270