SOLR-15956: Docs for building a Docker image from the TGZ#572
Merged
HoustonPutman merged 3 commits intoapache:mainfrom Jan 31, 2022
Merged
SOLR-15956: Docs for building a Docker image from the TGZ#572HoustonPutman merged 3 commits intoapache:mainfrom
HoustonPutman merged 3 commits intoapache:mainfrom
Conversation
janhoy
reviewed
Jan 27, 2022
Contributor
janhoy
left a comment
janhoy
left a comment
There was a problem hiding this comment.
Great work. I understand and like the improvements to the build.
Feel free to accept or reject nit-picks about casing etc. I feel the README may be confusing since it is the same in git, src-release and binary-release, but it can be fixed with some wording.
| RUN set -ex; \ | ||
| (cd /opt; ln -s solr-*/ solr); \ | ||
| rm -Rf /opt/solr/docs; | ||
| rm -Rf /opt/solr/docs /opt/solr/docker/Dockerfile; |
Contributor
There was a problem hiding this comment.
This will leave behind a docker/ folder with a README.md. Could just as well remove the folder?
Contributor
Author
There was a problem hiding this comment.
We can't because it also contains docker/scripts which is obviously very important 🙂 . We don't remove the other README.md files, so no real reason to do it explicitly here.
Co-authored-by: Jan Høydahl <janhoy@users.noreply.github.com>
Contributor
Author
|
Thanks for the suggestions! |
janhoy
approved these changes
Jan 28, 2022
HoustonPutman
added a commit
that referenced
this pull request
Jan 31, 2022
* Rename "Dockerfile.local" to "Dockerfile" in the TGZ * Remove the Dockerfile from the Docker image, as it is not needed there. Co-authored-by: Jan Høydahl <janhoy@users.noreply.github.com> (cherry picked from commit f19b9e8)
HoustonPutman
added a commit
that referenced
this pull request
Jan 31, 2022
* Rename "Dockerfile.local" to "Dockerfile" in the TGZ * Remove the Dockerfile from the Docker image, as it is not needed there. Co-authored-by: Jan Høydahl <janhoy@users.noreply.github.com> (cherry picked from commit f19b9e8)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/SOLR-15956
Note I have also renamed
solr-<version>/docker/Dockerfile.localtosolr-<version>/docker/Dockerfilein the distribution, as I don't think there would be any confusion there and it is just cleaner. It's completely compatible with the official Docker image, just uses the binary distribution instead of downloading official releases.As a last non-doc change, I added back in the logic to remove the Dockerfile from the Docker image. This is because without this logic, the official docker image would contain the local Dockerfile, which doesn't particularly make sense. Easier to just remove it as it doesn't provide value.
Will mention both above changes in the JIRA.