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

Remove all minikube dependencies from drivers #4933

Merged
merged 16 commits into from
Aug 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ out/minikube-installer.exe: out/minikube-windows-amd64.exe
mv out/windows_tmp/minikube-installer.exe out/minikube-installer.exe
rm -rf out/windows_tmp

out/docker-machine-driver-hyperkit: pkg/minikube/translate/translations.go
out/docker-machine-driver-hyperkit:
ifeq ($(MINIKUBE_BUILD_IN_DOCKER),y)
$(call DOCKER,$(HYPERKIT_BUILD_IMAGE),CC=o64-clang CXX=o64-clang++ /usr/bin/make $@)
else
Expand Down Expand Up @@ -441,18 +441,18 @@ release-minikube: out/minikube checksum
gsutil cp out/minikube-$(GOOS)-$(GOARCH) $(MINIKUBE_UPLOAD_LOCATION)/$(MINIKUBE_VERSION)/minikube-$(GOOS)-$(GOARCH)
gsutil cp out/minikube-$(GOOS)-$(GOARCH).sha256 $(MINIKUBE_UPLOAD_LOCATION)/$(MINIKUBE_VERSION)/minikube-$(GOOS)-$(GOARCH).sha256

out/docker-machine-driver-kvm2: pkg/minikube/translate/translations.go
out/docker-machine-driver-kvm2:
ifeq ($(MINIKUBE_BUILD_IN_DOCKER),y)
docker inspect -f '{{.Id}} {{.RepoTags}}' $(KVM_BUILD_IMAGE) || $(MAKE) kvm-image
$(call DOCKER,$(KVM_BUILD_IMAGE),/usr/bin/make $@ COMMIT=$(COMMIT))
# make extra sure that we are linking with the older version of libvirt (1.3.1)
test "`strings $@ | grep '^LIBVIRT_[0-9]' | sort | tail -n 1`" = "LIBVIRT_1.2.9"
else
go build \
-installsuffix "static" \
-ldflags="$(KVM2_LDFLAGS)" \
-tags "libvirt.1.3.1 without_lxc" \
-o $(BUILD_DIR)/docker-machine-driver-kvm2 \
go build \
-installsuffix "static" \
-ldflags="$(KVM2_LDFLAGS)" \
-tags "libvirt.1.3.1 without_lxc" \
-o $(BUILD_DIR)/docker-machine-driver-kvm2 \
k8s.io/minikube/cmd/drivers/kvm
endif
chmod +X $@
Expand Down
33 changes: 25 additions & 8 deletions pkg/drivers/hyperkit/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ import (
hyperkit "github.com/moby/hyperkit/go"
"github.com/pkg/errors"
pkgdrivers "k8s.io/minikube/pkg/drivers"

"k8s.io/minikube/pkg/minikube/constants"
commonutil "k8s.io/minikube/pkg/util"
)

const (
Expand Down Expand Up @@ -77,7 +74,6 @@ func NewDriver(hostName, storePath string) *Driver {
SSHUser: "docker",
},
CommonDriver: &pkgdrivers.CommonDriver{},
DiskSize: commonutil.CalculateSizeInMB(constants.DefaultDiskSize),
}
}

Expand Down Expand Up @@ -121,7 +117,7 @@ func (d *Driver) Create() error {

// DriverName returns the name of the driver
func (d *Driver) DriverName() string {
return constants.DriverHyperkit
return "hyperkit"
Copy link
Member

Choose a reason for hiding this comment

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

can we make it a constant inside the driver?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I disagree :-P

}

// GetSSHHostname returns hostname for use with ssh
Expand Down Expand Up @@ -227,7 +223,7 @@ func (d *Driver) Start() error {
h.Memory = d.Memory
h.UUID = d.UUID
// This should stream logs from hyperkit, but doesn't seem to work.
logger := golog.New(os.Stderr, constants.DriverHyperkit, golog.LstdFlags)
logger := golog.New(os.Stderr, "hyperkit", golog.LstdFlags)
h.SetLogger(logger)

if vsockPorts, err := d.extractVSockPorts(); err != nil {
Expand Down Expand Up @@ -269,12 +265,25 @@ func (d *Driver) Start() error {

d.IPAddress, err = GetIPAddressByMACAddress(mac)
if err != nil {
return &commonutil.RetriableError{Err: err}
return &tempError{err}
}
return nil
}

if err := commonutil.RetryAfter(30, getIP, 2*time.Second); err != nil {
// Implement a retry loop without calling any minikube code
for i := 0; i < 30; i++ {
log.Debugf("Attempt %d", i)
err = getIP()
if err == nil {
break
}
if _, ok := err.(*tempError); !ok {
return err
}
time.Sleep(2 * time.Second)
}

if err != nil {
return fmt.Errorf("IP address never found in dhcp leases file %v", err)
}
log.Debugf("IP: %s", d.IPAddress)
Expand All @@ -294,6 +303,14 @@ func (d *Driver) Start() error {
return nil
}

type tempError struct {
Err error
}

func (t tempError) Error() string {
return "Temporary error: " + t.Err.Error()
}

//recoverFromUncleanShutdown searches for an existing hyperkit.pid file in
//the machine directory. If it can't find it, a clean shutdown is assumed.
//If it finds the pid file, it checks for a running hyperkit process with that pid
Expand Down
12 changes: 1 addition & 11 deletions pkg/drivers/kvm/kvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ import (
"syscall"
"time"

"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/constants"
"k8s.io/minikube/pkg/util"

"github.com/docker/machine/libmachine/drivers"
"github.com/docker/machine/libmachine/log"
"github.com/docker/machine/libmachine/state"
Expand Down Expand Up @@ -102,14 +98,8 @@ func NewDriver(hostName, storePath string) *Driver {
SSHUser: "docker",
},
CommonDriver: &pkgdrivers.CommonDriver{},
Boot2DockerURL: constants.DefaultISOURL,
CPU: constants.DefaultCPUS,
DiskSize: util.CalculateSizeInMB(constants.DefaultDiskSize),
Memory: util.CalculateSizeInMB(constants.DefaultMemorySize),
PrivateNetwork: defaultPrivateNetworkName,
Network: defaultNetworkName,
DiskPath: filepath.Join(constants.GetMinipath(), "machines", config.GetMachineName(), fmt.Sprintf("%s.rawdisk", config.GetMachineName())),
ISO: filepath.Join(constants.GetMinipath(), "machines", config.GetMachineName(), "boot2docker.iso"),
ConnectionURI: qemusystem,
}
}
Expand Down Expand Up @@ -224,7 +214,7 @@ func (d *Driver) GetSSHHostname() (string, error) {

// DriverName returns the name of the driver
func (d *Driver) DriverName() string {
return constants.DriverKvm2
return "kvm2"
Copy link
Member

Choose a reason for hiding this comment

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

can we make it a constant inside the driver?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But why

Copy link
Member

Choose a reason for hiding this comment

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

@afbjorklund As those are the driver names I consider they good for a constant. To always have only one place to change it, even though now we don't have many. But as always, no strong feelings. and probably a too simple thing to start a big discussion, so all good to revert it to string only :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, at least it's not using the constants module. But the string is only used in 1 or 2 places ?

It's the not the most important thing, just thought it was an unnecessary step of indirection.

}

// Kill stops a host forcefully, including any containers that we are managing.
Expand Down