Fixed dockerng.list_tags

[farcaller]

What does this PR do?

Fixes a bug in dockerng module crashing when an image doesn't have tags.

Previous Behavior

$ docker pull registry/image:latest
...
# update the image in the registry
$ docker pull registry/image:latest
... # pulls new image
$ docker images
REPOSITORY                          TAG                 IMAGE ID            CREATED              SIZE
registry/image               latest              b510b2249009        About a minute ago   33.23 MB
registry/image               <none>              888075fb7fb6        About an hour ago    33.23 MB

     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/var/tmp/.prod_0615df_salt/py2/salt/state.py", line 1723, in call
                  **cdata['kwargs'])
                File "/var/tmp/.prod_0615df_salt/py2/salt/loader.py", line 1650, in wrapper
                  return f(*args, **kwargs)
                File "/var/tmp/.prod_0615df_salt/py2/salt/states/dockerng.py", line 1519, in running
                  if image not in __salt__['dockerng.list_tags']():
                File "/var/tmp/.prod_0615df_salt/py2/salt/modules/dockerng.py", line 2240, in list_tags
                  for repo_tag in item.get('RepoTags', []):
              TypeError: 'NoneType' object is not iterable

Tests written?

No

[cachedout]

Thanks, @farcaller

[hjc]

This affects more than just dockerng.list_tags, I had this cause dockerng.image_present to fail =/.

Any ETA on when this will get released as a bugfix for 2016.3?

For those who run into this issue, tagging the images missing tags will fix it:

hjc1710@app-int01:/opt/app$ sudo salt-call  state.highstate
# fails with the above error
hjc1710@app-int01:/opt/app$ docker images
REPOSITORY    TAG                 IMAGE ID            CREATED             SIZE
croscon/app   int                 26ad75aa118b        29 minutes ago      1.86 GB
croscon/app   <none>              57c6e7e2d3ae        10 hours ago        1.859 GB
...
hjc1710@app-int01:/opt/app$ docker tag 57c6e7e2d3ae croscon/app:flkasd
hjc1710@app-int01:/opt/app$ sudo salt-call  state.highstate
# proceeds to now work; clean up tags

[farcaller]

It also causes dockerng.running to fail, heh. All the stack is broken is you have untagged images, which is fun. I get lots of those, typically resting there for days until GC kicks in.

[ajslater]

as @hjc1710 notices in dockerng.image_present:

File "/usr/lib/python2.7/dist-packages/salt/modules/dockerng.py", line 2253, in list_tags
    for repo_tag in item.get('RepoTags', []):
        TypeError: 'NoneType' object is not iterable

This happens even though I have no untagged images. So there is no known workaround.
using: 2016.3.2+ds-1

[hjc]

This is the state I've been using to work around this issue: https://gist.github.com/hjc1710/8192e95dd8adc0069bea14a05360da4f.

The main thing to note is that it has to be run after ANY state that might untag an image. For me, this is pulling a new image, plus an image retag I have to do for organizational reasons. The only other thing missing from it, is that I require it in my dockerng.running states that get run after the image retag.

If the image_present state is all that will update tags, then you can just require it once, after that state and it should work.

The only caveat is that those images are no longer dangling, so common cleanup tools like docker rmi $(docker images -q -f "dangling=true") won't work anymore. I intend to sweep through and cleanup my images later, after this is fixed. Oh, and it's hacky as shit =/, but it works.

Also, now that I see @ajslater's output, I think this bug might not really be solved. I definitely have the pre-PR code; but he has the latest code and is still getting an error, might be something missed here!

[clinta]

I pulled this in and it did not fix the bug for me. The following does fix it, though I'll admit I don't quite understand why it's necessary. But for some reason it seems like item.get is still returning none instead of the default empty list.

def list_tags():
    '''
    Returns a list of tagged images

    CLI Example:

    .. code-block:: bash

        salt myminion dockerng.list_tags
    '''
    ret = set()
    for item in six.itervalues(images()):
        if not item['RepoTags']:
          continue
        for repo_tag in item.get('RepoTags', []):
            ret.add(repo_tag)
    return sorted(ret)

[hjc]

@clinta - this is entirely expected. That's the standard behavior of dictionary.get, it only returns the default if the key is missing. What we have here, is the key is present, but it's value is None, so None is still returned. See this:

>>> foo = {}
>>> foo.get('bar', [])
[]
>>> foo['bar'] = None
>>> print foo.get('bar', [])
None

Basically, dictionary.get is equivalent to:

if key in dict_:
    return dict_[key]
else:
    return default

And not:

if key in dict_ and dict_[key]:
    return dict_[key]
else:
    return default

[farcaller]

I've tried a different approach in #35308, looks cleaner. PTAL?