Add atomic CLI tests #273

mike-nguyen · 2017-11-08T19:15:13Z

This commit adds tests for the the atomic command. The tests revolve
mostly around pulling images and installing/running containers with
variations of the options in the atomic subcommand. These tests include:

Pull by fully qualified image name
Pull by short image name
Pull by fully qualified image name with --name option
Pull with tags
Pull by digest
Pull to ostree/docker storage
Images update command

Most of the tests use the cockpit container. This container was chosen
because it has most of the labels that the atomic command utilizes. For
simple tests, the busybox container was chosen because it is small.

mike-nguyen · 2017-11-08T19:19:33Z

centos will likely fail because of projectatomic/docker#285

miabbott · 2017-11-11T00:56:19Z

roles/atomic_images_delete/tasks/main.yml

- name: Remove specified image
-  command: atomic --debug --assumeyes images delete {{ image }}
+- name: Remove {{ aid_image }}
+  command: atomic --assumeyes images delete {{ aid_image }} {{ del_options }}


nit: no newline at the end of the file (GitHub actually highlights this in the diff!)

miabbott · 2017-11-11T01:01:04Z

tests/atomic/main.yml

+
+    - name: Set cockpit container name for Fedora/CentOS
+      set_fact:
+        cockpit_cname: "registry.fedoraproject.org/f25/cockpit"


Probably should bump this to F26; F27 should be released Nov 14 and that will EOL F25

miabbott · 2017-11-11T01:05:43Z

tests/atomic/main.yml

+    - role: atomic_run
+      ar_image: "{{ cockpit_cname }}"
+
+    - role: atomic_images_list_verify


Is it necessary to keep checking that the cockpit image is still there after each operation?

I can drop this this check.

Still looks like there a bunch of extra image checks in the test. Could you audit the entire test and see if we really need each instance of atomic_images_list_verify?

miabbott · 2017-11-11T01:07:08Z

tests/atomic/main.yml

+    - vars.yml
+
+  pre_tasks:
+    - name: Set cockpit container name for RHELAH


Maybe move these types of distro specific facts into vars files and use with_first_found to load them in?

I ended up using vars_files with the ansible_distribution name since there was no difference between the different versions of a distribution (ie fedora 26, 27, etc). The only thing I had to account for the ansible_distribution for cahc which is centosdev. If needed I could use include_vars with with_first_found. I didn't feel that was needed for this simple case.

miabbott · 2017-11-11T01:26:47Z

tests/atomic/main.yml

+        image_name: "{{ cockpit_short_name }}"
+      check_mode: false
+
+    - role: atomic_stop


Might want to drop a comment here to explain the json queries and what you are looking for.

miabbott · 2017-11-11T01:43:18Z

tests/atomic/main.yml

+        apl_image: "{{ fq_name }}"
+        apl_options: "--storage=ostree"
+
+    # The TYPE field doesn't show up in the json output so


Is this a bug in atomic itself?

I forgot to file an upstream issue for that. I'll add a comment in the code with the issue: projectatomic/atomic#1129

miabbott · 2017-11-11T01:48:27Z

Excellent work overall! I like that you prefixed variable names in the roles to make them more unique per role (like we are supposed to do!)

I have a couple of minor comments, but I'm betting this can get merged very soon. Would like to kick it around myself, though

This commit adds tests for the the atomic command. The tests revolve mostly around pulling images and installing/running containers with variations of the options in the atomic subcommand. These tests include: - Pull by fully qualified image name - Pull by short image name - Pull by fully qualified image name with --name option - Pull with tags - Pull by digest - Pull to ostree/docker storage - Images update command Most of the tests use the cockpit container. This container was chosen because it has most of the labels that the atomic command utilizes. For simple tests, the busybox container was chosen because it is small.

miabbott · 2017-11-20T16:42:24Z

fedora/27/atomic failure looks like a Fedora manifestation of https://bugzilla.redhat.com/show_bug.cgi?id=1498575

The build of atomic in F27 is from Aug 24!!! Newer versions should have the fix for the problem observed.

miabbott · 2017-11-20T17:22:53Z

tests/atomic/main.yml

+    - pull_tags
+
+  pre_tasks:
+    - name: Set cockpit container name for RHELAH


Is this comment correct, i.e. only for RHELAH?

miabbott · 2017-11-20T19:43:52Z

Could we move the vars files under tests/atomic/vars/ rather than have them all in the root directory?

mike-nguyen · 2017-11-20T19:48:16Z

Could we move the vars files under tests/atomic/vars/ rather than have them all in the root directory?

Should be possible.

mike-nguyen · 2017-11-21T20:31:53Z

bot, retest this please

miabbott · 2017-11-22T15:28:24Z

roles/atomic_containers_list/tasks/main.yml

+
+- name: Save JSON to variable
+  set_fact:
+    acl_json: "{{ acl.stdout | to_json | from_json }}"


Is it necessary to use both filters? Seems like you should just be able to use from_json?

I don't need both. I'm not quite sure why I did that.

miabbott · 2017-11-22T15:29:44Z

roles/atomic_images_list/tasks/main.yml

+
+- name: Save JSON to variable
+  set_fact:
+    ail_json: "{{ ail.stdout | to_json | from_json }}"


Same question about both filters here

miabbott · 2017-11-22T15:34:04Z

roles/docker_rmi/tasks/main.yml

+    msg: "drmi_image is not defined"
+  when: drmi_image is undefined
+
+- name: Set optioins


s/optioins/options/

miabbott · 2017-11-22T15:34:37Z

roles/docker_tag/tasks/main.yml

+- debug:
+    msg: "dt_img: {{ dt_image }}"
+
+- debug:


Did you want to leave these debug: statements here?

Nope, removed!

mike-nguyen · 2017-11-30T20:23:32Z

New fixup with the comments addressed. I also snuck in a fix to handle the new TOML /etc/containers/registries.conf

miabbott · 2017-11-30T20:25:51Z

roles/docker_private_registry/tasks/main.yml

+      replace: "[registries.insecure]\nregistries = ['{{ ansible_docker0.ipv4.address }}:5000']"
+      dest: /etc/containers/registries.conf
+    when: reg.stat.exists and toml.rc == 0
+
  - name: add registeries


s/registeries/registries/

https://i.pinimg.com/736x/3a/11/68/3a116884945f870924f1ffd3f36fc015--los-simpsons-homer-simpson.jpg

mike-nguyen · 2017-11-30T21:10:15Z

roles/docker_private_registry/tasks/main.yml

+    register: toml
+    when: reg.stat.exists
+    ignore_errors: true
+


I originally forgot to ignore errors on the grep so it failed when it wasn't a TOML file in the case of fedora/26/atomic. Fixed it here.

mike-nguyen · 2017-12-04T16:39:26Z

The CentOS failures are due to projectatomic/docker#285. CentOS won't use the fixed version of docker until projectatomic/docker#286 is fixed. The current version of docker on CentOS is using a version of docker that doesn't have a /etc/containers/registries.conf which causes the test to fail because the test uses an insecure registry. Once the docker fix is in CentOS the tests should pass.

miabbott reviewed Nov 11, 2017

View reviewed changes

mike-nguyen force-pushed the atomic_tests branch from 94760e7 to 7e0055a Compare November 15, 2017 20:25

mike-nguyen added 2 commits November 15, 2017 15:31

fixup! Add atomic CLI tests

f17c869

fixup! Add atomic CLI tests

51de90f

miabbott reviewed Nov 20, 2017

View reviewed changes

mike-nguyen added 2 commits November 20, 2017 15:09

fixup! Add atomic CLI tests

37a6a33

fixup! Add atomic CLI tests

5c685b9

miabbott reviewed Nov 22, 2017

View reviewed changes

fixup! Add atomic CLI tests

d638c45

miabbott reviewed Nov 30, 2017

View reviewed changes

mike-nguyen added 2 commits November 30, 2017 15:31

fixup! Add atomic CLI tests

184d3cf

fixup! Add atomic CLI tests

9d99904

mike-nguyen commented Nov 30, 2017

View reviewed changes

miabbott merged commit 31914d4 into projectatomic:master Dec 4, 2017

mike-nguyen deleted the atomic_tests branch March 28, 2018 13:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add atomic CLI tests #273

Add atomic CLI tests #273

mike-nguyen commented Nov 8, 2017

mike-nguyen commented Nov 8, 2017

miabbott Nov 11, 2017

miabbott Nov 11, 2017

miabbott Nov 11, 2017

mike-nguyen Nov 15, 2017

miabbott Nov 20, 2017

miabbott Nov 11, 2017

mike-nguyen Nov 15, 2017

miabbott Nov 11, 2017

miabbott Nov 11, 2017

mike-nguyen Nov 15, 2017

miabbott commented Nov 11, 2017

miabbott commented Nov 20, 2017

miabbott Nov 20, 2017

miabbott commented Nov 20, 2017

mike-nguyen commented Nov 20, 2017

mike-nguyen commented Nov 21, 2017

miabbott Nov 22, 2017

mike-nguyen Nov 30, 2017

miabbott Nov 22, 2017

miabbott Nov 22, 2017

miabbott Nov 22, 2017

mike-nguyen Nov 30, 2017

mike-nguyen commented Nov 30, 2017

miabbott Nov 30, 2017

mike-nguyen Nov 30, 2017

mike-nguyen Nov 30, 2017

mike-nguyen commented Dec 4, 2017

Add atomic CLI tests #273

Add atomic CLI tests #273

Conversation

mike-nguyen commented Nov 8, 2017

mike-nguyen commented Nov 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miabbott commented Nov 11, 2017

miabbott commented Nov 20, 2017

Choose a reason for hiding this comment

miabbott commented Nov 20, 2017

mike-nguyen commented Nov 20, 2017

mike-nguyen commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mike-nguyen commented Nov 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mike-nguyen commented Dec 4, 2017