Atomic/backends/_docker.py: Error prevention with atomic run

There were two primary cases where a secondary atomic run with a command
would trigger an exception.  The first was reported in
https://github.com/projectatomic/atomic/issues/1006. Basically it can
be summarized as:

```
atomic run registry.fedoraproject.org/fedora:25 date  # works fine
atomic run registry.fedoraproject.org/fedora:26 date  # tries to run in the existing f25 container
```

The second case is as simple as:

```
atomic run registry.fedoraproject.org/fedora:25 date  # works fine
atomic run registry.fedoraproject.org/fedora:25 date  # fails
```

This fails because atomic starts the stopped f25 container and then attempts a docker exec.  The
exec fails because the 'date' command is short-lived and the container exits prior to the exec
being run.

We now catch those exceptions and notify the user.  We added a `--replace` option to run where
atomic will now delete the container in question and re-run it from the correct image.

Closes: #1019
Approved by: baude

Atomic/backends/_docker.py

@@ -18,6 +18,8 @@ from Atomic import Atomic
 from requests.exceptions import HTTPError
 from Atomic.backends._docker_errors import NoDockerDaemon
 from Atomic.discovery import RegistryInspectError
+from Atomic.atomic import AtomicError
+from subprocess import CalledProcessError
 
 try:
     from subprocess import DEVNULL  # pylint: disable=no-name-in-module
@@ -516,6 +518,16 @@ class DockerBackend(Backend):
             list_item += value
             return list_item
 
+        def replace_existing_container(_iobject, _requested_image, _args):
+            if _args.debug:
+                util.write_out("Removing the container {} and running with {}".format(_iobject.name,
+                                                                                      requested_image.fq_name))
+            self.delete_container(_iobject.id, force=True)
+            _iobject = _requested_image
+            if _args.command:
+                _iobject.user_command = _args.command
+            return _iobject
+
         atomic = kwargs.get('atomic', None)
         args = kwargs.get('args')
         # atomic must be an instance of Atomic
@@ -537,10 +549,37 @@ class DockerBackend(Backend):
                                "{}s\n\n removes all containers based on an "
                                "image.".format(iobject.name, iobject.image_name, iobject.name, iobject.name,
                                                iobject.image_name, iobject.image_name, iobject.image_name))
+
+            requested_image = self.has_image(args.image)
+            if requested_image is None:
+                requested_image = self.has_image(iobject.image)
+
             if iobject.running:
+                if args.replace:
+                    iobject = replace_existing_container(iobject, requested_image, args)
                 return self._running(iobject, args, atomic)
             else:
