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

Fixes #9875: Better docker service restart #56

Merged
merged 1 commit into from
May 14, 2015
Merged

Fixes #9875: Better docker service restart #56

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
9 changes: 7 additions & 2 deletions templates/rhsm-katello-reconfigure.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,14 @@ if [ -d $CA_TRUST_ANCHORS ]; then
cp $KATELLO_CERT_DIR/$KATELLO_CERT $CA_TRUST_ANCHORS
update-ca-trust

#restart if docker service is installed
service docker status >/dev/null && \
# restart docker if it is installed and running
if [ -f /usr/lib/systemd/system/docker.service ]; then
Copy link

Choose a reason for hiding this comment

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

Unit files installed by packages are placed in /usr/lib/systemd/system/, but system administrators may also install unit files in /etc/systemd/system/. An administrator might even do something crazy like nuke the default service file and install a custom one to /etc/systemd/system/. As a result, this sort of test may be better:

if which systemctl >& /dev/null; then
    echo 'systemctl is available'
elif which service >& /dev/null; then
    echo 'service is available'
fi

>& sends both stdout and stderr to the specified file handle. See here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ichimonji10 even though I use the check you proposed I will have to check for those unit files to know if docker is installed or not. If the sysadmin changes places so the message showing that the services was restarted will not be printed.

Also other stuff I left according to what I found in other places in this codebase. But I can definitely update as per your suggestions, let's see what others tell about them.

Copy link

Choose a reason for hiding this comment

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

@elyezer, I totally forgot about checking for docker.service in particular, so my example code is incomplete. Whoops! Still, I think there's a case to be made for using utilities to check for the presence of unit files or service files instead of directly querying the filesystem for the presence of files. I'd love to hear what others have to say.

Copy link

Choose a reason for hiding this comment

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

This example deals with the case where the systemctl executable and docker.service unit file may or may not exist in standard locations on the system:

if systemctl list-unit-files docker.service | grep '^docker.service' >&/dev/null; then
    echo 'systemctl docker.service may be used'
elif [ -f /etc/init.d/docker ]; then
    echo 'service docker may be used'
fi

I am not terribly knowledgeable about service and where service files may exist on a system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ichimonji10 I tried your suggestion and get the following. Also I have to remove the docker.service option from list-unit-files. I will need to deal with the output of systemd not being available and that will make the code bigger. I will stick with 'Simple is better than complex.' here.

[root@rhel66 ~]# if systemctl list-unit-files | grep '^docker.service' >&/dev/null; then echo 'have docker'; else echo 'does not have docker'; fi
-bash: systemctl: command not found
does not have docker

Copy link

Choose a reason for hiding this comment

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

@elyezer OK. That sounds good.

systemctl status docker >/dev/null && \
systemctl restart docker >/dev/null 2&>1
elif [ -f /etc/init.d/docker ]; then
service docker status >/dev/null && \
service docker restart >/dev/null 2&>1
fi
fi

# restart goferd if it is installed and running
Expand Down