If acceptable, I'm willing to do something similar for the other standard scripts as well.

.strip() needs to be .split(',') for this to work as expected.

We're working on making things python 3 compatible. (See PR #40) Please make this except EnvironmentError as ex:.

Where is the cache file expected to be written to? /usr/share/ansible/inventory/standard-inventory-qcow2.~?

Also, how does the cache file ever get cleaned up?

Are you aware that the standard-inventory-qcow2 script is intended to automatically spin up a VM based on a cloud image test subject when ansible-playbook is started, and destroy/cleanup the VM automatically when ansible-playbook exits?

I like the idea of caching things, but I can't get it to work after running a test playbook the first time. The cache file never gets cleaned up, so subsequent test runs attempt to connect to a VM (from a previous playbook run) that no longer exists.

Where is the cache file expected to be written to? /usr/share/ansible/inventory/standard- inventory-qcow2.~?

The cache file ends up wherever the inventory script itself lives. I s'pose it could go anywhere else, I just picked that spot because it was convenient.

Also, how does the cache file ever get cleaned up?

Good idea, that would better support manually running tests several times. I'll add that.

fixup! addresses all review feedback (except cache removal)

fixup! (untested) suggests method for safe cache cleanup + minor code cleanup and comments

Is this an acceptable cleanup approach, or is there a better idea?

I'm not married to the cache-file location, that could use a tempfile if preferable.

DNR: Finishing up much better caching and cleanup re-write.

I know you're still working on it, but I tested out the latest revision anyhow and had a few issues that I want to be sure are addressed.

• If the script is run with $TEST_SUBJECTS unset, or set to something other than a qcow2 image, it caches empty data and will continue to return that cached (empty) data on future runs regardless of the setting of $TEST_SUBJECTS. The script needs to properly handle non-qcow2 values in $TEST_SUBJECTS since ansible will call it for any type of test subject(s).

For example:

$ sudo rm standard-inventory-qcow2.~.orig
$ sudo TEST_SUBJECTS=local ./standard-inventory-qcow2 --list
{
    "subjects": {
        "hosts": [],
        "vars": {}
    },
    "_meta": {
        "hostvars": {}
    },
    "localhost": {
        "hosts": [],
        "vars": {}
    }
}$
$ sudo TEST_SUBJECTS=$HOME/atomic26.qcow2 ./standard-inventory-qcow2 --list
{
    "subjects": {
        "hosts": [],
        "vars": {}
    },
    "_meta": {
        "hostvars": {}
    },
    "localhost": {
        "hosts": [],
        "vars": {}
    }
}$
$ cat standard-inventory-qcow2.~.orig
{"subjects": {"hosts": [], "vars": {}}, "_meta": {"hostvars": {}}, "localhost": {"hosts": [], "vars": {}}}$
$

• I am unable to get the updated script to run without error in the simplest case. Any ideas what I could be doing wrong? (Note: the VM does get launched and is left running in the background.)

$ sudo rm standard-inventory-qcow2.~.orig
$ sudo TEST_SUBJECTS=$HOME/atomic26.qcow2 ./standard-inventory-qcow2 --list
Launching virtual machine for /home/merlinm/atomic26.qcow2
Traceback (most recent call last):
  File "./standard-inventory-qcow2", line 316, in <module>
    sys.exit(main(sys.argv))
  File "./standard-inventory-qcow2", line 86, in main
    data = list(opts.subjects)
  File "./standard-inventory-qcow2", line 147, in list
    vars = host(subject)
  File "./standard-inventory-qcow2", line 261, in host
    with lock_cache_filepath(fcntl.LOCK_SH):
  File "/usr/lib64/python2.7/contextlib.py", line 17, in __enter__
    return self.gen.next()
  File "./standard-inventory-qcow2", line 120, in lock_cache_filepath
    yield fcntl.flock(cache_filepath(environ, argv), mode)
TypeError: argument must be an int, or have a fileno() method.
$

This suffix list incorrect. "~" and ".orig" are two separate suffixes.

This is no long monitoring the correct parent PID. Since this is running in the context of the forked child and its parent will have have exited, os.getppid() here will always return 1--which will never go away. This needs to check the parent PID that is captured by os.getppid() before the fork (as was originally implemented).

I still have issue with writing a cache file to /usr/share/ansible/inventory/. Somewhere such as /var/cache/ would be much more appropriate and conform to the Filesystem Hierarchy Standard. Also, how are things supposed to work if multiple instances of standard-inventory-qcow2 are run simultaneously on the same system--such as when multiple tests are being run in parallel?

By the way, thank you for your work on this! The refactoring is taking shape nicely.

@merlinm thank you so much for looking at the early work, yes, it's not working :D

I'm finding the re-implementation of the cache-handling piece really needs unittesting. So I'm working on adding some python-3 based unittests (which requires some learning). For now I'm tossing them into /tests, with the idea we can drop in the standard Ansible interface playbooks later.

The script needs to properly handle non-qcow2 values in $TEST_SUBJECTS since ansible will call it for any type of test subject(s).

Yes, I agree. My understanding is that other inventory scripts would do their own caching so it was safe to ignore non-qcow2 input items. That's not the case then? What should the JSON look like for non-qcow2 inputs?

TypeError: argument must be an int

Yep, it's busted :D

This suffix list incorrect. "~" and ".orig" are two separate suffixes.

and

I still have issue with writing a cache file to /usr/share/ansible/inventory/.

These problems both go away by moving the persistent cache elsewhere as suggested.

Currently I'm forming the file-path as: /<tempfile.gettempdir()>/<inventory-script-name>_<uid>_<gid>_<ppid>.cache. Which should tie it sufficiently to the proper context. Though I'm happy to change this to anything else.

Also, I'm implementing the cache mechanism generically enough that it can be re-used by the other inventory scripts.

@cevich,

The script needs to properly handle non-qcow2 values in $TEST_SUBJECTS since ansible will call it for any type of test subject(s).

