Develop way of handling multiple docker versions (flags)

[ldoktor]

Find a way to easily specify flags, which are named differently in various docker versions, eg.:

docker rmi --stop
docker rmi --force

[ldoktor]

Hi guys, I thought about it and now became suspicious that I over-engineer that... These are my findings: 1) Hyper-super-cool-automated version:

from autotest.client.shared import utils
from autotest.client.shared.error import CmdError
from dockertest.xceptions import DockerCommandError

# This should be in dockertest.version.py
class VersionableArgs(object):
    """
    Universal cusomizable class for args handling
    :param subtest: SubBase
    :param subcmd: Command to obtain desired testing output
    :param modifiers: dict of special values: {$NAMED_KEY: $MODIFIER, ...}
                where $MODIFIER is compound of
                ($DEFAULT, ($CHECK1, $VALUE1), ($CHECK2, $VALUE2), ...)
    :param params: Initial params added after init
    """
    def __init__(self, subtest, subcmd, modifiers, params=None):
        self._subtest = subtest
        self._subcmd = subcmd
        self._stdout = None
        self.custom_args = []
        self.named_keys = []
        self.named_values = []
        self._modifiers = modifiers
        if params:
            for param in params:
                self.add(param)

    @property
    def stdout(self):
        """
        Returns the subcmds stdout (with caching)
        """
        if not self._stdout:
            #self._stdout = self._cmd
            try:
                cmd = "%s %s" % (self._subtest.config['docker_path'],
                                 self._subcmd)
                self._stdout = utils.run(cmd, timeout=10, ignore_status=True,
                                         verbose=False)
            except CmdError, detail:
                raise DockerCommandError(detail.command, detail.result_obj,
                                     additional_text=detail.additional_text)
        return self._stdout

    def _apply_modifiers(self, param):
        """
        Applies the modifier and returns the new value
        """
        for key in self._modifiers[param][1:]:
            if key[0] in self.stdout:
                return key[1]
        return self._modifiers[param][0]

    def _add_named(self, param, onoff):
        """
        Add named variant (for simple removal)
        """
        value = self._apply_modifiers(param)
        if onoff is True:
            value += "=true"
        elif onoff is False:
            value += "=false"
        self.named_keys.append(param)
        self.named_values.append(value)

    def add(self, param, onoff=None):
        """
        Add new param (named ones will be replaced according to version)
        """
        if param in self._modifiers:
            self._add_named(param, onoff)
        else:
            self.custom_args.append(param)

    def rm(self, param):
        """
        Remove param (named or actual value)
        """
        if param in self.named_keys:
            idx = self.named_keys.index()
            self.named_keys = self.named_keys[:idx] + self.named_keys[idx + 1:]
            self.named_values = (self.named_values[:idx]
                                 + self.named_values[idx + 1:])
        elif param in self.custom_args:
            self.custom_args.pop(param)
        elif param in self.named_values:
            idx = self.named_values.index()
            self.named_keys = self.named_keys[:idx] + self.named_keys[idx + 1:]
            self.named_values = (self.named_values[:idx]
                                 + self.named_values[idx + 1:])
        else:
            raise KeyError

    def __contains__(self, param):
        """
        Whether the param is set or not.
        """
        if param in self.named_keys:
            return True
        elif param in self.custom_args:
            return True
        elif param in self.named_values:
            return True
        return False

    def __str__(self):
        """
        cmdline
        """
        return " ".join(self.custom_args + self.named_values)

# This in dockertest.images
class ImagesArgs(VersionableArgs):
    def __init__(self, handler, params=None):
        modifier = {'FORCE': ('--force',
                              ('--stop', '--stop')
                              )
                    }
        super(ImagesArgs, self).__init__(handler, modifier, params)

# this will be instead of DockerImages.images_args
class DockerImagesCLI(DockerImagesBase):
    ...
    #: Arguments to use when listing images
    images_args = ImagesArgs(["--no-trunc"])

# in test you'd just do:
di = DockerImages()
di.images_args.add('FORCE')
...
di.images_args.rm("FORCE")

# Another example usage (eg. custom commands)
aaa = ImagesArgs('--stop')
bbb = ImagesArgs('')
aaa.add('FORCE')
aaa.add('--asdf')
print aaa
bbb.add('--asdf')
bbb.add('FORCE', False)
print bbb

2) Less cool, only for DockerImages:

class DockerImagesCLI(DockerImagesBase):
    ...
    #: Arguments to use when listing images
    _images_args = "--no-trunc"

    @property
    def images_args(self, value):
        # series of if-then-else

3) Even less cool, universal

# in dockertest.version
def parse_image_args(...):
    # if-then-else which modifies the values...

Every version has it's benefits, IMO the first one could be easily updatable, but for now less-complicated solution could be also used. Please take a look and let me know your ideas...

[cevich]

We probably need the second one (and in DockerContainers too) in any case.

I like the first one, but I'm affraid it's going to need too much debugging, unittesting, review, etc. time. I also expect the version-specific handlers are going to see some amount of churn, so it would be nice to have them in config.. You're third one gave me an idea I think could be even more universal and a bit easier to write unittests for:

    # dockertest.version
    class DependantBehavior(object):

        # subthing: subtest | subsubtest
        def __init__(self, subthing, dependant_version_key):
            ...
                        self.dep_ver = LooseVersion(subthing.config[dependant_version_key])

        def __call__(self):
            cur_ver = LooseVersion(self.current_version())
            if cur_ver == dep_ver:
                return self.equal_to_func(subthing)
            elif cur_ver < dep_ver:
                return self.less_than_func(subthing)
            ...

        @property
        def current_version(self):
            # look up using self.subthing

        @staticmethod
        def lower_func(subthing):
            raise NotImplementedError

        #...@staticmethod for equal_to_func and greater_func, 

    class RmForceStop(DependantBehavior):

        def __init__(self, subthing, dependant_version="verdep_rm_force_stop"):
            ...
            self.equal_func = self.lower_func = lambda: return "--force"
            self.greater_func = lambda: return "--stop"

    # defaults.ini

Then your number two solution can pass through self.subtest (which it already knows about) and if a test needs the string for a (*)DockerCmd it can just go right to version module. I expect sometime in the 0.8.2 timeframe, we're going to be faced with adding higher-level things like dexpect, jiri's tooling, and whatnot. You're first solution could fit in there and make use of the above for it's plumbing...

...though maybe even this is too complicated...hmmm...we do need something quick, and number 3 would do the trick. We can always deprecate & replace it later with something more fancy I guess. The unittests would also be nearly trivial to write. I guess it's up to you depending on how many unittests you want to write, I'm fine with a stop-gap solution for now.

[cevich]

(See even I make stuff too 'cool' on the first try) :grinning:

[cevich]

FYI: It looks like this may have been revered, now I only see '--force' in help and it no longer gives a deprecation warning. Haven't investigated deeper, just noticed by chance.

[ldoktor]

Good to know. Probably we might postpone the actual implementation and do it right when there is a need for it. And don't worry, there will be need once we release stable version and simultaneously support older ones and new ones...

[cevich]

oh ya, it's certainly already a problem. There are already a few tests which check and do it themselves. Dealing with multiple versions of docker is just a reality, but your right, we do have a little more time now which is good. Let's move this out to the 0.8.x series so we ca focus and get 0.7.7 out the door and start the 0.7.8 clean/bug/doc branch.

[cevich]

Moving to 0.8.2

[ldoktor]

Is this still valid? Last update was on 2016, let's close it.

[cevich]

Hehe, forgot we even had open issues. Closing all of them...