-
Notifications
You must be signed in to change notification settings - Fork 70
[WIP] Initial setup for running the website in Docker #286
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Looking on feedback:
|
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.
I'm not sure we have an official place for these sorts of Docker images; most of our images are built during our release process by Prow; I'm not sure what the best option is here.
I would suggest mentioning your current Docker images on DockerHub (assuming that they're public) as an easier way than needing to do your own local docker build
.
docker/README.md
Outdated
This will run the service on port 9001, but you can pick any port you want. The container is listening on 1313. | ||
|
||
|
||
Diffs on script files (edited so that mounted volumes aren't deleted or copied over) |
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.
Suggestion: Rather than putting the script here, why not have a .patch
file created by something like:
diff -u ../website/scripts/localbuild.sh localbuild.sh >> diff.patch
diff -u ../website/scripts/processresourcefiles.sh processresourcefiles.sh >> diff.patch
This would allow you to use the patch
command to apply the patch to the original files in the Docker build, and avoid needing to keep an extra copy of the scripts here with only a few lines changed.
already been built
Addressed the review requests. Looking for more feedback on where this should live, right now it's website/docker. I don't know if that will cause any issues. It looks like the tests are failing right now due to another issue related to an addition recently. |
@mpetason: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Changes
/kind documentation
Fixes #