Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@thaJeztah
Copy link
Member

relates to:

The Commit type was introduced in 2790ac6, to assist triaging issues that were reported with an incorrect version of runc or containerd. At the time, both runc and containerd were not yet stable, and had to be built from a specific commit to guarantee compatibility.

We encountered various situations where unexpected (and incompatible) versions of those binaries were packaged, resulting in hard to trace bug-reports. For those situations, a "expected" version was set at compile time, to indicate if the version installed was different from the expected version;

docker info
...
runc version: a592beb5bc4c4092b1b1bac971afed27687340c5 (expected: 69663f0bd4b60df09991c08812a60108003fa340)

Both runc and containerd are stable now, and docker 19.03 and up set the expected version to the actual version since c65f0bd and 23.0 did the same for the init binary b585c64, to prevent the CLI from reporting "unexpected version".

In short; the Expected fields no longer serves a real purpose.

In future, we can even consider deprecating the ContainerdCommit, RuncCommit and InitCommit fields on the /info response (as we also include this information as part of the components returned in /version), but those can still be useful currently for situations where a user only provides docker info output.

This patch starts with deprecating the Expected field.

- What I did

- How I did it

- How to verify it

- Description for the changelog

Deprecated: The `ContainerdCommit.Expected`, `RuncCommit.Expected`, and  `InitCommit.Expected` fields in the `GET /info` endpoint are deprecated  and will be omitted in API v1.49.

Deprecated: The `api/types/system/Commit.Expected` field is deprecated and should no longer be used.

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

The `Commit` type was introduced in 2790ac6,
to assist triaging issues that were reported with an incorrect version of
runc or containerd. At the time, both `runc` and `containerd` were not yet
stable, and had to be built from a specific commit to guarantee compatibility.

We encountered various situations where unexpected (and incompatible) versions
of those binaries were packaged, resulting in hard to trace bug-reports.
For those situations, a "expected" version was set at compile time, to
indicate if the version installed was different from the expected version;

    docker info
    ...
    runc version: a592beb5bc4c4092b1b1bac971afed27687340c5 (expected: 69663f0bd4b60df09991c08812a60108003fa340)

Both `runc` and `containerd` are stable now, and docker 19.03 and up set the
expected version to the actual version since c65f0bd
and 23.0 did the same for the `init` binary b585c64,
to prevent the CLI from reporting "unexpected version".

In short; the `Expected` fields no longer serves a real purpose.

In future, we can even consider deprecating the `ContainerdCommit`, `RuncCommit`
and `InitCommit` fields on the `/info` response (as we also include this
information as part of the components returned in `/version`), but those
can still be useful currently for situations where a user only provides
`docker info` output.

This patch starts with deprecating the `Expected` field.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the deprecate_info_expected_version branch from 48867ca to ff191c5 Compare September 12, 2024 17:40
Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM

description: |
Commit ID of external tool expected by dockerd as set at build time.
**Deprecated**: This field is deprecated and will be omitted in a API v1.49.
Copy link
Member

Choose a reason for hiding this comment

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

This is in a response, right? So to actually omit it in 1.49+ we'll have to do something like omitempty on it (or a split/separate struct)? Would we then just make sure the code setting the value is conditional? (Making sure we're planning for how that's going to work/happen, and maybe even how we'll remember to implement that since the implementation isn't included in this PR that I can see 😅 perhaps it could be, since we do already know which API version it should be removed in?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this PR only did the deprecation part, but I had follow-up changes stashed to handle the omitempty and to make it conditional; that still required an API version bump, so didn't push those changes yet, and I kept the omitempty out, in case there would be contention around "omit" or "keep, but empty".

Just pushed a WIP branch with those stashed changes if you're interested in the approach I had in mind 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

@tianon tianon merged commit b5d04e4 into moby:master Sep 30, 2024
@thaJeztah thaJeztah deleted the deprecate_info_expected_version branch September 30, 2024 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.