Yes, I agree. My understanding is that other inventory scripts would do their own caching so it was safe to ignore non-qcow2 input items. That's not the case then? What should the JSON look like for non-qcow2 inputs?

Sorry if I wasn't clear.

Your understanding is correct. Each script should operate and cache independently. However, ansible indiscriminately calls each and every script in the $ANSIBLE_INVENTORY directory regardless of the subject types in $TEST_SUBJECTS.

If you follow my previous transcript, you'll see the standard-inventory-qcow2 script was caching the empty results after it was run with TEST_SUBJECTS=local, and then when called with TEST_SUBJECTS=$HOME/atomic26.qcow2 (which referenced a valid dowloaded image) it would keep returning the empty cached results from the earlier non-qcow2 call instead of launching the qcow2.

It is correct for standard-inventory-qcow2 to return the empty results if no qcow2 subjects are found. However, it should never cache the results for non-qcow2 subjects. Each of the inventory scripts should deal only with caching for it's own subject type (if applicable).

Does that make sense now?

These problems both go away by moving the persistent cache elsewhere as suggested.
Currently I'm forming the file-path as: //_.cache. Which should tie it sufficiently to the proper context. Though I'm happy to change this to anything else.

That sounds like it should work.

Also, I'm implementing the cache mechanism generically enough that it can be re-used by the other inventory scripts.

Perfect!

Does that make sense now?

Yes, so this is a problem with "negative" caching, gotcha.

Latest push is probably not working code, however I've added unittests and those are working.

Let me stub in something for adding a negative-cache unittest, that's a really important thing to check.

Latest push is probably not working code, however I've added unittests and those are working.

Yeah. I'm not having much luck running it.
If I set TEST_SUBJECTS to a qcow2 image, I get a NameError: global name 'image' is not defined exception.
If I set TEST_SUBJECTS=local, I get a ValueError: Error launching VM 'local' for 'local',does not end in '.qcow2'. exception.
If I leave TEST_SUBJECTS unset, I get zero-length output. (Even in the null case, it needs to output a skeletal JSON dictionary like that shown in the earlier comment.)

Right, thanks for the reminder, I'll add a unittest for that, it's important.

Neck-deep into happy/sad path unittests for high-level entry/exit conditions. Turns out patching/mocking sys.stderr doesn't play nicely with using pdb for debuggin/troubleshooting :confounded:. Making slow/steady progress though.

Whew! I got it (finally). Okay, unittests (tests/test_standard-inventory-qcow2.py) are 100% passing again, including a new one for @merlinm's desire for correct output when $TEST_SUBJECTS and the subjects arg are both empty or invalid.

No idea if the actual inventory-script passes, haven't gotten that far yet. I'm doing more of a TDD thing here. So...more tests are needed...

rebased and pushed. Found a much easier way to handle unittesting w/ stdout/stderr that doesn't fuss with pdb. https://docs.python.org/3.6/library/contextlib.html?highlight=contextlib#contextlib.redirect_stdout

Implemented that along with a few bug fixes to the inventory script itself. The script now runs under argument-error conditions and does the right thing. No idea if it actually creates hosts, probably not. Testing that stuff is the next step.

Made lots and lots of progress, many bugs found & squashed. Only three more functions left to write tests for.

... including a new one for @merlinm's desire for correct output when $TEST_SUBJECTS and the subjects arg are both empty or invalid.

Thanks. But it's not just my desire to handle $TEST_SUBJECTS correctly, it's absolutely necessary. Just because standard-inventory-qcow2 thinks one of the subjects in $TEST_SUBJECTS is invalid, it may be the expected value to one of the other inventory scripts.

I keep trying to point this out but I must not be doing a very good job explaining it. Ansible indiscriminately calls each and every script in the $ANSIBLE_INVENTORY directory regardless of the value of $TEST_SUBJECTS. Each inventory script is responsible for detecting the subjects within $TEST_SUBJECTS that it is interested in and acting upon them accordingly. Thus, standard-inventory-qcow2 should identify and act only upon a qcow2 subject, and ignore anything else. Likewise, standard-inventory-docker identifies and acts only upon a docker subject within $TEST_SUBJECTS--ignoring qcow2s and anything else. (However, each inventory script still needs to output the skeletal JSON dictionary even when it ignores all the subjects.)

Please let me know if you have any questions about this point!

I've only glanced at the code, but I did run a few quick manual tests on the latest version of the script and here are a few initial observations.

• As I attempted to explain in my previous comment, don't issue warnings just because a subject does not end with .qcow2--such as is shown in the following:

# TEST_SUBJECTS=local ./standard-inventory-qcow2  --list
Warning: Error launching VM 'local' with image 'local', image does not end in '.qcow2'.
{
    "localhost": {
        "hosts": [],
        "vars": {}
    },
    "subjects": {
        "hosts": [],
        "vars": {}
    },
    "_meta": {
        "hostvars": {}
    }
}

• It can get rather verbose during the "Contacting..." phase on a slow machine

Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #1/30                                                                                                  
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #2/30                                                                                                  
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #3/30                                                                                                  
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #4/30                                                                                                  
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #5/30                                                                                                  
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #6/30                                                                                                  
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #7/30                                                                                                  
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #8/30                                                                                                  
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #9/30                                                                                                  
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #10/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #11/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #12/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #13/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #14/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #15/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #16/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #17/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #18/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #19/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #20/30
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #21/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #22/30                                                                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #23/30                    
...

• On the positive side, the VM launched successfully was contactable using the information reported in the inventory JSON!

• Unfortunately, the VM doesn't get cleaned up when the parent process (eg., the one that launches standard-inventory-qcow2) exits. The forked standard-inventory-qcow2 process and the VM continue to run in the background. It should have been automatically stopped.

• Upon manually killing the qemu process to simulate the VM exiting, a traceback is thrown:

