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

Jump to bottom

Change outdated *make* syntax #213

Merged
merged 1 commit into from
Aug 12, 2020
Merged

Change outdated *make* syntax #213

merged 1 commit into from
Aug 12, 2020

Conversation

junaruga
Copy link
Member

@junaruga junaruga commented Jul 29, 2020 •
edited
Loading

Add documents to fix issues that I faced on Fedora 32.

I am running the installation steps on README.md on my local machine, Fedora 32.
Here are some suggestions to update it. The steps are until ./make.sh part which I am still trying to run.

1. Not everyone set PATH adding the current directory .. In the environment, *unmake* and *make* do not work.

$ *unmake*
bash: unmake.sh: command not found...

$ *make*
bash: make.sh: command not found...

I think ./make.sh and ./unmake.sh are better.

2. In the current README.md, the command prompt such as $ is not written. In this situation, it's hard to distinguish between the command line and the output. So, when we need to show the output of the command, I added $ as a command prompt. It is the format used in Docker's document.

$ ./make.sh
ERROR: for chris_ultron_backend_queue_1  Cannot start service queue: OCI runtime create failed: this ve
rsion of runc doesn't work on cgroups v2: unknown

3. Add possible error cases.

I am not sure if the places where I added the document is proper.

3.1. When people uses new Linux distribution where the kernel cgroups v2 is available, but cgroups v1 is not available, we need to enable cgroups v1 then restart Linux OS.

3.2. When docker deamon is running with --live-restore option, docker swarm init fails. I needed to remove the option from the config file /etc/sysconfig/docker like this, then restart docker daemon.

$ cat /etc/sysconfig/docker
# /etc/sysconfig/docker

# Modify these options if you want to change the way the docker daemon runs
# OPTIONS="--selinux-enabled \
#   --log-driver=journald \
#   --storage-driver=overlay2 \
#   --live-restore \
#   --default-ulimit nofile=1024:1024 \
#   --init-path /usr/libexec/docker/docker-init \
#   --userland-proxy-path /usr/libexec/docker/docker-proxy \
# "
OPTIONS="--selinux-enabled \
  --log-driver=journald \
  --storage-driver=overlay2 \
  --default-ulimit nofile=1024:1024 \
  --init-path /usr/libexec/docker/docker-init \
  --userland-proxy-path /usr/libexec/docker/docker-proxy \
"

3.3. First when I executed ./unmake.sh I had no idea about that the following error is valid or not.

$ ./unmake.sh
...
│Error response from daemon: This node is not part of a swarm                    │▒

@junaruga
Copy link
Member Author

junaruga commented Jul 30, 2020 •
edited
Loading

Seeing the code in moby, Docker is working in progress to support cgroups v2.

https://medium.com/nttlabs/cgroup-v2-596d035be4d7
moby/moby#40174
moby/moby@ed89041 : The latest commit related to cgroups v2 on June, 22th, 2020. It has not been released yet.

Here is the inclusive ticket.
moby/moby#40360

jennydaman
jennydaman reviewed Jul 30, 2020
View reviewed changes
README.md

