Skip to content

Commit

Permalink
Address code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Priya Wadhwa committed Nov 10, 2020
1 parent 291e5b6 commit 5569817
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 29 deletions.
5 changes: 1 addition & 4 deletions cmd/minikube/cmd/stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cmd

import (
"log"
"os"
"runtime"
"time"
Expand Down Expand Up @@ -97,9 +96,7 @@ func runStop(cmd *cobra.Command, args []string) {
}

// Kill any existing scheduled stops
if err := schedule.KillExisting(profilesToStop); err != nil {
log.Printf("error killing existing scheduled stops: %v", err)
}
schedule.KillExisting(profilesToStop)

if scheduledStopDuration != 0 {
if runtime.GOOS == "windows" {
Expand Down
55 changes: 33 additions & 22 deletions pkg/minikube/schedule/daemonize_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,30 +31,41 @@ import (
"k8s.io/minikube/pkg/minikube/localpath"
)

// KillExisting kills existing scheduled stops
func KillExisting(profiles []string) error {
// KillExisting kills existing scheduled stops by looking up the PID
// of the scheduled stop from the PID file saved for the profile and killing the process
func KillExisting(profiles []string) {
for _, profile := range profiles {
file := localpath.PID(profile)
f, err := ioutil.ReadFile(file)
if os.IsNotExist(err) {
return nil
if err := killPIDForProfile(profile); err != nil {
klog.Errorf("error killng PID for profile %s: %v", profile, err)
}
defer os.Remove(file)
if err != nil {
return errors.Wrapf(err, "reading %s", file)
}
pid, err := strconv.Atoi(string(f))
if err != nil {
return errors.Wrapf(err, "converting %v to int", string(f))
}
p, err := os.FindProcess(pid)
if err != nil {
return errors.Wrap(err, "finding process")
}
klog.Infof("killing process %v as it is an old scheduled stop", pid)
if err := p.Kill(); err != nil {
return errors.Wrapf(err, "killing %v", pid)
}
}

func killPIDForProfile(profile string) error {
file := localpath.PID(profile)
f, err := ioutil.ReadFile(file)
if os.IsNotExist(err) {
return nil
}
defer func() {
if err := os.Remove(file); err != nil {
klog.Errorf("error deleting %s: %v, you may have to delete in manually", file, err)
}
}()
if err != nil {
return errors.Wrapf(err, "reading %s", file)
}
pid, err := strconv.Atoi(string(f))
if err != nil {
return errors.Wrapf(err, "converting %s to int", f)
}
p, err := os.FindProcess(pid)
if err != nil {
return errors.Wrap(err, "finding process")
}
klog.Infof("killing process %v as it is an old scheduled stop", pid)
if err := p.Kill(); err != nil {
return errors.Wrapf(err, "killing %v", pid)
}
return nil
}
Expand All @@ -72,7 +83,7 @@ func daemonize(profiles []string, duration time.Duration) error {
func savePIDs(pid int, profiles []string) error {
for _, p := range profiles {
file := localpath.PID(p)
if err := ioutil.WriteFile(file, []byte(fmt.Sprintf("%v", pid)), 0644); err != nil {
if err := ioutil.WriteFile(file, []byte(fmt.Sprintf("%v", pid)), 0600); err != nil {
return err
}
}
Expand Down
8 changes: 5 additions & 3 deletions pkg/minikube/schedule/daemonize_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ package schedule
import (
"fmt"
"time"

"k8s.io/klog/v2"
)

// KillExisting kills existing scheduled stops
func KillExisting(profiles []string) error {
return fmt.Errorf("not yet implemented for windows")
// KillExisting will kill existing scheduled stops
func KillExisting(profiles []string) {
klog.Errorf("not yet implemented for windows")
}

func daemonize(profiles []string, duration time.Duration) error {
Expand Down

0 comments on commit 5569817

Please sign in to comment.