Bump default API version to 1.45 (Moby 26.0/26.1)

[thaJeztah]

• similar to Bump default API version to 1.43 (Moby 24.0) #3199

Update API version to the latest maintained release.

[thaJeztah]

We still need to look at raising the minimum version, but I guess that can wait;

We should also start looking at raising this minimum, but I don't know if there's code specific in docker-py that handles differences between versions @milas ?

We're starting to deprecate old API versions, and the initial step is to disable (by default) API versions < 1.24. API 1.24 is the last version of the API before introduction of API version negotiation, so that's the version the CLI falls back to if it's unable to negotiate.

• daemon: raise default minimum API version to v1.24 moby/moby#46887
• ⚠️ there's still some debate on the client-side of that though; client: deprecate old API versions moby/moby#46962

[krissetto]

LGTM

[thaJeztah]

oh! some failure; let me see if it's related

[thaJeztah]

Looks like it may be, and ISTR @robmry looked at some of that, but now wondering if it needs to be API-version aware (or the test rewritten to take both scenarios into account);

=================================== FAILURES ===================================
___________ ContainerCollectionTest.test_run_with_networking_config ____________
tests/integration/models_containers_test.py:136: in test_run_with_networking_config
    assert attrs['NetworkSettings']['Networks'][net_name]['Aliases'] \
E   AssertionError: assert ['hello'] == ['hello', '90b209030388']
E     Right contains one more item: '90b209030388'
E     Use -v to get more diff
_ ContainerCollectionTest.test_run_with_networking_config_only_undeclared_network _
tests/integration/models_containers_test.py:196: in test_run_with_networking_config_only_undeclared_network
    assert attrs['NetworkSettings']['Networks'][net_name]['Aliases'] == [attrs["Id"][:12]]
E   AssertionError: assert None == ['0ed603731bb5']
=========================== short test summary info ============================

[thaJeztah]

☝️ I think this PR was related to that, but looks like that one wasn't merged; maybe it got included in another PR?

• Fix tests that look at 'Aliases' #3233

[krissetto]

I remember seeing # In API version 1.45, the short-id will be removed..
Looks like that didn't happen? 🤔

[thaJeztah]

Wondering if TEST_API_VERSION is passed through everywhere; I see at least one dockerfiles using DOCKER_API_VERSION (not TEST_.....) 🤔

[thaJeztah]

I remember seeing # In API version 1.45, the short-id will be removed.. Looks like that didn't happen? 🤔

Oh; maybe it is removed, and that's the failure? AssertionError: assert None == ['0ed603731bb5']

But the test doesn't have a conditional on API version, so removing it would mean it'd fail on older API versions? 🤔

[krissetto]

I remember seeing # In API version 1.45, the short-id will be removed.. Looks like that didn't happen? 🤔

Oh; maybe it is removed, and that's the failure? AssertionError: assert None == ['0ed603731bb5']

But the test doesn't have a conditional on API version, so removing it would mean it'd fail on older API versions? 🤔

nah, the test seems to be assert ['hello'] == ['hello', '90b209030388'], indicating it's still present (?)

[krissetto]

i don't see it when running a docker inspect locally though, with engine and client at 26.0.1

[thaJeztah]

Tried updating the test, but now getting

=================================== FAILURES ===================================
___________ ContainerCollectionTest.test_run_with_networking_config ____________
tests/integration/models_containers_test.py:136: in test_run_with_networking_config
    assert (attrs['NetworkSettings']['Networks'][net_name]['Aliases']
E   AssertionError: assert ['hello'] is None
_ ContainerCollectionTest.test_run_with_networking_config_only_undeclared_network _
tests/integration/models_containers_test.py:202: in test_run_with_networking_config_only_undeclared_network
    assert attrs['NetworkSettings']['Networks'][net_name]['DNSNames'] \
E   AssertionError: assert None == ['hello', 'e293f5effd7e']

So, looks like that wasn't right 🙈

[thaJeztah]

Getting closer; only one failure remaining;

___________ ContainerCollectionTest.test_run_with_networking_config ____________
tests/integration/models_containers_test.py:136: in test_run_with_networking_config
    assert (attrs['NetworkSettings']['Networks'][net_name]['Aliases']
E   AssertionError: assert ['hello'] is None

[krissetto]

what is our expectation here? meaning, are the tests "wrong" or is the engine's behavior slightly out-of-spec from what we were expecting?

[thaJeztah]

Trying to figure that out honestly; thought let me update the test to see what it actually sees, then check with @robmry and @akerouanton if this is the expected result here. 😂

[thaJeztah]

LOL, and now it's back to;

___________ ContainerCollectionTest.test_run_with_networking_config ____________
tests/integration/models_containers_test.py:138: in test_run_with_networking_config
    assert attrs['NetworkSettings']['Networks'][net_name]['DNSNames'] \
E   AssertionError: assert None == ['hello', '34fe1fead4b8']

Is there some race condition? Or did I just broke it again? (the "expected" vs "actual" part is also confusing me for sure here 😂 )

[milas]

Unfortunately, I don't actually know anything about version negotiation in the Python code.

However, I would imagine it's mostly only ever been exercised in the other direction, e.g. to avoid sending a newly added field/query param to an older daemon.

The test logic might also be more for broad test suite compatibility rather than comprehensive matrix coverage.

(That is, I'm suggesting it's not unreasonable to be somewhat suspicious of both the library code & tests in this regard.)

[thaJeztah]

Oh! Looks like it's green now! Guess it's time for me to squash commits, and to discuss if these changes are all correct with the networking folks 🥹

[thaJeztah]

FWIW, I think I tried adding an assertion for DNSNames here, but that didn't work; wondering if something needs to be added to make that field being propagated here, and if that should be checked for.

For reference, here's what inspecting a container created with similar options looks like (Aliases is null, and both the short-ID and custom hello alias are shown in DNSNames);

docker inspect --format='{{json .NetworkSettings.Networks }}' hello | jq .
{
  "foo": {
    "IPAMConfig": null,
    "Links": null,
    "Aliases": null,
    "MacAddress": "02:42:ac:15:00:02",
    "NetworkID": "d527fac86492c97c660830566d00d410281b8b67cc2f9d85fc60229d5d5d76ee",
    "EndpointID": "4309540d4d79c6fbd21e883c867657bd5550b54fe55d1a623d9858bbf1846ee5",
    "Gateway": "172.21.0.1",
    "IPAddress": "172.21.0.2",
    "IPPrefixLen": 16,
    "IPv6Gateway": "",
    "GlobalIPv6Address": "",
    "GlobalIPv6PrefixLen": 0,
    "DriverOpts": null,
    "DNSNames": [
      "hello",
      "e9e430344c84"
    ]
  }
}