Traceback (most recent call last):
  File "./standard-inventory-qcow2", line 528, in <module>
    main(sys.argv)
  File "./standard-inventory-qcow2", line 340, in main
    create_host(opts, subject, environ)
  File "./standard-inventory-qcow2", line 462, in create_host
    subject_watcher(proc, opts, port, hostname)
  File "./standard-inventory-qcow2", line 523, in subject_watcher
    hostname = os.path.basename(subject)
NameError: name 'subject' is not defined

But it's not just my desire to handle

@merlinm No need to essplain, I gotcha, we're on the same page :D

I added some unittests for main(), and they're passing (returning skeleton JSON, ignoring the request). If it's still breaking otherwise...there's a bug I needa fix...

Since the latest standard-inventory-qcow2 did successfully launch the requested VM, just for fun I tried using it in the normal manner as a dynamic inventory provider to ansible to run a playbook in the VM.

The first problem I ran into was that it caused ansible to hang. Suspecting it might be related to PR #47 (and Issue #52), I applied the associated update.

I tried again. This time the VM was launched and ansible proceeded (without hanging) to start running the playbook in the VM. It got a short way into the playbook and then stopped and prompted me for the VM's root password. Upon entering the password, ansible aborted:

root@127.0.0.3's password: 
fatal: [Fedora-Atomic-26-20170821.0.x86_64.qcow2]: FAILED! => {"changed": false, "cmd": "/bin/rsync --delay-updates -F --compress --archive --rsh=/bin/ssh -S none -i /tmp/inventory-cloudxzgoe9cu/ssh_priv_key -o Port=2222 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null --out-format=<<CHANGED>>%i %n%L /home/merlinm/upstreamfirst/sed/ root@127.0.0.3:/usr/local/bin/", "failed": true, "msg": "Warning: Identity file /tmp/inventory-cloudxzgoe9cu/ssh_priv_key not accessible: No such file or directory.\nWarning: Permanently added '[127.0.0.3]:2222' (ECDSA) to the list of known hosts.\r\nAuthentication failed.\r\nrsync: connection unexpectedly closed (0 bytes received so far) [sender]\nrsync error: unexplained error (code 255) at io.c(226) [sender=3.1.2]\n", "rc": 255}

Upon trying that again, I noticed the inventory script had already removed the /tmp/inventory-cloudXXXXXXXX directory (including the private key identity file needed by ansible). The inventory script shouldn't clean up that directory until after it has shut down the VM.

I also noticed the first "shebang" line of the inventory script forces the use of python3. Please make sure the final version of the script runs under either python2 or python3, but the shebang line should just reference "python".

Thank you again for all your work!

don't issue warnings

Oh okay, can-do. Or maybe I'll make that a --debug only option.

It can get rather verbose during the "Contacting..."

Yes, I agree. Okay, --debug only option then? Or I could make it print "......" instead?

On the positive side, the VM launched successfully

No way! I haven't even tested it that far. Cool!

Unfortunately, the VM doesn't get cleaned up

Upon manually killing the qemu process

I expect not, I haven't addressed any of that code in a long while, and there's no unittests (yet). I'm sure it's horribly broken. Baby steps.

the inventory script had already removed the

Oops, that's my bad. I'll fix that (and add a unittest for it).

I also noticed the first "shebang" line of the

Got it, yep, okay no prob.

Thanks for the quick-review, you're braver than I am :grinning:

@merlinm this unittest (of subtests) confirms empty-inventory output as required. The test_input dict. (above) contains all the sets of argv and environ inputs to main() that I could think of. Did I miss any?

This is the call to main() for the test_empty_mmlist unittest (below)

This is the confirmation that the JSON output for all non-qcow2 cases match the default (empty, boiler-plate) JSON.

All the rest here is just confirming that create_host() was called properly

(even though it did NOT create a host :grinning:)

I haven't actually looked at the tester script yet. I'll do that one of these days in the near future and then I'll take a shot at answering your question. :confused:

main() catches this exception and ignores it. But it's useful to have the exception for unittesting create_host() directly (w/o going through main()). That's why this test is here.

Note to me: Only check this if --debug was used?

Note to me: Only write() if --debug?

Note to me: UN-fix this :grin:

I haven't actually looked at the tester script yet.

Aww :cry: that's the tasty part. Testing is always 10-times harder than writing bad code :smile_cat: Python's unittest.mock never fails to turn my brain inside-out, make my eyes cross, and my butt-cheeks clench :astonished:

Note to me: Move this under subject_watcher(). Don't kill the ssh-key file before Ansible is done with the host.

nitpick: probably singe -> single

@cevich

Could you please make sure recently merged PRs #53 and #56 don't get lost the next time you rebase?

Also, many of the comments you made above on tests/test_standard-inventory-qcow2.py would be appropriate to include as in-line comments directly within the code. Would you consider doing so?

Thank you!

@cevich This PR addresses Issue #54, right? (At least for standard-inventory-qcow2.)

Note to me: Fix "singe -> single"

Could you please make sure recently merged PRs #53 and #56 don't get lost the next time you rebase?