```bash
git clone https://github.com/FNNDSC/ChRIS_ultron_backend
cd ChRIS_ultron_backend
# Run full CUBE instantiation with tests:
*unmake* ; sudo rm -fr FS; rm -fr FS; *make*
./unmake.sh ; sudo rm -fr FS; rm -fr FS; ./make.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with these changes, *make* syntax is outdated copied-and-pasted stuff.

README.md Outdated

#### Make sure to enable cgroups v1.

If you find the following error, plesae enable cgroups v1. The instraction is [here](https://fedoramagazine.org/docker-and-fedora-32/).
Copy link
Contributor

Choose a reason for hiding this comment

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

The fine line between thorough and excessive documentation is hard to find.

Our software relies on Docker, is it reasonable to assume that it is the user's responsibility to have a 100% working installation of Docker before proceeding to installation of our repo?

This might be better as a wiki page linked to by README.md: "before starting, make sure Docker works. --> wiki link"

The bottom line is that Fedora 32 does not support docker [1] [2]. That is a huge problem. The most direct solution is obviously to use a supported platform (Ubuntu 20.04, any other Linux, Docker on Mac, a VM, or possibly Fedora 31).
Fedora 32 users are responsible for figuring out their own Docker problems as they are platform-specific.

[1] https://docs.docker.com/engine/install/fedora/

To install Docker Engine, you need the 64-bit version of one of these Fedora versions:

  • Fedora 30
  • Fedora 31

[2] https://fedoramagazine.org/docker-and-fedora-32/

At the time of writing, Docker is not supported on Fedora 32.

Copy link
Member Author

Choose a reason for hiding this comment

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

Our software relies on Docker, is it reasonable to assume that it is the user's responsibility to have a 100% working installation of Docker before proceeding to installation of our repo?

This might be better as a wiki page linked to by README.md: "before starting, make sure Docker works. --> wiki link"

I agree. It's better to move the precondition part to run docker to the wiki page.

Copy link
Member Author

@junaruga junaruga Jul 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The most direct solution is obviously to use a supported platform (Ubuntu 20.04, any other Linux, Docker on Mac, a VM, or possibly Fedora 31).

Yeah, Fedora 32 users need to fix it by themselves in this case. I am happy to prepare the wiki page. But as I do not work for this project every day, if you like, you can go a head.

The basic container use case works on Fedora 32. :)

$ cat /etc/fedora-release 
Fedora release 32 (Thirty Two)

$ docker run --rm -t hello-world
...
Hello from Docker!
This message shows that your installation appears to be working correctly.
...

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you using podman or moby?

The SELinux troubles only happen when mounting volumes, stateless containers have fewer problems.

Copy link
Member Author

@junaruga junaruga Jul 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I am using moby for this project so far. The moby-engine RPM package in Fedora repository.

$ rpm -qf /usr/bin/docker
moby-engine-19.03.11-1.ce.git42e35e6.fc32.x86_64

The SELinux troubles only happen when mounting volumes, stateless containers have fewer problems.

That's good information to check. Thanks.
SELinux is enabled right now.

$ sestatus 
SELinux status:                 enabled
SELinuxfs mount:                /sys/fs/selinux
SELinux root directory:         /etc/selinux
Loaded policy name:             targeted
Current mode:                   enforcing
Mode from config file:          enforcing
Policy MLS status:              enabled
Policy deny_unknown status:     allowed
Memory protection checking:     actual (secure)
Max kernel policy version:      32

@junaruga junaruga force-pushed the wip/installation-doc branch 2 times, most recently from 7d0c650 to a084565 Compare July 31, 2020 10:25
@junaruga
Copy link
Member Author

I rebased this PR adding the wiki link: https://github.com/FNNDSC/ChRIS_ultron_backEnd/wiki/Making-sure-preconditions.
Currently I am working at https://github.com/junaruga/ChRIS_ultron_backEnd/wiki/Making-sure-preconditions .

@junaruga
Copy link
Member Author

junaruga commented Jul 31, 2020 •
edited
Loading

Another change in unmake.sh is to suppress the following error message instead of writing it in README.md.

$ docker swarm leave --force
Error response from daemon: This node is not part of a swarm

That is to run docker swarm leave --force only when docker node ls returns exit status 0, swarm is available.

$ docker swarm init

$ docker node ls
ID                            HOSTNAME                STATUS              AVAILABILITY        MANAGER STATUS      ENGINE VERSION
ilqhodgx889t3jl8e6oe00ll5 *   localhost.localdomain   Ready               Active              Leader              19.03.11

$ echo $?
0

$ docker swarm leave --force
Node left the swarm.

@junaruga junaruga changed the title Update README installation part WIP: Update README installation part Aug 2, 2020
@junaruga junaruga force-pushed the wip/installation-doc branch from a084565 to aa75f9c Compare August 12, 2020 09:24
@junaruga junaruga changed the title WIP: Update README installation part Change outdated *make* syntax in README Aug 12, 2020
@junaruga junaruga force-pushed the wip/installation-doc branch from aa75f9c to aefeb2f Compare August 12, 2020 09:27
@junaruga junaruga force-pushed the wip/installation-doc branch from aefeb2f to 2b53687 Compare August 12, 2020 09:30
@junaruga
Copy link
Member Author

junaruga commented Aug 12, 2020 •
edited
Loading

I rebased changing the commit message. This PR is now only to change the outdated *make* syntax, as I might take time to fix my environment issue.

[ci skip] in the commit message is the syntax to skip Travis builds, as this PR is only for documentation.

Could you merge this PR?

@junaruga junaruga changed the title Change outdated *make* syntax in README Change outdated *make* syntax Aug 12, 2020
@jbernal0019
Copy link
Member

Thanks @junaruga

@jbernal0019 jbernal0019 merged commit e5939e4 into FNNDSC:master Aug 12, 2020
@junaruga junaruga deleted the wip/installation-doc branch August 20, 2020 11:52
@bradscalio bradscalio mentioned this pull request Aug 21, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jennydaman jennydaman jennydaman left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@junaruga @jbernal0019 @jennydaman