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.

Sign up for GitHub

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

Various fixes for the none driver #4778

Closed
wants to merge 3 commits into from
Closed

Various fixes for the none driver #4778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e0844c
Only provision buildroot with BuildrootProvisioner
afbjorklund Jul 14, 2019
1f09590
Provide default InstallURL in EngineOptions
afbjorklund Jul 14, 2019
1b5a527
Don't try to start/stop drivers without VMs
afbjorklund Jul 15, 2019
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
40 changes: 24 additions & 16 deletions cmd/minikube/cmd/stop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package cmd

import (
"os"
"time"

"github.com/docker/machine/libmachine/mcnerror"
Expand Down Expand Up @@ -51,24 +52,31 @@ func runStop(cmd *cobra.Command, args []string) {
}
defer api.Close()

nonexistent := false
cc, err := pkg_config.Load()
if err != nil && !os.IsNotExist(err) {
console.ErrLn("Error loading profile config: %v", err)
}

if err == nil && cc.MachineConfig.VMDriver != constants.DriverNone {
nonexistent := false

stop := func() (err error) {
err = cluster.StopHost(api)
switch err := errors.Cause(err).(type) {
case mcnerror.ErrHostDoesNotExist:
console.OutStyle(console.Meh, "%q VM does not exist, nothing to stop", profile)
nonexistent = true
return nil
default:
return err
stop := func() (err error) {
err = cluster.StopHost(api, cc.MachineConfig)
switch err := errors.Cause(err).(type) {
case mcnerror.ErrHostDoesNotExist:
console.OutStyle(console.Meh, "%q VM does not exist, nothing to stop", profile)
nonexistent = true
return nil
default:
return err
}
}
if err := pkgutil.RetryAfter(5, stop, 2*time.Second); err != nil {
exit.WithError("Unable to stop VM", err)
}
if !nonexistent {
console.OutStyle(console.Stopped, "%q stopped.", profile)
}
}
if err := pkgutil.RetryAfter(5, stop, 2*time.Second); err != nil {
exit.WithError("Unable to stop VM", err)
}
if !nonexistent {
console.OutStyle(console.Stopped, "%q stopped.", profile)
}

if err := cmdUtil.KillMountProcess(); err != nil {
Expand Down
44 changes: 29 additions & 15 deletions pkg/minikube/cluster/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"time"

"github.com/docker/machine/libmachine"
"github.com/docker/machine/libmachine/drivers"
"github.com/docker/machine/libmachine/engine"
"github.com/docker/machine/libmachine/host"
"github.com/docker/machine/libmachine/mcnerror"
Expand Down Expand Up @@ -116,15 +117,24 @@ func StartHost(api libmachine.API, config cfg.MachineConfig) (*host.Host, error)
return nil, errors.Wrap(err, "Error getting state for host")
}

if s == state.Running {
console.OutStyle(console.Running, "Re-using the currently running %s VM for %q ...", h.Driver.DriverName(), cfg.GetMachineName())
} else {
console.OutStyle(console.Restarting, "Restarting existing %s VM for %q ...", h.Driver.DriverName(), cfg.GetMachineName())
if err := h.Driver.Start(); err != nil {
return nil, errors.Wrap(err, "start")
if config.VMDriver != constants.DriverNone {
if s == state.Running {
console.OutStyle(console.Running, "Re-using the currently running %s VM for %q ...", h.Driver.DriverName(), cfg.GetMachineName())
} else {
console.OutStyle(console.Restarting, "Restarting existing %s VM for %q ...", h.Driver.DriverName(), cfg.GetMachineName())
if err := h.Driver.Start(); err != nil {
return nil, errors.Wrap(err, "start")
}
if err := api.Save(h); err != nil {
return nil, errors.Wrap(err, "save")
}

}
if err := api.Save(h); err != nil {
return nil, errors.Wrap(err, "save")
} else {
if s == state.Running {
console.OutStyle(console.Running, "Re-using the currently running %s VM for %q ...", h.Driver.DriverName(), cfg.GetMachineName())
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this say VM?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Most people should be running none on a dedicated VM, but yeah - we normally use "VM" as a term for the host, in the case of "none" or "generic" or "docker" that might not actually be true.

Normally it's called a machine, as in "something runs docker".

} else {
exit.WithCode(exit.Unavailable, "Unable to reach %s VM for %q", h.Driver.DriverName(), cfg.GetMachineName())
}
}

Expand All @@ -149,6 +159,7 @@ func configureHost(h *host.Host, e *engine.Options) error {
if err != nil {
return errors.Wrap(err, "detecting provisioner")
}
glog.Infof("Provisioning with %s...", provisioner.String())
if err := provisioner.Provision(*h.HostOptions.SwarmOptions, *h.HostOptions.AuthOptions, *h.HostOptions.EngineOptions); err != nil {
return errors.Wrap(err, "provision")
}
Expand Down Expand Up @@ -231,18 +242,20 @@ func trySSHPowerOff(h *host.Host) {
}

// StopHost stops the host VM, saving state to disk.
func StopHost(api libmachine.API) error {
func StopHost(api libmachine.API, config cfg.MachineConfig) error {
host, err := api.Load(cfg.GetMachineName())
if err != nil {
return errors.Wrapf(err, "load")
}
console.OutStyle(console.Stopping, "Stopping %q in %s ...", cfg.GetMachineName(), host.DriverName)
if err := host.Stop(); err != nil {
alreadyInStateError, ok := err.(mcnerror.ErrHostAlreadyInState)
if ok && alreadyInStateError.State == state.Stopped {
return nil
if config.VMDriver != constants.DriverNone {
console.OutStyle(console.Stopping, "Stopping %q in %s ...", cfg.GetMachineName(), host.DriverName)
if err := host.Stop(); err != nil {
alreadyInStateError, ok := err.(mcnerror.ErrHostAlreadyInState)
if ok && alreadyInStateError.State == state.Stopped {
return nil
}
return &util.RetriableError{Err: errors.Wrapf(err, "Stop: %s", cfg.GetMachineName())}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If "stop" is unsupported by none, I think StopHost should return an unsupported error instead of nothing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, StopHost is unsupported - but we should try to stop Kubernetes. Just TBD.

Maybe I should add a stopKubernetes() function, similar to our uninstallKubernetes()

And just make it empty or log something friendly...

The problem is described in that other issue linked.

return &util.RetriableError{Err: errors.Wrapf(err, "Stop: %s", cfg.GetMachineName())}
}
return nil
}
Expand Down Expand Up @@ -314,6 +327,7 @@ func engineOptions(config cfg.MachineConfig) *engine.Options {
InsecureRegistry: append([]string{pkgutil.DefaultServiceCIDR}, config.InsecureRegistry...),
RegistryMirror: config.RegistryMirror,
ArbitraryFlags: config.DockerOpt,
InstallURL: drivers.DefaultEngineInstallURL,
}
return &o
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/minikube/cluster/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func TestStartHostConfig(t *testing.T) {

func TestStopHostError(t *testing.T) {
api := tests.NewMockAPI()
if err := StopHost(api); err == nil {
if err := StopHost(api, defaultMachineConfig); err == nil {
t.Fatal("An error should be thrown when stopping non-existing machine.")
}
}
Expand All @@ -221,7 +221,7 @@ func TestStopHost(t *testing.T) {
t.Errorf("createHost failed: %v", err)
}

if err := StopHost(api); err != nil {
if err := StopHost(api, defaultMachineConfig); err != nil {
t.Fatal("An error should be thrown when stopping non-existing machine.")
}
if s, _ := h.Driver.GetState(); s != state.Stopped {
Expand Down Expand Up @@ -289,7 +289,7 @@ func TestGetHostStatus(t *testing.T) {

checkState(state.Running.String())

if err := StopHost(api); err != nil {
if err := StopHost(api, defaultMachineConfig); err != nil {
t.Errorf("StopHost failed: %v", err)
}
checkState(state.Stopped.String())
Expand Down
5 changes: 5 additions & 0 deletions pkg/provision/buildroot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ func (p *BuildrootProvisioner) String() string {
return "buildroot"
}

// CompatibleWithHost checks if provisioner is compatible with host
func (p *BuildrootProvisioner) CompatibleWithHost() bool {
return p.OsReleaseInfo.ID == "buildroot"
}

// escapeSystemdDirectives escapes special characters in the input variables used to create the
// systemd unit file, which would otherwise be interpreted as systemd directives. An example
// are template specifiers (e.g. '%i') which are predefined variables that get evaluated dynamically
Expand Down