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

[YUNIKORN-2481] Convert yunikorn-site to use pnpm #409

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,8 @@
# Ignore files to keep site clean
Dockerfile
README.md
*.swp
**/node_modules/
**/.pnpm-store/
local-build.sh
check_license.sh
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ package-lock.json
yarn.lock
_site/
.docusaurus/
.pnpm-store/
node_modules/
build/
12 changes: 7 additions & 5 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
ARG NODE_VERSION=
FROM node:${NODE_VERSION}

ADD . /yunikorn-site
WORKDIR /yunikorn-site
RUN npm install -g pnpm
COPY pnpm-lock.yaml /yunikorn-site
RUN pnpm fetch

RUN yarn install
RUN yarn add @docusaurus/theme-search-algolia
RUN yarn build
ENTRYPOINT yarn start --host 0.0.0.0
COPY . /yunikorn-site
RUN pnpm install -r --offline
RUN pnpm build
ENTRYPOINT pnpm start --host 0.0.0.0
12 changes: 5 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,16 @@ For an overview of all options of the local build script run:
```

## Local build
Instead of running the build inside a docker image you can also run it locally when you have yarn installed.
Instead of running the build inside a docker image you can also run it locally when you have pnpm installed.
This is faster than running the build inside a Docker image:
```shell script
yarn install
yarn add @docusaurus/theme-search-algolia
yarn build
pnpm install
pnpm build
```
Run the website server locally in development mode with the following command:
```shell script
yarn install
yarn add @docusaurus/theme-search-algolia
yarn run start
pnpm install
pnpm run start
```

## Updating the site
Expand Down
4 changes: 2 additions & 2 deletions check_license.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ OS=$(uname -s | tr '[:upper:]' '[:lower:]')
echo "checking license headers:"
# run different finds on mac vs linux
if [ "${OS}" = "darwin" ]; then
find -E . ! -path "./.git*" ! -path "./.docusaurus*" ! -path "./node_modules*" -regex ".*(Dockerfile|\.(js|sh|md|conf|yaml|yml|html|css))" -exec grep -L "Licensed to the Apache Software Foundation" {} \; > LICRES
find -E . ! -path "./.git*" ! -path "./.docusaurus*" ! -path "./node_modules*" ! -path ./pnpm-lock.yaml -regex ".*(Dockerfile|\.(js|sh|md|conf|yaml|yml|html|css))" -exec grep -L "Licensed to the Apache Software Foundation" {} \; > LICRES
else
find . ! -path "./.git*" ! -path "./.docusaurus*" ! -path "./node_modules*" -regex ".*\(Dockerfile\|\.\(js\|sh\|md\|conf\|yaml\|yml\|html\|css\)\)" -exec grep -L "Licensed to the Apache Software Foundation" {} \; > LICRES
find . ! -path "./.git*" ! -path "./.docusaurus*" ! -path "./node_modules*" ! -path ./pnpm-lock.yaml -regex ".*\(Dockerfile\|\.\(js\|sh\|md\|conf\|yaml\|yml\|html\|css\)\)" -exec grep -L "Licensed to the Apache Software Foundation" {} \; > LICRES
fi
# any file mentioned in the output is missing the license
if [ -s LICRES ]; then
Expand Down
25 changes: 9 additions & 16 deletions local-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ function clean() {
echo "removing build artifacts:"
echo " docusaurus install" && rm -rf .docusaurus
echo " node modules" && rm -rf node_modules
echo " yarn lock file" && rm -f yarn.lock
echo " build output" && rm -rf build
}

Expand All @@ -44,8 +43,12 @@ function image_build() {
# build local docker image
cat <<EOF >.dockerfile.tmp
FROM node:${NODE_VERSION}
ADD . /yunikorn-site
WORKDIR /yunikorn-site
RUN npm install -g pnpm
COPY pnpm-lock.yaml /yunikorn-site
RUN pnpm fetch
COPY . /yunikorn-site
RUN pnpm install -r --offline
EOF

if ! docker build -t yunikorn/yunikorn-website:latest -f .dockerfile.tmp .; then
Expand All @@ -59,7 +62,7 @@ EOF
function web() {
# start the web server
echo " Starting development server with locale $LOCALE"
docker exec -it yunikorn-site-local /bin/bash -c "yarn start --locale=$LOCALE --host 0.0.0.0"
docker exec -it yunikorn-site-local /bin/bash -c "pnpm start --locale=$LOCALE --host 0.0.0.0"
RET=$?
[ ${RET} -eq 131 ] && echo " ctrl-\ caught, restarting" && return 2
[ ${RET} -eq 130 ] && echo " ctrl-c caught, exiting build" && return 0
Expand All @@ -73,23 +76,13 @@ function run_base() {
if ! docker run -it --name yunikorn-site-local -d \
-p 3000:3000 \
-v "$PWD":/yunikorn-site \
--mount type=volume,dst=/yunikorn-site/node_modules \
Copy link
Contributor

Choose a reason for hiding this comment

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

It isolated the host's and docker's node_modules/ .
Could I confirm what is the previous weirdness in the old build? I wasn't aware of it before.

Most of the weirdness in the old build was due to node_modules being (incorrectly) shared between the host and the Docker environment.

Copy link
Contributor

@chenyulin0719 chenyulin0719 Mar 17, 2024

Choose a reason for hiding this comment

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

I think it would be better if we can explain the reason under run_base()'s comments.
ex:

function run_base() {
  # run docker container
  # mount the repo root to the container working dir,
  # so that changes made in the repo can trigger the web-server auto-update
  # Also prevent sharing node_modules/ between the host and the Docker environment to avoid potential conflict.

Copy link
Contributor Author

@craigcondit craigcondit Mar 17, 2024

Choose a reason for hiding this comment

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

Docker behavior is well documented already and I'd rather not clutter a simple function with a large comment here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It isolated the host's and docker's node_modules/ .

Could I confirm what is the previous weirdness in the old build? I wasn't aware of it before.

Most of the weirdness in the old build was due to node_modules being (incorrectly) shared between the host and the Docker environment.

We had all sorts of strange brokenness around algolia search having to be reinstalled constantly. I suspect there was platform-specific code being added and this caused issues when node_modules was shared between different platforms.

yunikorn/yunikorn-website:latest; then

echo "run local docker image failed"
return 1
fi

# install dependency in docker container
if ! docker exec -it yunikorn-site-local /bin/bash -c "yarn install"; then
echo "yarn install failed"
return 1
fi

# install dependency in docker container
if ! docker exec -it yunikorn-site-local /bin/bash -c "yarn add @docusaurus/theme-search-algolia"; then
echo "yarn add failed"
return 1
fi
return 0
}

Expand All @@ -105,8 +98,8 @@ function run_web() {

function run_build() {
# run build inside the container
if ! docker exec -it yunikorn-site-local /bin/bash -c "yarn build"; then
echo "yarn build failed"
if ! docker exec -it yunikorn-site-local pnpm build; then
echo "pnpm build failed"
return 1
fi
return 0
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"version": "2.0.0",
"private": true,
"scripts": {
"preinstall": "npx only-allow pnpm",
"start": "docusaurus start",
"build": "docusaurus build",
"swizzle": "docusaurus swizzle",
Expand Down
Loading