Yes, thanks for the reminder. Those should cause conflicts (which I'll address).

Also, many of the comments you made above

Sure, if you think they're helpful, someone else might too. No prob.

This PR addresses Issue #54, right?

Yes it will.

Note to me: ^^^^^

--force push after --rebase, addressed all the above concerns, integrated other mentioned PRs, updated comments, and fixed up unittests to pass :grinning:

Running for a non-qcow2 subject type returns template JSON. Good. But running it a second time results in an exception. I can't even re-run it for a qcow2 subject...

root# TEST_SUBJECTS="local" ./inventory/standard-inventory-qcow2 --list
{
    "subjects": {
        "hosts": [],
        "vars": {}
    },
    "_meta": {
        "hostvars": {}
    },
    "localhost": {
        "hosts": [],
        "vars": {}
    }
}root#
root# TEST_SUBJECTS="local" ./inventory/standard-inventory-qcow2 --list
Traceback (most recent call last):
  File "./inventory/standard-inventory-qcow2", line 548, in <module>
    main(sys.argv)
  File "./inventory/standard-inventory-qcow2", line 319, in main
    environ["TEST_ARTIFACTS"] = artifacts_dirpath(environ)
  File "/usr/lib64/python2.7/os.py", line 473, in __setitem__
    putenv(key, item)
TypeError: putenv() argument 2 must be string, not None
root#
root# TEST_SUBJECTS="/root/Fedora-Atomic-26-20170821.0.x86_64.qcow2" ./inventory/standard-inventory-qcow2 --list
Traceback (most recent call last):
  File "./inventory/standard-inventory-qcow2", line 548, in <module>
    main(sys.argv)
  File "./inventory/standard-inventory-qcow2", line 319, in main
    environ["TEST_ARTIFACTS"] = artifacts_dirpath(environ)
  File "/usr/lib64/python2.7/os.py", line 473, in __setitem__
    putenv(key, item)
TypeError: putenv() argument 2 must be string, not None
root#

If the artifacts directory exists, it never gets to the return artifacts--causing artifacts_dirpath() to return None and raise an exception. Move the return to after the try block.

qemu spews a warning message when starting the VM.

root# rm -rf artifacts
root# TEST_SUBJECTS="/root/Fedora-Atomic-26-20170821.0.x86_64.qcow2" ./inventory/standard-inventory-qcow2 --list                                                       
qemu-system-x86_64: -mon chardev=mon0,mode=readline,default: option 'default' does nothing and is deprecated                                                           
{                                        
    "subjects": {                        
        "hosts": [                       
            "Fedora-Atomic-26-20170821.0.x86_64.qcow2"                             
        ],                               
        "vars": {}                       
    },                                   
    "_meta": {                           
        "hostvars": {                    
            "Fedora-Atomic-26-20170821.0.x86_64.qcow2": {                          
                "ansible_ssh_common_args": "-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no",                                                              
                "ansible_port": "2222",  
                "ansible_ssh_private_key_file": "/home/merlinm/pagure/_fork/cevich/standard-test-roles/artifacts/.inventory-cloudLgggl4.temp/ssh_priv_key",            
                "ansible_ssh_pass": "foobar",                                      
                "ansible_user": "root",  
                "ansible_host": "127.0.0.3",                                       
                "qemu_monitor_port": 2223                                          
            }                            
        }                                
    },                                   
    "localhost": {                       
        "hosts": [                       
            "Fedora-Atomic-26-20170821.0.x86_64.qcow2"                             
        ],                               
        "vars": {}                       
    }                                    
}root# qemu-system-x86_64: terminating on signal 15 from pid 21441 (python)        
root# 

The VM also exits as soon as the script exits. And where's the background watcher process?

root# ps auxww | grep qemu
root       707  0.0  0.0  30744  1616 ?        Ss   Sep15   0:00 /usr/bin/qemu-ga
root     21452  0.0  0.0 119452   992 pts/2    S+   13:46   0:00 grep --color=auto qemu
root# ps auxww | grep inventory
root     21454  0.0  0.0 119452   928 pts/2    S+   13:46   0:00 grep --color=auto inventory
root# 

By the way, I made the following patch to my local copy of standard-inventory-qcow2 before running the commands in the previous comment. Otherwise, it just continued to raise exceptions as soon as it re-created the artifacts directory mid-run...

--- a/inventory/standard-inventory-qcow2
+++ b/inventory/standard-inventory-qcow2
@@ -403,10 +403,10 @@ def artifacts_dirpath(environ=None):
     try:
         artifacts = environ.get("TEST_ARTIFACTS", os.path.join(os.getcwd(), "artifacts"))
         os.makedirs(artifacts)
-        return artifacts
     except OSError as exc:
         if exc.errno != errno.EEXIST or not os.path.isdir(artifacts):
             raise
+    return artifacts

Environment variable TEST_DEBUG is ignored now. I see you added a --debug command line option--which is fine, except ansible will never be able to call the script with such a command line option. So, make sure $TEST_DEBUG can also be used to enable debugging!

root# TEST_DEBUG=1 TEST_SUBJECTS="/root/Fedora-Atomic-26-20170821.0.x86_64.qcow2" ./inventory/standard-inventory-qcow2 --list                                          
qemu-system-x86_64: -mon chardev=mon0,mode=readline,default: option 'default' does nothing and is deprecated                                                           
{                                        
    "subjects": {                        
        "hosts": [                       
            "Fedora-Atomic-26-20170821.0.x86_64.qcow2"                             
        ],                               
        "vars": {}                       
    },                                   
    "_meta": {                           
        "hostvars": {                    
            "Fedora-Atomic-26-20170821.0.x86_64.qcow2": {                          
                "ansible_ssh_common_args": "-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no",                                                              
                "ansible_port": "2222",  
                "ansible_ssh_private_key_file": "/home/merlinm/pagure/_fork/cevich/standard-test-roles/artifacts/.inventory-cloud7dKVcy.temp/ssh_priv_key",            
                "ansible_ssh_pass": "foobar",                                      
                "ansible_user": "root",  
                "ansible_host": "127.0.0.3",                                       
                "qemu_monitor_port": 2223                                          
            }                            
        }                                
    },                                   
    "localhost": {                       
        "hosts": [                       
            "Fedora-Atomic-26-20170821.0.x86_64.qcow2"                             
        ],                               
        "vars": {}                       
    }                                    
}root# qemu-system-x86_64: terminating on signal 15 from pid 21788 (python)        

An exception is raised when specifying the --debug command line option. But on the bright side, the VM is left running!

root# rm -rf artifacts
root# TEST_SUBJECTS="/root/Fedora-Atomic-26-20170821.0.x86_64.qcow2" ./inventory/standard-inventory-qcow2 --list --debug                                               
Launching VM Fedora-Atomic-26-20170821.0.x86_64.qcow2 for /root/Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #1/5                                                 
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #1/30              
qemu-system-x86_64: -mon chardev=mon0,mode=readline,default: option 'default' does nothing and is deprecated                                                           
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #2/30              
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #3/30              
DEBUG: Access host with:  ssh -p 2222 -o StrictHostKeyChecking=no,UserKnownHostsFile=/dev/null root@127.0.0.3  # (root's password: foobar)                             
Traceback (most recent call last):       
  File "./inventory/standard-inventory-qcow2", line 548, in <module>               
    main(sys.argv)                       
  File "./inventory/standard-inventory-qcow2", line 356, in main                   
    create_host(opts, subject, environ)  
  File "./inventory/standard-inventory-qcow2", line 505, in create_host            
    sew("Access host's monitor with:  telnet {1} {2}".format(VM_IPV4_ADDR, mon_port))                                                                                  
IndexError: tuple index out of range     
root# 
root# ps auxww | grep qemu
root       707  0.0  0.0  30744  1616 ?        Ss   Sep15   0:00 /usr/bin/qemu-ga
root     21794 11.5 24.9 1625560 511140 pts/2  Sl   14:09   0:58 /usr/bin/qemu-system-x86_64 -m 1024 /root/Fedora-Atomic-26-20170821.0.x86_64.qcow2 -enable-kvm -snapshot -cdrom /home/merlinm/pagure/_fork/cevich/standard-test-roles/artifacts/.inventory-cloudzmVoxM.temp/cloud-init.iso -net nic,model=virtio -net user,hostfwd=tcp:127.0.0.3:2222-:22 -device isa-serial,chardev=pts2 -chardev file,id=pts2,path=/home/merlinm/pagure/_fork/cevich/standard-test-roles/artifacts/Fedora-Atomic-26-20170821.0.x86_64.qcow2.log -chardev socket,host=127.0.0.3,port=2223,id=mon0,server,nowait -mon chardev=mon0,mode=readline,default -display none
root     21858  0.0  0.0 119452   984 pts/2    S+   14:18   0:00 grep --color=auto qemu
root# 

The diagnostic message providing the ssh command line to use to access the VM is invalid and should be fixed so it can be cut & pasted. (But after manually entering the corrected command, the VM can be accessed just fine.)

root# ssh -p 2222 -o StrictHostKeyChecking=no,UserKnownHostsFile=/dev/null root@127.0.0.3
command-line line 0: unsupported option "no,UserKnownHostsFile".
root# ssh -p 2222 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@127.0.0.3
Warning: Permanently added '[127.0.0.3]:2222' (ECDSA) to the list of known hosts.
root@127.0.0.3's password: 
[root@localhost ~]# exit
logout
Connection to 127.0.0.3 closed.
root# 

I know I've provided feedback on the above line of code before... This is not monitoring for the correct parent PID. Since this portion of the code is running in the context of the forked child and its parent will have have exited, os.getppid() here will always return 1--which will never go away. You need to capture the original parent PID of the script before the fork and monitor that.

So, if proc.pid exists (as just confirmed by the previous statement) in the try, you TERM it? Why? proc.pid is the qemu process running the VM. No wonder the VM immediately goes away! This script needs to wait around until either (1) it's original parent process goes away or (2) the qemu VM process goes away due to external causes. Then, and only then, should this script make sure the VM is dead and clean up the temporary directory.

@merlinm Oh wow, thanks! Yeah, the watcher thing is pretty well fubar'd, I started writing unittests for it, and now you've found a bunch more that are needed :D

TEST_DEBUG

Oops, didn't mean to kill that.

Double-run and die

Eek! okay, best add a unittest for that one. Not sure what's going on but the tests always find out :D

...and countless other blunders I made...

Thanks again, we're into the meat of 'er now, and VMs are actually starting, so must be nearer to the end than the beginning :grinning:

Running for a non-qcow2 subject type returns template JSON. Good. But running it a second time results in an exception. I can't even re-run it for a qcow2 subject...

Okay, found/fixed that problem.

qemu spews a warning message when starting the VM.

Fixed.

Environment variable TEST_DEBUG is ignored now.

Fixed.

The diagnostic message providing the ssh command

Fixed

background process

Did a little work here, should be more-betterer now, but still untested / needs unittests.

rebase & --force push. Old unittests broken, but new ones for the above are ready. Will continue hacking on this later this week.

I tried giving this latest version a whirl.

It spun up the VM just fine, but no watcher process was left running to clean up...

root# TEST_SUBJECTS="/root/Fedora-Atomic-26-20170821.0.x86_64.qcow2" ./inventory/standard-inventory-qcow2 --list                 
{                                        
    "subjects": {                        
        "hosts": [                                                                 
            "Fedora-Atomic-26-20170821.0.x86_64.qcow2"                             
        ],                               
        "vars": {}                       
    },                                                                             
    "_meta": {
        "hostvars": {
            "Fedora-Atomic-26-20170821.0.x86_64.qcow2": {
                "ansible_ssh_common_args": "-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no",                                                              
                "ansible_port": "2222",                                                                                                                                
                "ansible_ssh_private_key_file": "/home/merlinm/pagure/_fork/cevich/standard-test-roles/artifacts/.inventory-cloud6zUKzF.temp/ssh_priv_key",            
                "ansible_ssh_pass": "foobar",
                "ansible_user": "root",
                "ansible_host": "127.0.0.3",
                "qemu_monitor_port": 2223
            }
        }
    },
    "localhost": {
        "hosts": [
            "Fedora-Atomic-26-20170821.0.x86_64.qcow2"
        ],
        "vars": {}
    }
}root# 
root# ps auxww | grep inventory
root     16766 14.7 24.9 1622384 509976 pts/3  Sl   14:21   0:59 /usr/bin/qemu-system-x86_64 -m 1024 /root/Fedora-Atomic-26-20170821.0.x86_64.qcow2 -enable-kvm -snapsh
ot -cdrom /home/merlinm/pagure/_fork/cevich/standard-test-roles/artifacts/.inventory-cloud6zUKzF.temp/cloud-init.iso -net nic,model=virtio -net user,hostfwd=tcp:127.0.
0.3:2222-:22 -device isa-serial,chardev=pts2 -chardev file,id=pts2,path=/home/merlinm/pagure/_fork/cevich/standard-test-roles/artifacts/Fedora-Atomic-26-20170821.0.x86
_64.qcow2.log -chardev socket,host=127.0.0.3,port=2223,id=mon0,server,nowait -mon chardev=mon0,mode=readline -display none
root     16851  0.0  0.0 119444   968 pts/3    S+   14:27   0:00 grep --color=auto inventory
root#

Yep, more unit-tests are needed to nail down the last few not working thingies. Good progress though, fixing all those other things you found + adding unittests for each of them :D

It's tricky to debug when enabling debugging triggers an exception. :stuck_out_tongue_winking_eye:

root# rm -rf artifacts
root# TEST_DEBUG=1 TEST_SUBJECTS="/root/Fedora-Atomic-26-20170821.0.x86_64.qcow2" ./inventory/standard-inventory-qcow2 --list
# DEBUG: Debugging enabled
Launching VM Fedora-Atomic-26-20170821.0.x86_64.qcow2 for /root/Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #1/5
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #1/30
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #2/30
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #3/30
Contacting VM Fedora-Atomic-26-20170821.0.x86_64.qcow2, attempt #4/30
# DEBUG: Access host with:  ssh -p 2222 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@127.0.0.3)
Traceback (most recent call last):
  File "./inventory/standard-inventory-qcow2", line 562, in <module>
    main(sys.argv)
  File "./inventory/standard-inventory-qcow2", line 358, in main
    create_host(opts, subject, environ)
  File "./inventory/standard-inventory-qcow2", line 508, in create_host
    debug("({1}'s password: {2})".format(DEF_USER, DEF_PASSWD))
IndexError: tuple index out of range
root# 

Note: there's a stray ')' at the end of the ssh copy and paste line.

Ouch. Closing all the file descriptors leaves no way to see debug messages, exceptions, etc. from the child from this point on...

It's tricky to debug when enabling debugging triggers an exception.

Lol, doh!

Note: there's a stray ')' at the end of the ssh copy and paste line.

Okay, I'll fix that and the fact that I can't count properly :D

Okay, fixed those two things and got the unit-tests working again. I still don't expect the watcher will work, some more unittests are needed for that, and to catch the debug-mode exception you found.

Added unittests to catch the debug-exceptions and ssh command output. Only unittests for the watcher remain.

rebased, --autosquash, and --force push. All unittests are written, though code is untested because...

... @merlinm My concern is in the fixme comment :D What should the behavior be in this case?

As-written, nothing is done, the watcher simply exits.

Should it attempt some cleanup, removing the host from cache, removing temp files, etc. or maybe log the exception somewhere?

(question answered on IRC)

Ugg, something broke, can't push:

$ git push origin
FATAL: W any requests/forks/cevich/standard-test-roles cevich DENIED by fallthru
(or you mis-spelled the reponame)
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.

Yay! Got the watcher working w/ and w/o debug mode. All unittests passing.

There's one small exception happening at the end of the watcher under some circumstances. IIUC it correctly, it just needs to be try-wrapped-n-ignored.

