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.

Sign up for GitHub

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

Fix locale in CentOS based image #4056

Merged
merged 1 commit into from
Feb 8, 2017
Merged

Fix locale in CentOS based image #4056

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 13 additions & 9 deletions dockerfiles/che/Dockerfile.centos
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,16 @@
# Dharmit Shah - Initial implementation
# Mario Loriedo - Improvements
#
# To build it, run in the current folder after executing build.sh:
# `docker build -t registry.centos.org/eclipse/che-server -f Dockerfile.centos .`
# To build it, in current folder:
# cp Dockerfile.centos Dockerfile
# ./build.sh
#
# To run it:
# docker run --net=host \
# --name che \
# -v /var/run/docker.sock:/var/run/docker.sock \
# -v /home/user/che/lib:/home/user/che/lib-copy \
# -v /home/user/che/workspaces:/home/user/che/workspaces \
# -v /home/user/che/storage:/home/user/che/storage \
# registry.centos.org/eclipse/che-server
# docker run -d -p 8080:8080 \
# --name che \
# -v /var/run/docker.sock:/var/run/docker.sock \
# -v ~/.che/workspaces:/data \
# eclipse/che-server:nightly
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure we want to require nightly. eclipse/che-server:nightly => eclipse/che-server. Should change -v ~/.che/workspaces:/data => -v <LOCAL_PATH>:/data.

Copy link
Contributor

Choose a reason for hiding this comment

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

about the first point if you use build script by default it will produce nightly so if you need to run this newly image you'll have to use that version

Copy link
Contributor

Choose a reason for hiding this comment

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

Still not sure if nightly should be used. Who will use the dockerfile? Seems better to allow user to choose version. Seems like it possible but I have not investigated https://github.com/eclipse/che/blob/master/dockerfiles/build.include#L33 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JamesDrummond I like ~/.che/ more because I can just copy paste and it works. I don't need an extra step of editing the command. But I understand you want to make it clear that the path is configurable. I just wanted to be sure that you understand why I prefer ~/.che/ (and potentially other users would prefer it too). If you still think that setting <LOCAL_PATH> is a better choice I will change that.

What is the issue with nightly? I understand you want to change that but I don't understand why. I'm asking because in the past I've fixed an issue with non nightly tags and I ran into another issue recently. So when I build it I usually use the default tag (nightly) to improve my chance to build/run successfully.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is why I ask who are the users going to be for the dockerfile. ~/.che will not work on a windows machine. Using nightly label is fine but I don't think it should be required. What if you want to produce a stable version of the centos to host on a repo such as dockerhub. Again I am not sure of the use case of this dockerfile. If you would like to keep ~/.che and nightly in place because that is what the target use will be I can approve. Just let me know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

James this Dockerfile is an alternative to the che-server alpine based Dockerfile. The image should be used by individuals or organizations that prefer running Che on CentOS then alpine.

Instructions in the Dockerfiles should be for developers that know what they are doing and want hack the Dockerfile (to fix a bug in the Dockerfile for example). A user that just want to run the image should pull the pre-built images available from the Dockerhub and in registry.centos.org (and run it using the CLI).

Copy link
Contributor

Choose a reason for hiding this comment

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

Great thanks.

#
FROM registry.centos.org/centos/centos:latest

Expand All @@ -40,6 +39,11 @@ RUN yum -y update && \
sed -i 's/Defaults requiretty/#Defaults requiretty/g' /etc/sudoers && \
rm -rf /tmp/* /var/cache/yum

# The following lines are needed to set the correct locale after `yum update`
# c.f. https://github.com/CentOS/sig-cloud-instance-images/issues/71
RUN localedef -i en_US -f UTF-8 C.UTF-8
ENV LANG="C.UTF-8"

EXPOSE 8000 8080
COPY entrypoint.sh /entrypoint.sh
ENTRYPOINT ["/entrypoint.sh"]
Expand Down