Skip to content
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.

Added Dockerfile in order have containerized runs of tests #167

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

samvarankashyap
Copy link

$Subject

Any additions/suggestions to the dockerfile are highly appreciated.

@rh-atomic-bot
Copy link

Can one of the admins verify this patch?
I understand the following commands:

  • bot, add author to whitelist
  • bot, test pull request
  • bot, test pull request once

@miabbott
Copy link
Collaborator

miabbott commented Jun 2, 2017

@samvarankashyap Thanks for the PR! There are a bunch of optimizations we can do for this Dockerfile to make it a bit leaner and a bit more readable:

FROM registry.fedoraproject.org/fedora:25
RUN dnf -y install \
    git \
    python-pip \
    libselinux-python \
    python-devel \
    libffi-devel \
    redhat-rpm-config \
    openssl-devel && \
    dnf -y groupinstall "Development Tools"
RUN git clone https://github.com/projectatomic/atomic-host-tests
WORKDIR "/atomic-host-tests"
RUN pip install -r requirements.txt
RUN echo "$PWD"

If we could come up with a good way to pass in things like the inventory file, the test we want to run, and any other command line parameters for ansible-playbook then we'd have a real working container to run these tests from.

@miabbott miabbott changed the title Added Dockerfile inoder have containerized runs of tests Added Dockerfile in order have containerized runs of tests Jun 2, 2017
@samvarankashyap
Copy link
Author

@miabbott Thank you for the suggestions , I will be Updating PR . I still finding a ways to pass inventories to ansible-playbook. I found have found a hack to do the same. Will be sharing them shortly.

@samvarankashyap
Copy link
Author

@miabbott :

Ways to pass an inventory file into ansible-playbook run

  1. Copy the file and run the command # crude way of doing things

    • step1: copy the inventory_file using docker cp command
    • step2: run the ansible-playbook
  2. Use docker volumes : # cool stuff, tried and tested.

    • step1: pass the inventory file during the run
      example: docker run -v path_to_file_on_host:path_to_file_on_container -t imagename:tag ansible-playbook -i path_to_file_on_container
    • no step2
  3. Create a shared volume between the container and host: # seems complicated but if have bunch of files to share across between host and container , this is the way .

    • create a shared volume
    • attach it to container
    • mount it and use the inventories in the mounted volume on command line.

Personally, I feel the 2nd way is sufficient in our approach as we are sharing just the inventory_file

@miabbott
Copy link
Collaborator

miabbott commented Jun 5, 2017

@samvarankashyap I thought about this some more and did some more testing on my side. I wanted to solve the following problems:

  1. allow users to pass arguments to ansible-playbook as arguments to the docker run command
  2. use the git checkout of the tests in the container
  3. specify the test to run when invoking docker run of the container

This required some additional work to the the Dockerfile and the inclusion of a helper script. I'll just show my work here:

$ cat Dockerfile 
FROM registry.fedoraproject.org/fedora:25
RUN dnf -y install \
    git \
    python-pip \
    libselinux-python \
    python-devel \
    libffi-devel \
    redhat-rpm-config \
    openssl-devel && \
    dnf -y groupinstall "Development Tools"
RUN git clone https://github.com/projectatomic/atomic-host-tests
WORKDIR "/atomic-host-tests"
RUN pip install -r requirements.txt
COPY .aht.sh /aht.sh
ENTRYPOINT ["/aht.sh"]

$ cat .aht.sh 
#!/bin/bash
if [ -z "$TEST_PATH" ]; then
        echo "No test provided; please supply a value for TEST_PATH"
        exit 1
fi
if [ ! -f "/atomic-host-tests/$TEST_PATH" ]; then
        echo "The value for TEST_PATH does not exist"
        exit 1
fi
env ANSIBLE_HOST_KEY_CHECKING=False ansible-playbook "/atomic-host-tests/$TEST_PATH" "$@"

The Dockerfile looks mostly the same, but now pulls in the helper script and sets it as the entrypoint.

The helper script does a simple check to make sure there is a $TEST_PATH environment variable and that it exists in the container.

Now after building the container (mine is named sk_aht), the user would invoke the container like this:

$ sudo docker run -v /home/miabbott/.ssh/insecure_id_rsa:/root/.ssh/id_rsa:z -e TEST_PATH=tests/improved-sanity-test/main.yml sk_aht -i 10.8.174.97, -u cloud-user --tags cloud_image

In this example I'm able to:

  • pass in a private key if I'm running the playbook on a remote resource that has been pre-configured
  • I can pass in the args to ansible-playbook at the end of the docker run command
  • I can point to the test I want to run inside the container using the $TEST_PATH variable

This worked in a simple test against a remote host; I think it should work if you run the container on the host under test (with the exception of the tests that reboot the host).

What do you think?

@samvarankashyap
Copy link
Author

@miabbott : Introducing a script as an entry point to a container sounds like a brilliant idea.
I will test out this apporoach today , and get you back on this. Would you like me to update the PR w.r.to same If it goes well ?

@miabbott
Copy link
Collaborator

miabbott commented Jun 6, 2017

@samvarankashyap Please do! I hope we can make this container robust enough to handle many scenarios, so please test as much as you can. 👍

@miabbott
Copy link
Collaborator

miabbott commented Jun 8, 2017

@samvarankashyap I think we are getting closer to having this ready, but it occurred to me that in its current form, the Docker image that will result from the Dockerfile will have a static checkout of the repo.

Could you modify the helper script to do a fresh clone of the repo before running the test?

Also, if you could add some documentation in the README about how to use the Docker image, that would be useful.

@samvarankashyap samvarankashyap force-pushed the dockerfile branch 2 times, most recently from 9a3c79c to c48e908 Compare July 17, 2017 15:03
@mike-nguyen
Copy link
Collaborator

@samvarankashyap LGTM. Like @miabbott mentioned some documentation in the README would be useful.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants