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

Delete: output underlying status failure, better machine function names #7043

Merged
merged 2 commits into from
Mar 14, 2020
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
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/config/open.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ var addonsOpenCmd = &cobra.Command{
if err != nil {
exit.WithError("Error getting control plane", err)
}
if !machine.IsHostRunning(api, driver.MachineName(*cc, cp)) {
if !machine.IsRunning(api, driver.MachineName(*cc, cp)) {
os.Exit(1)
}
addon, ok := assets.Addons[addonName] // validate addon input
Expand Down
4 changes: 2 additions & 2 deletions cmd/minikube/cmd/config/profile_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ var printProfilesTable = func() {
if err != nil {
exit.WithError("error getting primary control plane", err)
}
p.Status, err = machine.GetHostStatus(api, driver.MachineName(*p.Config, cp))
p.Status, err = machine.Status(api, driver.MachineName(*p.Config, cp))
if err != nil {
glog.Warningf("error getting host status for %s: %v", p.Name, err)
}
Expand Down Expand Up @@ -121,7 +121,7 @@ var printProfilesJSON = func() {
if err != nil {
exit.WithError("error getting primary control plane", err)
}
status, err := machine.GetHostStatus(api, driver.MachineName(*v.Config, cp))
status, err := machine.Status(api, driver.MachineName(*v.Config, cp))
if err != nil {
glog.Warningf("error getting host status for %s: %v", v.Name, err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ var dashboardCmd = &cobra.Command{
exit.WithCodeT(exit.NoInput, "kubectl not found in PATH, but is required for the dashboard. Installation guide: https://kubernetes.io/docs/tasks/tools/install-kubectl/")
}

if !machine.IsHostRunning(api, machineName) {
if !machine.IsRunning(api, machineName) {
os.Exit(1)
}

Expand Down
6 changes: 3 additions & 3 deletions cmd/minikube/cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func DeleteProfiles(profiles []*config.Profile) []error {
err := deleteProfile(profile)

if err != nil {
mm, loadErr := machine.Load(profile.Name)
mm, loadErr := machine.LoadMachine(profile.Name)

if !profile.IsValid() || (loadErr != nil || !mm.IsValid()) {
invalidProfileDeletionErrs := deleteInvalidProfile(profile)
Expand Down Expand Up @@ -208,7 +208,6 @@ func deleteProfileContainersAndVolumes(name string) {

func deleteProfile(profile *config.Profile) error {
viper.Set(config.ProfileName, profile.Name)

deleteProfileContainersAndVolumes(profile.Name)

api, err := machine.NewAPIClient()
Expand All @@ -217,6 +216,7 @@ func deleteProfile(profile *config.Profile) error {
return DeletionError{Err: delErr, Errtype: Fatal}
}
defer api.Close()

cc, err := config.Load(profile.Name)
if err != nil && !config.IsNotExist(err) {
delErr := profileDeletionErr(profile.Name, fmt.Sprintf("error loading profile config: %v", err))
Expand Down Expand Up @@ -328,7 +328,7 @@ func uninstallKubernetes(api libmachine.API, cc config.ClusterConfig, n config.N
return DeletionError{Err: fmt.Errorf("unable to get bootstrapper: %v", err), Errtype: Fatal}
}

host, err := machine.CheckIfHostExistsAndLoad(api, driver.MachineName(cc, n))
host, err := machine.LoadHost(api, driver.MachineName(cc, n))
if err != nil {
exit.WithError("Error getting host", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/minikube/cmd/docker-env.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,15 @@ var dockerEnvCmd = &cobra.Command{
}
for _, n := range cc.Nodes {
machineName := driver.MachineName(*cc, n)
host, err := machine.CheckIfHostExistsAndLoad(api, machineName)
host, err := machine.LoadHost(api, machineName)
if err != nil {
exit.WithError("Error getting host", err)
}
if host.Driver.DriverName() == driver.None {
exit.UsageT(`'none' driver does not support 'minikube docker-env' command`)
}

hostSt, err := machine.GetHostStatus(api, machineName)
hostSt, err := machine.Status(api, machineName)
if err != nil {
exit.WithError("Error getting host status", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/node_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var nodeStartCmd = &cobra.Command{
exit.WithError("creating api client", err)
}

if machine.IsHostRunning(api, name) {
if machine.IsRunning(api, name) {
out.T(out.Check, "{{.name}} is already running", out.V{"name": name})
os.Exit(0)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/pause.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func runPause(cmd *cobra.Command, args []string) {
glog.Infof("config: %+v", cc)

for _, n := range cc.Nodes {
host, err := machine.CheckIfHostExistsAndLoad(api, driver.MachineName(*cc, n))
host, err := machine.LoadHost(api, driver.MachineName(*cc, n))
if err != nil {
exit.WithError("Error getting host", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/minikube/cmd/podman-env.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,15 @@ var podmanEnvCmd = &cobra.Command{
}
for _, n := range cc.Nodes {
machineName := driver.MachineName(*cc, n)
host, err := machine.CheckIfHostExistsAndLoad(api, machineName)
host, err := machine.LoadHost(api, machineName)
if err != nil {
exit.WithError("Error getting host", err)
}
if host.Driver.DriverName() == driver.None {
exit.UsageT(`'none' driver does not support 'minikube podman-env' command`)
}

hostSt, err := machine.GetHostStatus(api, machineName)
hostSt, err := machine.Status(api, machineName)
if err != nil {
exit.WithError("Error getting host status", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ var serviceCmd = &cobra.Command{
exit.WithError("Error getting control plane", err)
}
machineName := driver.MachineName(*cfg, cp)
if !machine.IsHostRunning(api, machineName) {
if !machine.IsRunning(api, machineName) {
os.Exit(1)
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/service_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ var serviceListCmd = &cobra.Command{
if err != nil {
exit.WithError("Error getting primary control plane", err)
}
if !machine.IsHostRunning(api, driver.MachineName(*cfg, cp)) {
if !machine.IsRunning(api, driver.MachineName(*cfg, cp)) {
exit.WithCodeT(exit.Unavailable, "profile {{.name}} is not running.", out.V{"name": profileName})
}
serviceURLs, err := service.GetServiceURLs(api, serviceListNamespace, serviceURLTemplate)
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ var sshCmd = &cobra.Command{
if err != nil {
exit.WithError("Error getting primary control plane", err)
}
host, err := machine.CheckIfHostExistsAndLoad(api, driver.MachineName(*cc, cp))
host, err := machine.LoadHost(api, driver.MachineName(*cc, cp))
if err != nil {
exit.WithError("Error getting host", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/minikube/cmd/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func status(api libmachine.API, name string) (*Status, error) {
Kubeconfig: Nonexistent,
}

hs, err := machine.GetHostStatus(api, name)
hs, err := machine.Status(api, name)
glog.Infof("%s host status = %q (err=%v)", name, hs, err)
if err != nil {
return st, errors.Wrap(err, "host")
Expand Down Expand Up @@ -196,7 +196,7 @@ func status(api libmachine.API, name string) (*Status, error) {
st.Kubeconfig = Configured
}

host, err := machine.CheckIfHostExistsAndLoad(api, name)
host, err := machine.LoadHost(api, name)
if err != nil {
return st, err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/unpause.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ var unpauseCmd = &cobra.Command{

for _, n := range cc.Nodes {
machineName := driver.MachineName(*cc, n)
host, err := machine.CheckIfHostExistsAndLoad(api, machineName)
host, err := machine.LoadHost(api, machineName)
if err != nil {
exit.WithError("Error getting host", err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/addons/addons.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ func enableOrDisableAddon(cc *config.ClusterConfig, name string, val string) err
}

mName := driver.MachineName(*cc, cp)
host, err := machine.CheckIfHostExistsAndLoad(api, mName)
if err != nil || !machine.IsHostRunning(api, mName) {
host, err := machine.LoadHost(api, mName)
if err != nil || !machine.IsRunning(api, mName) {
glog.Warningf("%q is not running, writing %s=%v to disk and skipping enablement (err=%v)", mName, addon.Name(), enable, err)
return nil
}
Expand Down Expand Up @@ -247,7 +247,7 @@ func enableOrDisableStorageClasses(cc *config.ClusterConfig, name string, val st
if err != nil {
return errors.Wrap(err, "getting control plane")
}
if !machine.IsHostRunning(api, driver.MachineName(*cc, cp)) {
if !machine.IsRunning(api, driver.MachineName(*cc, cp)) {
glog.Warningf("%q is not running, writing %s=%v to disk and skipping enablement", driver.MachineName(*cc, cp), name, val)
return enableOrDisableAddon(cc, name, val)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/drivers/kic/kic.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func (d *Driver) GetState() (state.State, error) {
}
o := strings.TrimSpace(string(out))
if err != nil {
return state.Error, errors.Wrapf(err, "get container %s status", d.MachineName)
return state.Error, errors.Wrapf(err, "%s: %s", strings.Join(cmd.Args, " "), o)
}
switch o {
case "running":
Expand Down
2 changes: 1 addition & 1 deletion pkg/minikube/cluster/ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func GetVMHostIP(host *host.Host) (net.IP, error) {

// GetHostDriverIP gets the ip address of the current minikube cluster
func GetHostDriverIP(api libmachine.API, machineName string) (net.IP, error) {
host, err := machine.CheckIfHostExistsAndLoad(api, machineName)
host, err := machine.LoadHost(api, machineName)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/minikube/machine/cache_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func CacheAndLoadImages(images []string) error {
}
for _, n := range c.Nodes {
m := driver.MachineName(*c, n)
status, err := GetHostStatus(api, m)
status, err := Status(api, m)
if err != nil {
glog.Warningf("skipping loading cache for profile %s", pName)
glog.Errorf("error getting status for %s: %v", pName, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/minikube/machine/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func TestDeleteHostErrMachineNotExist(t *testing.T) {
}
}

func TestGetHostStatus(t *testing.T) {
func TestStatus(t *testing.T) {
RegisterMockDriver(t)
api := tests.NewMockAPI(t)

Expand All @@ -384,7 +384,7 @@ func TestGetHostStatus(t *testing.T) {
m := driver.MachineName(cc, config.Node{Name: "minikube"})

checkState := func(expected string, machineName string) {
s, err := GetHostStatus(api, machineName)
s, err := Status(api, machineName)
if err != nil {
t.Fatalf("Unexpected error getting status: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/minikube/machine/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ func DeleteHost(api libmachine.API, machineName string) error {
}

// Get the status of the host. Ensure that it exists before proceeding ahead.
status, err := GetHostStatus(api, machineName)
status, err := Status(api, machineName)
if err != nil {
// Warn, but proceed
out.WarningT("Unable to get the status of the {{.name}} cluster.", out.V{"name": machineName})
out.WarningT(`Unable to get host status for "{{.name}}": {{.error}}`, out.V{"name": machineName, "error": err})
}

if status == state.None.String() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"github.com/pkg/errors"
)

// GetHostStatus gets the status of the host VM.
func GetHostStatus(api libmachine.API, machineName string) (string, error) {
// Status returns the status of a libmachine host
func Status(api libmachine.API, machineName string) (string, error) {
exists, err := api.Exists(machineName)
if err != nil {
return "", errors.Wrapf(err, "%s exists", machineName)
Expand All @@ -46,9 +46,9 @@ func GetHostStatus(api libmachine.API, machineName string) (string, error) {
return s.String(), nil
}

// IsHostRunning asserts that this profile's primary host is in state "Running"
func IsHostRunning(api libmachine.API, name string) bool {
s, err := GetHostStatus(api, name)
// IsRunning asserts that a libmachine host is in state "Running"
func IsRunning(api libmachine.API, name string) bool {
s, err := Status(api, name)
if err != nil {
glog.Warningf("host status for %q returned error: %v", name, err)
return false
Expand All @@ -60,8 +60,8 @@ func IsHostRunning(api libmachine.API, name string) bool {
return true
}

// CheckIfHostExistsAndLoad checks if a host exists, and loads it if it does
func CheckIfHostExistsAndLoad(api libmachine.API, machineName string) (*host.Host, error) {
// LoadHost returns a libmachine host by name
func LoadHost(api libmachine.API, machineName string) (*host.Host, error) {
glog.Infof("Checking if %q exists ...", machineName)
exists, err := api.Exists(machineName)
if err != nil {
Expand All @@ -71,9 +71,9 @@ func CheckIfHostExistsAndLoad(api libmachine.API, machineName string) (*host.Hos
return nil, errors.Errorf("machine %q does not exist", machineName)
}

host, err := api.Load(machineName)
h, err := api.Load(machineName)
if err != nil {
return nil, errors.Wrapf(err, "loading machine %q", machineName)
}
return host, nil
return h, nil
}
59 changes: 3 additions & 56 deletions pkg/minikube/machine/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,8 @@ import (
"io/ioutil"
"path/filepath"

"github.com/docker/machine/libmachine"
"github.com/docker/machine/libmachine/host"
"github.com/docker/machine/libmachine/state"
"github.com/golang/glog"
"github.com/pkg/errors"
"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/driver"
"k8s.io/minikube/pkg/minikube/localpath"
)

Expand Down Expand Up @@ -63,37 +58,14 @@ func (h *Machine) IsValid() bool {
return true
}

// List return all valid and invalid machines
// If a machine is valid or invalid is determined by the cluster.IsValid function
func List(miniHome ...string) (validMachines []*Machine, inValidMachines []*Machine, err error) {
pDirs, err := machineDirs(miniHome...)
if err != nil {
return nil, nil, err
}
for _, n := range pDirs {
p, err := Load(n)
if err != nil {
glog.Infof("%s not valid: %v", n, err)
inValidMachines = append(inValidMachines, p)
continue
}
if !p.IsValid() {
inValidMachines = append(inValidMachines, p)
continue
}
validMachines = append(validMachines, p)
}
return validMachines, inValidMachines, nil
}

// Load loads a machine or throws an error if the machine could not be loaded.
func Load(name string) (*Machine, error) {
// LoadMachine returns a Machine abstracting a libmachine.Host
func LoadMachine(name string) (*Machine, error) {
api, err := NewAPIClient()
if err != nil {
return nil, err
}

h, err := CheckIfHostExistsAndLoad(api, name)
h, err := LoadHost(api, name)
if err != nil {
return nil, err
}
Expand All @@ -104,7 +76,6 @@ func Load(name string) (*Machine, error) {
} else {
return nil, errors.New("host is nil")
}

return &mm, nil
}

Expand All @@ -122,27 +93,3 @@ func machineDirs(miniHome ...string) (dirs []string, err error) {
}
return dirs, err
}

// CreateSSHShell creates a new SSH shell / client
func CreateSSHShell(api libmachine.API, cc config.ClusterConfig, n config.Node, args []string) error {
machineName := driver.MachineName(cc, n)
host, err := CheckIfHostExistsAndLoad(api, machineName)
if err != nil {
return errors.Wrap(err, "host exists and load")
}

currentState, err := host.Driver.GetState()
if err != nil {
return errors.Wrap(err, "state")
}

if currentState != state.Running {
return errors.Errorf("%q is not running", machineName)
}

client, err := host.CreateSSHClient()
if err != nil {
return errors.Wrap(err, "Creating ssh client")
}
return client.Shell(args...)
}
Loading