Unf. Can't push b/c error above :confounded: No idea what the problem is.

Okay, problem's fixed now. --force pushed and removed [WIP]. Unittest coverage isn' t 100% but it's a pretty-bit better than the 0% it was before :grinning: I did some manual testing with two VMs and it appears to be working as intended with and without --debug.

This is ready for final reviews and any last-minute fixes/changes needed.

Uggg, nope, this PR shows HEAD as 5680ddeae5 but mine is 44755d6cc so there's still a problem with the PR tracking my branch :confounded: Working on it...

...should be fixed now. Ready for review.

@merlinm ping, is there something blocking final review of this?

is there something blocking final review of this?

@cevich: Other burning priorities at the moment. Sorry for the continuing delay.

@merlinm Ahh, okay, you're still alive, good. N/P, take your time, I was just worried the zombie-apocalypse had begun and I didn't notice yet :grinning:

@cevich is this still needed? Sorry for the delayed review.

@tflink NP I totally forgot about it as well. Yes it's needed to fix the issue in the description as well as a few actual open issues.

Rebased & --force pushed. Incorporated support for $LOCK_ON_FILE env. var. Didn't add unittests for that yet, but all others pass.

@bgoncalv could you please test build for this PR? #7 from copr. thank you!

I tested standard-test-roles-2.9-1.fc26.7c11a00.1.noarch , but the command gets stuck and it looks like some python process crashes [python] <defunct>...

# TEST_DEBUG=1 ANSIBLE_INVENTORY=$(test -e inventory && echo inventory || echo /usr/share/ansible/inventory) TEST_SUBJECTS=../atomic.qcow2 ansible-playbook --tags=atomic tests.yml
-------------------------------------------------------------
root      5243  5124  0 11:50 pts/2    00:00:01 /usr/bin/python2 /usr/bin/ansible-playbook --tags=atomic tests.yml
root      5250  5243  0 11:50 pts/2    00:00:00 [python] <defunct>
root      5252     1 54 11:50 pts/2    00:01:08 /usr/bin/qemu-system-x86_64 -m 1024 ../atomic.qcow2 -enable-kvm -snapshot -cdrom /tmp/bgoncalv/curl/artifacts/.inventory-cloudxQ8OEG.temp/cloud-init.iso -net nic,m

@bgoncalv thanks for checking it out, I'll see if I can reproduce that, fix, and add a unittest.

@bgoncalv I couldn't reproduce the defunct process problem, but I think i spotted what might have been causing it. Can you give me a quick rundown of what you did to tested standard-test-roles-2.9-1.fc26.7c11a00.1.noarch so I can verify if I caught the bug or not?

@cevich I've tested standard-test-roles-2.9-1.fc26.79ca0e4.1.noarch, but I've got the same result.

# TEST_DEBUG=1 ANSIBLE_INVENTORY=$(test -e inventory && echo inventory || echo /usr/share/ansible/inventory) TEST_SUBJECTS=../atomic.qcow2 ansible-playbook --tags=atomic tests.yml -vvv
ansible-playbook 2.4.3.0
config file = /etc/ansible/ansible.cfg
configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
ansible python module location = /usr/lib/python2.7/site-packages/ansible
executable location = /usr/bin/ansible-playbook
python version = 2.7.14 (default, Nov  2 2017, 18:42:05) [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]
Using /etc/ansible/ansible.cfg as config file
Parsed /usr/share/ansible/inventory/standard-inventory-docker inventory source with script plugin
Parsed /usr/share/ansible/inventory/standard-inventory-local inventory source with script plugin
^C [ERROR]: User interrupted execution

Just after starting the playbook...

root      8344  6941  6 03:29 pts/2    00:00:01 /usr/bin/python2 /usr/bin/ansible-playbook --tags=atomic tests.yml
root      8351  8344  0 03:29 pts/2    00:00:00 python /usr/share/ansible/inventory/standard-inventory-qcow2 --list
root      8353  8351 99 03:29 pts/2    00:00:17 /usr/bin/qemu-system-x86_64 -m 1024 ../atomic.qcow2 -enable-kvm -snapshot -cdrom /tmp/bgoncalv/curl/artifacts/.atomic.qcow2__vGtG_.temp/cloud-init.iso -net nic,mod
root      8359     2  0 03:29 ?        00:00:00 [kvm-pit/8353]
root      8370   941  5 03:29 ?        00:00:00 sshd: root [priv]
root      8375  8370  0 03:29 ?        00:00:00 sshd: root@pts/3
root      8376  8375  0 03:29 pts/3    00:00:00 -bash
root      8395  8351 99 03:29 pts/2    00:00:01 /usr/bin/python2 /usr/bin/ansible --inventory /tmp/bgoncalv/curl/artifacts/.atomic.qcow2__vGtG_.temp/inventory.ini atomic.qcow2 --module-name raw --args /bin/true

After about 1 minute....

root      8344  6941  1 03:29 pts/2    00:00:01 /usr/bin/python2 /usr/bin/ansible-playbook --tags=atomic tests.yml
root      8351  8344  0 03:29 pts/2    00:00:00 [python] <defunct>
root      8353     1 99 03:29 pts/2    00:01:02 /usr/bin/qemu-system-x86_64 -m 1024 ../atomic.qcow2 -enable-kvm -snapshot -cdrom /tmp/bgoncalv/curl/artifacts/.atomic.qcow2__vGtG_.temp/cloud-init.iso -net nic,mod
root      8359     2  0 03:29 ?        00:00:00 [kvm-pit/8353]
root      8370   941  0 03:29 ?        00:00:00 sshd: root [priv]
root      8375  8370  0 03:29 ?        00:00:00 sshd: root@pts/3
root      8376  8375  0 03:29 pts/3    00:00:00 -bash
root      8435     1  0 03:30 ?        00:00:00 ssh: /root/.ansible/cp/9397280a35 [mux]
root      8438     1  0 03:30 ?        00:00:00 python /usr/share/ansible/inventory/standard-inventory-qcow2 --list

@bgoncalv Thanks for the extra details, I'll try to reproduce as you explained on IRC and reply with results. Thanks.

@bgoncalv Ha! Figured it out. Problem had to do with holding onto open file-descriptors from the parent process (duhhh). I've got that fixed now, and am scratching my skull with how in the heck to unittest for that...

@bgoncalv Okay, try this one in your environment, it should be better behaved. Though I'm still uncertain what's breaking WRT stderr and debug logging, it's "mostly working" :grin: Assuming it's no-longer hanging for you, I'll tackle those shenanigans and the unittests next.

@cevich thanks for the fix the tests now are now able to run :-)

I've noticed though some issue when running /usr/bin/merge-standard-inventory. I think we should use "--list" if no argument is passed.

using standard-test-roles-2.9-1.fc26.054dd75.1.noarch

# /usr/bin/merge-standard-inventory
usage: standard-inventory-qcow2 [-h] (--list | --host HOST) [--debug]
                                [subjects [subjects ...]]
standard-inventory-qcow2: error: one of the arguments --list --host is required
Traceback (most recent call last):
File "/usr/bin/merge-standard-inventory", line 208, in <module>
    sys.exit(main(sys.argv))
File "/usr/bin/merge-standard-inventory", line 37, in main
    merged_data = merge_standard_inventories(argv[1:])
File "/usr/bin/merge-standard-inventory", line 96, in merge_standard_inventories
    raise RuntimeError("Could not run: {0}".format(str(cmd)))
RuntimeError: Could not run: ['/usr/share/ansible/inventory/standard-inventory-qcow2']

while with standard-test-roles-2.9-1.fc26.noarch

# /usr/bin/merge-standard-inventory
{
    "subjects": {
        "hosts": [],
        "vars": {}
    },
    "_meta": {
        "hostvars": {}
    },
    "localhost": {
        "hosts": [],
        "vars": {}
    }
}

Not really an issue, but do you think the script could print an message when it tries to start the VM. Such as Launching virtual machine for <image name> that was there before?

I think we should use "--list" if no argument is passed.

I'm not sure what merge-standard-inventory is/does but ansible dynamic-inventory interface stipulates either --list or --host must be specified. The whole motivation/point of this PR is precisely to bring the script into standards-compliance. My intention (once this is merged) was to re-use the InvCache class among the other inventory scripts, to bring them into compliance as well.

Point is, there's are lots of good reasons to comply with standards, and mostly unpredictable/terrible side-effects that come from deviating. Especially in a testing-environment, the goal is to expose unpredictability and side-effects in the subject, not the infrastructure :D

Not really an issue, but do you think the script could print an message when it tries to start the VM. Such as Launching virtual machine for that was there before?

I spent a good part of yesterday and this morning troubleshooting this...it's really, really strange that output appears at all (during the playbook run). This appears like an example of "working-by-accident", but I'm still troubleshooting how/why the message appears at all. In all cases, I don't think it's something that should be relied on. The best thing is to log any/all output to a file, otherwise count on nobody seeing it. To see for yourself, try this inventory:

