machine-drivers / machine

Machine management for a container-centric world
Apache License 2.0
25 stars 16 forks source link

Replace github.com/docker/docker/pkg/term with github.com/moby/term #40

Closed spowelljr closed 1 year ago

spowelljr commented 1 year ago

Docker 23.0.0 removed github.com/docker/docker/pkg/term so I'm getting build errors when trying to update the Docker version in minikube.

k8s.io/minikube/cmd/minikube/cmd imports
    github.com/docker/machine/libmachine/ssh imports
    github.com/docker/docker/pkg/term: module github.com/docker/docker@latest found (v23.0.1+incompatible), but does not contain package github.com/docker/docker/pkg/term
spowelljr commented 1 year ago

I believe the build error is due to using an old Go version in the CI.

medyagh commented 1 year ago

is the continuous-integration/appveyor/pr failure legit?

spowelljr commented 1 year ago

is the continuous-integration/appveyor/pr failure legit?

The Go version in the CI is too old

# golang.org/x/sys/windows
c:\gopath\pkg\mod\golang.org\x\sys@v0.5.0\windows\syscall.go:84:16: undefined: unsafe.Slice
c:\gopath\pkg\mod\golang.org\x\sys@v0.5.0\windows\syscall_windows.go:131:29: undefined: unsafe.Slice
note: module requires Go 1.17
# golang.org/x/crypto/ssh
c:\gopath\pkg\mod\golang.org\x\crypto@v0.1.0\ssh\cipher.go:499:13: undefined: io.Discard
c:\gopath\pkg\mod\golang.org\x\crypto@v0.1.0\ssh\session.go:508:14: undefined: io.Discard
c:\gopath\pkg\mod\golang.org\x\crypto@v0.1.0\ssh\session.go:521:14: undefined: io.Discard
note: module requires Go 1.17
afbjorklund commented 1 year ago

This change looks enormous, and I'm not sure it's a good idea to update to docker 23.0.x in the deprecated machine ?

Changing from github.com/docker/docker/pkg/term to github.com/moby/term is a much smaller 20.10.x cherry-pick:

https://github.com/moby/moby/commit/41d4112e89473095947b4fb4ebf10e8258e4273c#diff-3ddc2641607200d452c308c4034b2087234c56eb599df9c84e91a9b9f9f7431d

https://github.com/moby/moby/commit/8312004f41e9500824fa16ae991eeee0083f4771

 go.mod                   | 1 +
 go.sum                   | 6 ++++++
 libmachine/ssh/client.go | 2 +-
 3 files changed, 8 insertions(+), 1 deletion(-)
--- a/go.mod
+++ b/go.mod
@@ -26,6 +26,7 @@ require (
        github.com/jinzhu/copier v0.0.0-20180308034124-7e38e58719c3
        github.com/jmespath/go-jmespath v0.0.0-20151117175822-3433f3ea46d9
        github.com/mitchellh/mapstructure v0.0.0-20140721150620-740c764bc614
+       github.com/moby/term v0.0.0-20200416134343-063f2cd0b49d
        github.com/pmezard/go-difflib v1.0.0
        github.com/rackspace/gophercloud v0.0.0-20150408191457-ce0f487f6747
        github.com/samalba/dockerclient v0.0.0-20151231000007-f661dd4754aa
--- a/libmachine/ssh/client.go
+++ b/libmachine/ssh/client.go
@@ -11,7 +11,7 @@ import (
        "strconv"
        "strings"

-       "github.com/docker/docker/pkg/term"
+       "github.com/moby/term"
        "github.com/docker/machine/libmachine/log"
        "github.com/docker/machine/libmachine/mcnutils"
        "golang.org/x/crypto/ssh"
spowelljr commented 1 year ago

@afbjorklund Updated github.com/moby/term to v0.0.0-20200416134343-063f2cd0b49d instead

afbjorklund commented 1 year ago

Right, but it is missing some kind of back-story on why for instance Go and Docker should be updated now ?

There is no CI pipeline and no releases, and the whole organization is mostly for providing machine drivers...

Those things would be needed (including a new boot2docker.iso), if machine is to continue as a separate project.

If not, maybe it is better to do those upgrades in minikube - if there is a desire to go beyond go1.13 and 19.03.x


Other than that, it looks more reasonable. Could be even smaller, if just wanting to get rid of the docker repository...

i.e. backporting the breakout to the ancient 18.04/2019 version of Docker, with the same library versions as the rest.