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.

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

Added '-q' as a default docker build command to fix issue #53 #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ranvirm
Copy link

@ranvirm ranvirm commented Dec 21, 2020

Added '-q' as a default docker build command to fix issue #53 where 'eb local run' incorrectly parses last word of docker build stdout as image id to docker run

Issue #, if available:
#53

Description of changes:

As documented in issue #53, there is an issue when running 'eb local run' to run a docker image locally via the eb cli.
The problem is documented in detail below but in short is caused by the eb cli parsing the last line of docker build stdout (which is the time taken to complete the stage)
as the image id which is then used as the image id to the docker run command.

I looked at the flow of getting the image ID to use in the container run command and found that this function is the one that builds the image and returns an image id (for use in the container run function):

ebcli/containers/commands.py line 49

def build_img(docker_path, file_path=None):
    """
    Builds a docker image using Dockerfile found in docker path.
    :param docker_path: str: path of dir containing the Dockerfile
    :param file_path: str: optional name of Dockerfile
    :return: str: id of the new image
    """

    opts = ['-f', file_path] if file_path else []
    args = ['docker', 'build'] + opts + [docker_path]
    output = _run_live(args)
    return _grab_built_image_id(output)

Now there are two possible sources of error (and solutions here) - Solution 1 is to alter the _grab_built_image_id() function or Solution 2: Alter the function that runs the actual docker build command (_run_live() or build_img()) so that the output is formatted correctly.

The _run_live() function is called with args = ['docker', 'build'] + opts + [docker_path]
The function:

ebcli/containers/commands.py line 277

def _run_live(args):
    try:
        return utils.exec_cmd_live_output(args)
    except CommandError as e:
        _handle_command_error(e)

As you can see, it then calls exec_cmd_live() which is:

ebcli/lib/utils.py line 236
exec_cmd_live_output = exec_cmd

which is:

ebcli/lib/utils.py line 199

def exec_cmd(args, live_output=True):
    """
    Execute a child program (args) in a new process. Displays
    live output by default.
    :param args: list: describes the command to be run
    :param live_output: bool: whether to print live output
    :return str: child program output
    """

    LOG.debug(' '.join(args))

    process = Popen(args, stdout=PIPE, stderr=STDOUT)
    output = []

    for line in iter(process.stdout.readline, b''):
        line = line.decode('utf-8')
        if line != os.linesep:
            if live_output:
                sys.stdout.write(line)
                sys.stdout.flush()
            else:
                LOG.debug(line)

        output.append(line)

    process.stdout.close()
    process.wait()

    returncode = process.returncode
    error_msg = 'Exited with return code {}'.format(returncode)
    output_str = ''.join(output)

    if returncode:
        raise CommandError(error_msg, output_str, returncode)
    return output_str

exec_cmd() actually executes the docker build command with:

process = Popen(args, stdout=PIPE, stderr=STDOUT)

This basically runs a system command and returns the running process so that the output logs can be read.

So I then tried to re-create the docker build command that generates the build logs we are seeing when running eb local run and found that this command does that:

docker build --progress plain -t tmp-image .

Note the progress plain param to the docker build call - this is what produces the build output we see. Last line of output from above docker build command:

#13 DONE 0.0s

And there's the 0.0s that's being used by the AWS eb cli as the image id.

So then solution 2: Instead of instructing docker to produce those build logs, the docker build command could be run quietly as below:

docker build -q -t tmp-image .

Which will just write out the desired image id.

The issue with running the docker build quietly though is that we won't see the build logs from eb local run which could bother some people but creating rules on the text output from docker build is flimsy as it will break if the output from docker build changes again.

I tested solution 2 proposed above, changing the docker build command by altering this function:

ebcli/containers/commands.py line 49

FROM

def build_img(docker_path, file_path=None):
    """
    Builds a docker image using Dockerfile found in docker path.
    :param docker_path: str: path of dir containing the Dockerfile
    :param file_path: str: optional name of Dockerfile
    :return: str: id of the new image
    """

    opts = ['-f', file_path] if file_path else []
    args = ['docker', 'build'] + opts + [docker_path]
    output = _run_live(args)
    return _grab_built_image_id(output)

TO

def build_img(docker_path, file_path=None):
    """
    Builds a docker image using Dockerfile found in docker path.
    :param docker_path: str: path of dir containing the Dockerfile
    :param file_path: str: optional name of Dockerfile
    :return: str: id of the new image
    """

    opts = ['-q', '-f', file_path] if file_path else ['-q']
    args = ['docker', 'build'] + opts + [docker_path]
    output = _run_live(args)
    return _grab_built_image_id(output)

Added -q to always run docker build quietly and thus only output the image id

The command eb local run --port 3838 then ran successfully

This fix works and should work on all platforms as I'm pretty sure that docker build quietly will always
output the same (image id only) on all platforms
(as documented here)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…ocal run' incorrectly parses last word of docker build stdout as image id to docker run - issue aws#53
@ranvirm ranvirm changed the title Added '-q' as a default docker build command in to fix issue #53 Added '-q' as a default docker build command to fix issue #53 Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant