Skip to content

client: remove API-version compatibility for API < v1.44#51120

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:raise_fallback_api_step2
Oct 9, 2025
Merged

client: remove API-version compatibility for API < v1.44#51120
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:raise_fallback_api_step2

Conversation

@thaJeztah
Copy link
Member

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added this to the 29.0.0 milestone Oct 7, 2025
@thaJeztah thaJeztah added area/cli Client status/2-code-review kind/refactor PR's that refactor, or clean-up code module/client labels Oct 7, 2025
@thaJeztah thaJeztah force-pushed the raise_fallback_api_step2 branch 2 times, most recently from 53cd29b to 67c2d8e Compare October 7, 2025 09:44
@thaJeztah
Copy link
Member Author

I have some small amount of other patches after this, but more "TBD", which is to make the client fail hard when setting a lower API version, even when done manually (WithVersion, WithVersionFromEnv)

@thaJeztah
Copy link
Member Author

Some tests to look into w.r.t. MacAddress (either test is wrong, or something broke).

We also need to

  • check for integration tests that are overriding API versions (may be easier to find those if we disallow setting old API versions I guess)
  • but if we don't allow old versions; we wouldn't be able to test the daemon downgrading to old versions, so either raise the daemon limit as well,
    or (which we should do either way); document old API versions as "degraded" / "partial support", and better outline what our intent is for old
    API versions.
=== RUN   TestMacAddressIsAppliedToMainNetworkWithShortID
    run_linux_test.go:291: assertion failed: 1a:4c:73:5a:5f:40 (c.NetworkSettings.Networks["testnet"].MacAddress string) != 02:42:08:26:a9:55 (string)
--- FAIL: TestMacAddressIsAppliedToMainNetworkWithShortID (3.82s)
=== RUN   TestInspectCfgdMAC/ctr-wide_address_default_bridge
    mac_addr_test.go:230: assertion failed: 
        --- configMAC
        +++ tc.desiredMAC
          string(
        - 	"",
        + 	"02:42:ac:11:00:42",
          )
        
=== RUN   TestWatchtowerCreate
    mac_addr_test.go:281: assertion failed: 16:91:31:ac:a5:81 (netSettings.MacAddress string) != 02:42:ac:11:00:42 (ctrMAC string)

@thaJeztah thaJeztah force-pushed the raise_fallback_api_step2 branch 3 times, most recently from 052b771 to 229f8c7 Compare October 8, 2025 21:36
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the raise_fallback_api_step2 branch from 229f8c7 to 7652f38 Compare October 8, 2025 21:44
Comment on lines +32 to +48
// FIXME(thaJeztah): remove this once we updated our (integration) tests;
// some integration tests depend on this to test old API versions; see https://github.com/moby/moby/pull/51120#issuecomment-3376224865
if config.MacAddress != "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
// Make sure we negotiated (if the client is configured to do so),
// as code below contains API-version specific handling of options.
//
// Normally, version-negotiation (if enabled) would not happen until
// the API request is made.
if err := cli.checkVersion(ctx); err != nil {
return response, err
}
if versions.GreaterThanOrEqualTo(cli.ClientVersion(), "1.44") {
// Since API 1.44, the container-wide MacAddress is deprecated and triggers a WARNING if it's specified.
//
// FIXME(thaJeztah): remove the field from the API
config.MacAddress = "" //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept this one in place for now, as some integration tests currently depend on this; we can remove this later.

@thaJeztah thaJeztah marked this pull request as ready for review October 9, 2025 01:57
Copy link
Member

@akerouanton akerouanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@robmry robmry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member Author

OK; let's bring this one in 👍

@thaJeztah thaJeztah merged commit a3a6058 into moby:master Oct 9, 2025
211 of 212 checks passed
@thaJeztah thaJeztah deleted the raise_fallback_api_step2 branch October 9, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants