Skip to content

Commit

Permalink
Delete: output underlying status failure, refactor wordy code
Browse files Browse the repository at this point in the history
  • Loading branch information
tstromberg committed Mar 13, 2020
1 parent 5986083 commit 196a8d7
Show file tree
Hide file tree
Showing 26 changed files with 136 additions and 155 deletions.
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", cmd.Args, out)
}
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
18 changes: 9 additions & 9 deletions pkg/minikube/machine/status.go → pkg/minikube/machine/host.go
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

0 comments on commit 196a8d7

Please sign in to comment.