-                return self._start(iobject, args, atomic)
+                # Container with the name exists
+                image_id = iobject.image
+                if requested_image.id != image_id:
+                    if args.replace:
+                        iobject = replace_existing_container(iobject, requested_image, args)
+                    else:
+                        try:
+                            requested_image_fq_name = requested_image.fq_name
+                        except RegistryInspectError:
+                            requested_image_fq_name = args.image
+                        raise AtomicError("Warning: container '{}' already points to {}\nRun 'atomic run {}' to run "
+                                          "the existing container.\nRun 'atomic run --replace '{}' to replace "
+                                          "it".format(iobject.name,
+                                                      iobject.original_structure['Config']['Image'],
+                                                      iobject.name,
+                                                      requested_image_fq_name))
+                else:
+                    if args.replace:
+                        iobject = replace_existing_container(iobject, requested_image, args)
+                    else:
+                        return self._start(iobject, args, atomic)
 
         if iobject.get_label('INSTALL') and not args.ignore and not util.InstallData.image_installed(iobject):
             raise ValueError("The image '{}' appears to have not been installed and has an INSTALL label.  You "
@@ -660,15 +699,23 @@ class DockerBackend(Backend):
                     util.write_out("Container is running")
 
     def _start(self, con_obj, args, atomic):
+        exec_error = "Failed to execute the command inside the existing container. In some situations " \
+                     "this can happen because the entry point command of the container only runs for " \
+                     "a short time. You might want to replace the container by executing your " \
+                     "command with --replace. Note any updates to the existing container will be lost"
+
         if con_obj.interactive:
             if args.command:
-                util.check_call(
-                    [atomic.docker_binary(), "start", con_obj.name],
-                    stderr=DEVNULL)
+                util.check_call([atomic.docker_binary(), "start", con_obj.name], stderr=DEVNULL)
                 container_command = args.command if isinstance(args.command, list) else args.command.split()
-                return util.check_call(
-                    [atomic.docker_binary(), "exec", "-t", "-i", con_obj.name] +
-                    container_command)
+                try:
+                    return util.check_call([atomic.docker_binary(), "exec", "-t", "-i", con_obj.name] + container_command)
+                except CalledProcessError as e:
+                    if args.debug:
+                        util.write_out(str(e))
+                    raise AtomicError(exec_error)
+
+
             else:
                 return util.check_call(
                     [atomic.docker_binary(), "start", "-i", "-a", con_obj.name],
@@ -678,9 +725,15 @@ class DockerBackend(Backend):
                 util.check_call(
                     [atomic.docker_binary(), "start", con_obj.name],
                     stderr=DEVNULL)
-                return util.check_call(
-                    [atomic.docker_binary(), "exec", "-t", "-i", con_obj.name] +
-                    con_obj.command)
+                try:
+                    return util.check_call(
+                        [atomic.docker_binary(), "exec", "-t", "-i", con_obj.name] +
+                        con_obj.command)
+                except CalledProcessError as e:
+                    if args.debug:
+                        util.write_out(str(e))
+                    raise AtomicError(exec_error)
+
             else:
                 return util.check_call(
                     [atomic.docker_binary(), "start", con_obj.name],

Atomic/run.py

@@ -45,6 +45,9 @@ def cli(subparser):
     runp.set_defaults(_class=Run, func='run')
     run_group = runp.add_mutually_exclusive_group()
     util.add_opt(runp)
+    runp.add_argument("--replace", "-r", dest="replace", default=False,
+                      action="store_true", help=_("Replaces an existing container by the same name"
+                                                  "if it exists."))
     runp.add_argument("--storage", dest="storage", default=None,
                           help=_("Specify the storage. Default is currently '%s'.  You can"
                                  " change the default by editing /etc/atomic.conf and changing"

bash/atomic

@@ -675,6 +675,7 @@ _atomic_run() {
 		--spc
 	       --display
 	--quiet
+	       --replace
 	    --storage
 	"
 

docs/atomic-run.1.md

@@ -9,6 +9,7 @@ atomic-run - Execute container image run method
 [**-h**|**--help**]
 [**--display**]
 [**-n**][**--name**[=*NAME*]]
+[**-r**, **--replace**]
 [**--spc**]
 [**--storage**]
 [**--quiet**]
@@ -66,6 +67,9 @@ If --display is not specified the run command will execute.
    Use this name for creating run content for the container.
 NAME will default to the IMAGENAME if it is not specified.
 
+**-r** **--replace**
+   Replaces an existing container by the same name if it exists prior to running.
+   
 **--spc**
   Run container in super privileged container mode.  The image will run with the following command:
 

tests/integration/test_run.sh

@@ -0,0 +1,69 @@
+#!/bin/bash -x
+set -euo pipefail
+IFS=$'\n\t'
+
+ATOMIC=${ATOMIC:="/usr/bin/atomic"}
+ATOMIC=$(grep -v -- --debug <<< "$ATOMIC")
+DOCKER=${DOCKER:="/usr/bin/docker"}
+
+teardown () {
+	set +e
+	${DOCKER} rm -f RUN_TEST > /dev/null
+	set -e
+}
+
+fail () {
+	echo "Fail: TEST ${1} should have failed and did not"
+	exit 1
+}
+
+passed () {
+	echo "Passed: TEST ${1}"
+}
+
+
+failed () {
+	echo "Fail: TEST ${1}"
+	exit 1
+}
+
+
+trap teardown EXIT
+
+# Check that atomic run's naming
+TEST_NUM=1
+${ATOMIC} run -n RUN_TEST atomic-test-1 date
+CID=$("$DOCKER" ps -alq)
+NAME=$("$DOCKER" inspect --format='{{.Name}}' "$CID")
+
+
+if [[ "${NAME}" != /RUN_TEST ]]; then
+    failed "${TEST_NUM}"
+fi
+passed "${TEST_NUM}"
+
+TEST_NUM=2
+rc=0
+${ATOMIC} run -n RUN_TEST atomic-test-1 date 1>/dev/null || rc=$?
+if [[ ${rc} != 1 ]]; then
+    # Test failed
+    fail "${TEST_NUM}"
+fi
+
+passed "${TEST_NUM}"
+
+
+TEST_NUM=3
+${ATOMIC} run --replace -n RUN_TEST atomic-test-1 date
+NEW_CID=$("$DOCKER" ps -alq)
+NAME=$("$DOCKER" inspect --format='{{.Name}}' "$NEW_CID")
+
+if [[ "${NAME}" != /RUN_TEST ]]; then
+    failed "${TEST_NUM}"
+fi
+
+if [[ "${NEW_CID}" == "${CID}" ]]; then
+    failed "${TEST_NUM}"
+fi
+
+passed "${TEST_NUM}"