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

chore: upgrade to docker 25.0.5, remove dependency on dharmadheeraj/moby fork #107

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

haddscot
Copy link
Contributor

Issue #, if available:

Description of changes:

this commit fixes a bug in the CreateLogStream API, but was not included in upstream moby until 23.0.0. now that we have upgraded to 23.0.3, we can remove the dependency on the forked version of moby that contained the fix. this change also bumps us to 25.0.5, since it is the latest stable version.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@haddscot haddscot changed the title upgrade to docker 25.0.5, remove dependency on dharmadheeraj/moby fork chore: upgrade to docker 25.0.5, remove dependency on dharmadheeraj/moby fork Mar 25, 2024
austinvazquez
austinvazquez previously approved these changes Mar 25, 2024
Copy link
Member

@austinvazquez austinvazquez left a comment

Choose a reason for hiding this comment

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

LGTM

@BinSquare
Copy link
Contributor

I may need a refresher on this issue but can we validate assumptions with a test within shim-loggers-for-containerd given the commit seems to refer to CreateLogStream ?

@haddscot
Copy link
Contributor Author

sorry for the confusion – the bug itself was in Fargate, which called the CreateLogStream more than once on task startup. the bug fix commit in moby was to expose a flag in the CreateLogStream API that would allow skipping the creation of the log stream – see here

the only change that was made in shim-loggers was to depend upon a fork of moby that would enable access to that flag before upstream merged, but newer versions of moby now recognize that flag.

@haddscot
Copy link
Contributor Author

haddscot commented Apr 1, 2024

rebased – no other changes

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.

5 participants