Skip to content

Conversation

@chalin
Copy link
Contributor

@chalin chalin commented Nov 16, 2024

In addition to fixing the issue named below, this PR has the container use the project's specified NPM dependencies, rather than using a separate list, keeping things DRY.

  • Fixes make container-build command fails #48739
  • Updates Dockerfile to:
    • Copy the project's package.json package-lock.json files
    • Run npm ci ... so that Hugo commands from the container use the same NPM packages we do in local development.
  • Fixes container-build, and changes the command so that it writes to the local public folder, making it easier to debug generated files (without this change, the container build command fails because it can't write to ./public -- which means the only other alternative would be to write to another container-only folder such as /tmp/public, but I think that the solution proposed here is better).

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 16, 2024
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Nov 16, 2024
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 16, 2024
This was referenced Nov 16, 2024
@netlify
Copy link

netlify bot commented Nov 16, 2024

Pull request preview available for checking

Name Link
🔨 Latest commit 9d1d332
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/673b4edeef526c000840113d
😎 Deploy Preview https://deploy-preview-48741--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@chalin chalin force-pushed the chalin-im-container-local-npm-pkgs-2024-11-16 branch from 7e6f74a to 9d1d332 Compare November 18, 2024 14:27
@chalin
Copy link
Contributor Author

chalin commented Nov 19, 2024

/cc @nate-double-u

@jihoon-seo
Copy link
Member

I have verified in my local env that the make container-build command succeeded without errors. LGTM.

$ make container-build
mkdir -p public
"docker" run --rm --interactive --tty --volume "/home/jhseo/website:/src:ro,Z" --read-only \
	--mount type=tmpfs,destination=/tmp,tmpfs-mode=01777 \
	--mount type=bind,source=/home/jhseo/website/public,target=/src/public gcr.io/k8s-staging-sig-docs/k8s-website-hugo:v0.133.0-0baf80536a84 \
	hugo --cleanDestinationDir --buildDrafts --buildFuture --environment preview --noBuildLock
Start building sites … 
hugo v0.133.0+extended linux/amd64 BuildDate=unknown


                   |  EN  | BN  | ZH-CN | FR  | DE  | HI  | ID  | IT  | JA  | KO  | PL  | PT-BR | RU  | ES  | UK  | VI   
-------------------+------+-----+-------+-----+-----+-----+-----+-----+-----+-----+-----+-------+-----+-----+-----+------
  Pages            | 2237 | 236 |  1838 | 380 | 210 | 136 | 337 |  92 | 569 | 617 |  71 |   364 | 193 | 319 |  95 |  79  
  Paginator pages  |   61 |   0 |    23 |   0 |   0 |   0 |   0 |   0 |   2 |   0 |   0 |     0 |   0 |   0 |   0 |   0  
  Non-page files   |  731 | 601 |   614 |  84 |  83 |  75 |  80 |  40 | 525 |  86 |  74 |    86 |  80 |  83 |  82 |  75  
  Static files     |  841 | 841 |   841 | 841 | 841 | 841 | 841 | 841 | 841 | 841 | 841 |   841 | 841 | 841 | 841 | 841  
  Processed images |   14 |   0 |     1 |   0 |   0 |   0 |   0 |   0 |   0 |   0 |   0 |     0 |   0 |   0 |   0 |   0  
  Aliases          |    8 |   1 |     6 |   2 |   1 |   0 |   1 |   1 |   3 |   3 |   0 |     3 |   4 |   2 |   1 |   0  
  Cleaned          |    0 |   0 |     0 |   0 |   0 |   0 |   0 |   0 |   0 |   0 |   0 |     0 |   0 |   0 |   0 |   0  

Total in 34650 m

@sftim
Copy link
Contributor

sftim commented Nov 20, 2024

I'll take the LGTM from #48741 (comment) as confirmation this works better than it did.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 20, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 3d7128f6d5631f4b7a19766565bbb0bba7cbf6ce

@nate-double-u
Copy link
Contributor

I confirm this corrects the build issue. Thanks @chalin

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nate-double-u

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 20, 2024
@k8s-ci-robot k8s-ci-robot merged commit 0f00267 into kubernetes:main Nov 20, 2024
6 checks passed
@chalin chalin deleted the chalin-im-container-local-npm-pkgs-2024-11-16 branch November 20, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

make container-build command fails

5 participants