Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go.mod: update to docker v28.0.0 #12545

Merged
merged 5 commits into from
Feb 21, 2025
Merged

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Feb 10, 2025

pkg/compose: stopDependentContainers: rename var that shadowed

go.mod: golang.org/x/exp v0.0.0-20241108190413-2d47ceb2692f

go.mod: github.com/moby/buildkit v0.20.0

go.mod: docker/docker, docker/cli v28.0.0, buildx v0.21.0

full diff:

What I did

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

@thaJeztah thaJeztah force-pushed the bump_buildx_engine branch 3 times, most recently from 42dc745 to f055253 Compare February 10, 2025 15:32
MacAddress: macAddress,
MacAddress: macAddress, // FIXME(thaJeztah) this should be moved to networksettings -> EndpointSettings.MacAddress
Copy link
Member Author

Choose a reason for hiding this comment

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

Note to self; looks like we need to update this to use the new field

Copy link
Member Author

Choose a reason for hiding this comment

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

@robmry @akerouanton could one of you help doing a follow-up to adjust the code in compose for the new approach? (I wasn't 100% sure if "only new" should be set, or "both old and new to same value")

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, sure - "only new" should do the trick, the old field is deprecated.

For now, we migrate the container-wide MAC address to the endpoint if it's unambiguous. But, we'll remove that at some point.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I wasn't sure what would happen if we did NOT set the old one, if compose would connect to an older version of the engine, but perhaps we take care of that in the client

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I guess we'll have to ... the change was in API 1.44.

Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't need a FIXME, we just support older API and migrate to new API when relevant (see prepareContainerMACAddress)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! I missed that one; in that case, I may add a //nolint: and a comment to prevent confusion; that sound good?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added; let me know if that looks good (thanks!)

@thaJeztah thaJeztah force-pushed the bump_buildx_engine branch 6 times, most recently from c0f1494 to 5b006b2 Compare February 11, 2025 17:59
@thaJeztah thaJeztah closed this Feb 11, 2025
@thaJeztah thaJeztah reopened this Feb 11, 2025
@thaJeztah thaJeztah force-pushed the bump_buildx_engine branch 2 times, most recently from 6f5440e to cfd04b3 Compare February 15, 2025 20:39
@thaJeztah thaJeztah changed the title WIP: go.mod: update to docker v28.0.0-rc.1 WIP: go.mod: update to docker v28.0.0-rc.2 Feb 15, 2025
@thaJeztah thaJeztah force-pushed the bump_buildx_engine branch 4 times, most recently from c27b3a5 to 4da4898 Compare February 17, 2025 21:23
@thaJeztah thaJeztah changed the title WIP: go.mod: update to docker v28.0.0-rc.2 go.mod: update to docker v28.0.0-rc.2 Feb 17, 2025
@thaJeztah thaJeztah changed the title go.mod: update to docker v28.0.0-rc.2 go.mod: update to docker v28.0.0-rc.3 Feb 19, 2025
@ndeloof
Copy link
Contributor

ndeloof commented Feb 20, 2025

I can't push to your fork so created #12564

@thaJeztah thaJeztah changed the title go.mod: update to docker v28.0.0-rc.3 go.mod: update to docker v28.0.0 Feb 20, 2025
@thaJeztah thaJeztah force-pushed the bump_buildx_engine branch 2 times, most recently from 4c26249 to 1d6ee91 Compare February 20, 2025 10:43
@thaJeztah thaJeztah marked this pull request as ready for review February 20, 2025 10:43
@thaJeztah thaJeztah requested a review from a team as a code owner February 20, 2025 10:43
@thaJeztah thaJeztah requested review from ndeloof and glours February 20, 2025 10:43
@thaJeztah
Copy link
Member Author

Updated to v28.0.0 and included your fix from 152490f @ndeloof

@thaJeztah
Copy link
Member Author

I can't push to your fork

Also not sure why that didn't work 🤔 something broken on GitHub?

Screenshot 2025-02-20 at 11 45 08

Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

Sounds good to me, thanks @thaJeztah and @ndeloof 👍

@thaJeztah
Copy link
Member Author

Derp; I must've had a different version of the linter running locally that did complain 😂

#22 114.9 pkg/compose/create.go:277:32: directive `//nolint:staticcheck // Deprecated since API v1.44, but kept for compatibility with older API versions.` is unused for linter "staticcheck" (nolintlint)

@thaJeztah
Copy link
Member Author

OK: fixed now 👍

@thaJeztah
Copy link
Member Author

Rebased to get a fresh run of CI

@thaJeztah
Copy link
Member Author

oh! buildx 0.21.1 is there; I can update 😂 docker/docker-ce-packaging#1167

thaJeztah and others added 2 commits February 21, 2025 15:13
Seems like it resolves the alias, and uses that instead, instead of
the actual type.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Member Author

oh! buildx 0.21.1 is there; I can update 😂 docker/docker-ce-packaging#1167

Done!

docker/buildx@v0.21.0...v0.21.1

@ndeloof ndeloof enabled auto-merge (rebase) February 21, 2025 14:22
@ndeloof ndeloof merged commit 20f780e into docker:main Feb 21, 2025
24 of 25 checks passed
@thaJeztah thaJeztah deleted the bump_buildx_engine branch February 21, 2025 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants