Generate stable state for cache

[piotrminkina]

Relates to: #431

Rather than relying on the output of the docker-compose ps command, where the output from different versions may vary, I suggest for this command to limit itself to only retrieving a list of container IDs. The ID list thus obtained is then passed to the docker inspect command, whose output format has always been JSON. Then on the results of the docker inspect command build the state for the cache.

I think there is nothing to go further in parsing the results of the docker-compose ps command. A recent fix was made by @tom-wozniakowski-tw in #468, but that didn't ultimately solve the state stability problem either.

[augi]

Thank you! Are you please sure that the docker inspect really doesn't contain any value that would be changed between executions?

[piotrminkina]

Thank you! Are you please sure that the docker inspect really doesn't contain any value that would be changed between executions?

The result of docker inspect had the key Log, in the path .State.Health, which was variable. This case is solved by calling .excludeFieldsByName('Log'). In my case, without additional sorting, the .HostConfig.Links list returned differently sorted results each time, but I fixed this problem and for all lists using .addConverter(List), in which I perform conditional sorting. In the body of this method I avoid sorting for the main list (key == null) and the list in the key Args, while any other list will be sorted.

I can't give assurance that I covered all cases with this piece of code, while it solved the problem with the order of published ports that I had. There are certainly a great many cases controlled here that could force a restart of the service stack. It would be useful for someone else to test this at their site, because as far as I can see, there is a lack of automated testing for a piece of code?

[augi]

Thanks for the explanation 👍 Regarding the tests, there is no direct test, but there are these two tests for the reconnecting feature (the state-cache is used under-the-hood).

[piotrminkina]

@augi So what do we do with this PR next?

[augi]

I would like to see a real value for the cache - grab it manually or even better create a new test for it. The reason for this is that I'm afraid that this change could be even worse than the current state because there could be more volatile field in the docker inspect output.

I personally don't have time for this now (~this month), so if you are willing to do it, it would be great 🙏

[piotrminkina]

@augi I wrote a script in Groovy that should illuminate whether our concerns about the stability of the results of the docker inspect command are correct. The script should be saved in a file of any name and should be given execution rights or run via the groovy command. I suggest that anyone willing to test it should call this command in the following manner.

foo@bar:~$ docker ps -aq | xargs ./script.groovy

In my case, all tests on 18 containers in various states were successful. When I turned off sorting in the code, quite quickly the tests were terminated with an error, due to differences between states.

#!/usr/bin/env groovy

import groovy.cli.commons.CliBuilder
import groovy.cli.Option
import groovy.cli.Unparsed
import groovy.json.JsonGenerator
import groovy.json.JsonOutput
import groovy.json.JsonSlurper

String firstState, nextState
CliBuilder cli = new CliBuilder(usage: 'groovy script.groovy [OPTIONS] NAME|ID [NAME|ID...]')
Options options = []

cli.parseFromInstance(options, args)
if (options.help || !options.containersIds) {
    cli.usage()
    System.exit(options.help ? 0 : 1)
}

firstState = getStateForCache(options.containersIds)

for (long i = 0; i < options.numberOfTests; i++) {
    print("Test #$i: ")
    nextState = getStateForCache(options.containersIds)
    assert firstState == nextState : 'Unexpected differences between two states detected'
    println('SUCCESS')
    sleep(options.waitForNextTest)
}

static def getStateForCache(String... containersIds) {
    def state = []

    try {
        def containersInfos = new JsonSlurper().parseText(
            ['docker', 'inspect', *containersIds].execute().text
        )
        def jsonGenerator = new JsonGenerator.Options()
            .excludeFieldsByName('Log')
            .addConverter(List) { items, key ->
                // Prevent sorting of objects in the root list and list
                // items in the "Args" key (usually command arguments)
                if (key in [null, 'Args']) {
                    return items
                }

                // Sort the elements of all other lists
                return items.sort()
            }
            .build()

        state += JsonOutput.prettyPrint(
            jsonGenerator.toJson(containersInfos)
        )
    } catch (Throwable t) {
        throw new RuntimeException(
            'Cannot generate inspections for containers, for this reason the state will be incomplete',
            t
        )
    }

    return state.join('\n')
}

class Options {
    @Option(shortName = 'h', longName = 'help', description = 'Show usage information')
    boolean help

    @Option(shortName = 't', longName = 'tests', description = 'Number of comparison tests to be performed')
    long numberOfTests = 100

    @Option(shortName = 'w', longName = 'wait', description = 'Waiting time for next comparison test in ms')
    long waitForNextTest = 0

    @Unparsed(description = 'Names or IDs of containers')
    String[] containersIds
}

[piotrminkina]

I did a test of 100 trials, which lasted for almost 2 minutes.

foo@bar:~$ docker ps -aq | time xargs ./script.groovy -t 100 -w 1000
Test #0: SUCCESS
Test #1: SUCCESS
[...]
Test #98: SUCCESS
Test #99: SUCCESS
6.93user 1.39system 1:43.88elapsed 8%CPU (0avgtext+0avgdata 992608maxresident)k
0inputs+0outputs (0major+637858minor)pagefaults 0swaps
xargs ./script.groovy -t 100 -w 1000  6,94s user 1,40s system 8% cpu 1:43,89 total

[augi]

Wouldn't it be possible to convert it to a new test? To have it reproducible, and also to be able to test it easily in different environments (Docker versions, OSs) 🙏

[piotrminkina]

@augi I could do this if I knew how to perform these tests on my machine. Unfortunately, a simple ./gradlew check ends up with such a result that out of 66 tests as many as 24 end with an error. I guess all of them for this reason: java.lang.RuntimeException: Exit-code 1 when calling docker-compose, stdout: N/A.