Update Docker dependencies#4957
Conversation
| memInfo, err := meminfo.Read() | ||
| mem := int32(0) | ||
| fs, err := procfs.NewFS("/proc") | ||
| if err == nil { |
There was a problem hiding this comment.
| if err == nil { | |
| if err != nil { |
There was a problem hiding this comment.
And it needs some kind of early return, right?
There was a problem hiding this comment.
Also, it looks like (thanks to the vendor/ diff 😬) that the old docker dependency supported Windows, and this new prom one might not. I have absolutely no clue personally about Windows, other than I know ECS notionally supports it. There are certainly files in this repo that target Windows.
There was a problem hiding this comment.
Hmm, Interesting, I didn't think about Windows compatibility because I thought all container support on Windows was done via WSL / Linux VMs. So procfs would be there.
There was a problem hiding this comment.
How do you feel about manually vendoring in the pkg/meminfo from moby? The code is simple, the license is compatible. This would avoid having to pull in all of the upstream moby go.mod dependencies just to pick up one package.
There was a problem hiding this comment.
Changes LGTM now. Of course the vendoring bit is up to the maintainers.
Replace the deprecated `github.com/docker/docker` Go modules with the new `github.com/moby/moby/api`. Use `github.com/prometheus/procfs` to read `/proc/meminfo` so we don't have to pull in the whole `github.com/moby/moby` repo. Signed-off-by: SuperQ <superq@gmail.com>
|
Thanks for contributing @SuperQ! Could you switch the destination branch to point to |
|
Done, now |
| // StatsResponse is the v4 Stats response for a container. | ||
| type StatsResponse struct { | ||
| *types.StatsJSON | ||
| *container.StatsResponse |
There was a problem hiding this comment.
Since we're going from types.StatsJSON to container.StatsResponse, let's also make similar changes in agent/handlers/v4/ (this may cascade to a few other files that needs to be updated as well)
There was a problem hiding this comment.
Oof, I just looked through the agent directory, it has an absolute ton of github.com/docker/docker use.
Much regret.
Signed-off-by: SuperQ <superq@gmail.com>
3 participants
Summary
Replace the deprecated
github.com/docker/dockerGo modules with the newgithub.com/moby/moby/api.Use
github.com/prometheus/procfsto read/proc/meminfoso we don't have to pull in the wholegithub.com/moby/mobyrepo.Implementation details
Testing
New tests cover the changes:
Description for the changelog
Additional Information
Does this PR include breaking model changes? If so, Have you added transformation functions?
Does this PR include the addition of new environment variables in the README?
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.