$ cat inventory/foobar.sh 
#!/bin/bash
echo -e "Localhost is all there is\n" > /dev/stderr
if [[ "$1" == "--list" ]]
then
    cat << EOF
{"all": { "hosts": ["localhost"], "vars": {} },
 "_meta": { "hostvars": { "localhost": {"ansible_connection": "local"} } } }
EOF
else
    echo "{}"
fi

and playbook:

---
- hosts: localhost
  gather_facts: False
  tasks:
    - debug: msg="test message"

This is what I get:

$ ansible-playbook -i inventory/foobar.sh ./test.yml 
PLAY [localhost] ****************************************************************************************
TASK [debug] ********************************************************************************************
ok: [localhost] => {
    "msg": "test message"
}
PLAY RECAP **********************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0   

@astepano as discussed, I added a commit to resolve --list/--host, and opened an issue regarding the new ansible-inventory command.

@bgoncalv Investigating how/why Ansible passes through stderr from an inventory script is turning into a ratt's hole. Assuming it's okay, I'm going to give up that effort.

However, I will commit to some minor cleanup work on the logging/debugging code (it's ugly), and leave this PR at that (unless a bug is found). I also expect any stderr output from other inventory scripts, is likely to unpredictably stop working at some point (Just as an FYI).

@cevich, I think I found out how the stderr messages was showing on console. If you write to tty it will show there.
The code on master does it by redirecting stderr to tty:

try:
    tty = os.open("/dev/tty", os.O_WRONLY)
    os.dup2(tty, 2)
except OSError:
    tty = None

I've tested the latest package standard-test-roles-2.9-1.fc26.bdc6945.1.noarch and noticed that some command output is not what we expected. For example:

inventory]# ./standard-inventory-qcow2 --list
{
    "subjects": {
        "hosts": [],
        "vars": {}
    },
    "_meta": {
        "hostvars": {}
    },
    "localhost": {
        "hosts": [],
        "vars": {}
    }
}

This is correct

# TEST_SUBJECTS=/tmp/bgoncalv/atomic.qcow2 ./standard-inventory-qcow2 --list
Launching VM atomic.qcow2 for /tmp/bgoncalv/atomic.qcow2, attempt #1/5
Contacting VM atomic.qcow2, attempt #1/30
Contacting VM atomic.qcow2, attempt #2/30
Contacting VM atomic.qcow2, attempt #3/30
Contacting VM atomic.qcow2, attempt #4/30
{
    "subjects": {
        "hosts": [
            "atomic.qcow2"
        ],
        "vars": {}
    },
    "_meta": {
        "hostvars": {
            "atomic.qcow2": {
                "ansible_ssh_common_args": "-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no",
                "ansible_port": "2222",
                "ansible_ssh_private_key_file": "/usr/share/ansible/inventory/artifacts/.atomic.qcow2_vPxlLp.temp/ssh_priv_key",
                "ansible_ssh_pass": "foobar",
                "ansible_user": "root",
                "ansible_host": "127.0.0.3",
                "qemu_monitor_port": 2223
            }
        }
    },
    "localhost": {
        "hosts": [
            "atomic.qcow2"
        ],
        "vars": {}
    }
}

This output is also what we expected it to be.

Now when we run the first command again the output does not seem correct, we would expect the same output as in the first case, but it seems to output the cache inventory...

# ./standard-inventory-qcow2 --list
{
    "subjects": {
        "hosts": [
            "atomic.qcow2"
        ],
        "vars": {}
    },
    "_meta": {
        "hostvars": {
            "atomic.qcow2": {
                "ansible_ssh_common_args": "-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no",
                "ansible_port": "2222",
                "ansible_ssh_private_key_file": "/usr/share/ansible/inventory/artifacts/.atomic.qcow2_vPxlLp.temp/ssh_priv_key",
                "ansible_ssh_pass": "foobar",
                "ansible_user": "root",
                "ansible_host": "127.0.0.3",
                "qemu_monitor_port": 2223
            }
        }
    },
    "localhost": {
        "hosts": [
            "atomic.qcow2"
        ],
        "vars": {}
    }
}

Now when we run the first command again the output does not seem correct, we would expect the same output as in the first case, but it seems to output the cache inventory...

Unless I'm misunderstanding the comment, that's exactly what it's supposed to do (and the whole motivation behind this PR, besides adding unittests).

Quoting from the documented standard:

the script must output a JSON encoded hash/dictionary of all the groups to be managed

Since Ansible can/will query inventory at any moment, (and possibly from multiple threads). Inventory scripts must always represent the complete, atomic, state.

The forked watcher process, will remove a host from the cache only if --debug/$TEST_DEBUG was not set, the qemu process exits, or if the parent-process (ansible) or $LOCK_ON_FILE goes away.

Am I misunderstanding something?

I think I found out how the stderr messages was showing on console

Ooohhhh, jeeze, that seems an evil thing to do. Writing directly to another process's controlling terminal? Okay, I can replicate that, but, man, it sure gives me the heebie-jeebies. Thanks for tracking that down.

Update: Struggling a lot with getting the tty-output change working w/ the unittests. Considering backing up, and applying my own advice. Breaking the code up into discrete modules. Should be much easier to unittest vs trying to do so much, all at once.

okay, rebased from master, and fixed up unittests to support writing to /dev/tty. I ended up not splitting things up into modules, since that can be done via another PR. Goal here is to get it working properly per ansible expectations and get it unittested. Unless there are any last-minute code-cleanup items needed, I think this is ready. PTAL. Thanks.

@cevich https://jenkins-fedora-atomic-process.apps.ci.centos.org/job/str_pr_test/62/console
There is a Jenkins job for each PR update.

@astepano Thanks. I'm going to hold off on work here in light of #167 conclusions - seems like that means abandoning this work (which is fine - simpler overall architecture is better for everyone).