Merge pull request kubernetes#17936 from spowelljr/reimplementAutoPause

Addons auto-pause: Remove memory leak & add configurable interval

Makefile

@@ -24,7 +24,7 @@ KIC_VERSION ?= $(shell grep -E "Version =" pkg/drivers/kic/types.go | cut -d \"
 HUGO_VERSION ?= $(shell grep -E "HUGO_VERSION = \"" netlify.toml | cut -d \" -f2)
 
 # Default to .0 for higher cache hit rates, as build increments typically don't require new ISO versions
-ISO_VERSION ?= v1.32.1-1703784139-17866
+ISO_VERSION ?= v1.32.1-1708020063-17936
 
 # Dashes are valid in semver, but not Linux packaging. Use ~ to delimit alpha/beta
 DEB_VERSION ?= $(subst -,~,$(RAW_VERSION))

cmd/auto-pause/auto-pause.go

@@ -28,42 +28,40 @@ import (
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/cruntime"
 	"k8s.io/minikube/pkg/minikube/exit"
-	"k8s.io/minikube/pkg/minikube/out"
 	"k8s.io/minikube/pkg/minikube/reason"
-	"k8s.io/minikube/pkg/minikube/style"
 )
 
-var unpauseRequests = make(chan struct{})
-var done = make(chan struct{})
-var mu sync.Mutex
+var (
+	unpauseRequests = make(chan struct{})
+	done            = make(chan struct{})
+	mu              sync.Mutex
+	runtimePaused   bool
+	version         = "0.0.1"
 
-var runtimePaused bool
-var version = "0.0.1"
-
-var runtime = flag.String("container-runtime", "docker", "Container runtime to use for (un)pausing")
+	runtime  = flag.String("container-runtime", "docker", "Container runtime to use for (un)pausing")
+	interval = flag.Duration("interval", time.Minute*1, "Interval of inactivity for pause to occur")
+)
 
 func main() {
 	flag.Parse()
 
-	// TODO: #10595 make this configurable
-	const interval = time.Minute * 1
+	tickerChannel := time.NewTicker(*interval)
 
 	// Check current state
 	alreadyPaused()
 
 	// channel for incoming messages
 	go func() {
 		for {
-			// On each iteration new timer is created
 			select {
-			// TODO: #10596 make it memory-leak proof
-			case <-time.After(interval):
+			case <-tickerChannel.C:
+				tickerChannel.Stop()
 				runPause()
 			case <-unpauseRequests:
-				fmt.Printf("Got request\n")
-				if runtimePaused {
-					runUnpause()
-				}
+				tickerChannel.Stop()
+				log.Println("Got request")
+				runUnpause()
+				tickerChannel.Reset(*interval)
 
 				done <- struct{}{}
 			}
@@ -86,9 +84,10 @@ func runPause() {
 	mu.Lock()
 	defer mu.Unlock()
 	if runtimePaused {
-		out.Styled(style.AddonEnable, "Auto-pause is already enabled.")
+		log.Println("Already paused, skipping")
 		return
 	}
+	log.Println("Pausing...")
 
 	r := command.NewExecRunner(true)
 
@@ -104,13 +103,17 @@ func runPause() {
 
 	runtimePaused = true
 
-	out.Step(style.Unpause, "Paused {{.count}} containers", out.V{"count": len(uids)})
+	log.Printf("Paused %d containers", len(uids))
 }
 
 func runUnpause() {
-	fmt.Println("unpausing...")
 	mu.Lock()
 	defer mu.Unlock()
+	if !runtimePaused {
+		log.Println("Already unpaused, skipping")
+		return
+	}
+	log.Println("Unpausing...")
 
 	r := command.NewExecRunner(true)
 
@@ -125,7 +128,7 @@ func runUnpause() {
 	}
 	runtimePaused = false
 
-	out.Step(style.Unpause, "Unpaused {{.count}} containers", out.V{"count": len(uids)})
+	log.Printf("Unpaused %d containers", len(uids))
 }
 
 func alreadyPaused() {
@@ -142,5 +145,5 @@ func alreadyPaused() {
 	if err != nil {
 		exit.Error(reason.GuestCheckPaused, "Fail check if container paused", err)
 	}
-	out.Step(style.Check, "containers paused status: {{.paused}}", out.V{"paused": runtimePaused})
+	log.Printf("containers paused status: %t", runtimePaused)
 }

cmd/minikube/cmd/config/configure.go

@@ -20,6 +20,7 @@ import (
 	"net"
 	"os"
 	"regexp"
+	"time"
 
 	"github.com/spf13/cobra"
 	"k8s.io/minikube/pkg/addons"
@@ -45,6 +46,8 @@ var addonsConfigureCmd = &cobra.Command{
 			exit.Message(reason.Usage, "usage: minikube addons configure ADDON_NAME")
 		}
 
+		profile := ClusterFlagValue()
+
 		addon := args[0]
 		// allows for additional prompting of information when enabling addons
 		switch addon {
@@ -109,12 +112,11 @@ var addonsConfigureCmd = &cobra.Command{
 				acrPassword = AskForPasswordValue("-- Enter service principal password to access Azure Container Registry: ")
 			}
 
-			cname := ClusterFlagValue()
 			namespace := "kube-system"
 
 			// Create ECR Secret
 			err := service.CreateSecret(
-				cname,
+				profile,
 				namespace,
 				"registry-creds-ecr",
 				map[string]string{
@@ -136,7 +138,7 @@ var addonsConfigureCmd = &cobra.Command{
 
 			// Create GCR Secret
 			err = service.CreateSecret(
-				cname,
+				profile,
 				namespace,
 				"registry-creds-gcr",
 				map[string]string{
@@ -155,7 +157,7 @@ var addonsConfigureCmd = &cobra.Command{
 
 			// Create Docker Secret
 			err = service.CreateSecret(
-				cname,
+				profile,
 				namespace,
 				"registry-creds-dpr",
 				map[string]string{
@@ -175,7 +177,7 @@ var addonsConfigureCmd = &cobra.Command{
 
 			// Create Azure Container Registry Secret
 			err = service.CreateSecret(
-				cname,
+				profile,
 				namespace,
 				"registry-creds-acr",
 				map[string]string{
@@ -194,7 +196,6 @@ var addonsConfigureCmd = &cobra.Command{
 			}
 
 		case "metallb":
-			profile := ClusterFlagValue()
 			_, cfg := mustload.Partial(profile)
 
 			validator := func(s string) bool {
@@ -214,7 +215,6 @@ var addonsConfigureCmd = &cobra.Command{
 				out.ErrT(style.Fatal, "Failed to configure metallb IP {{.profile}}", out.V{"profile": profile})
 			}
 		case "ingress":
-			profile := ClusterFlagValue()
 			_, cfg := mustload.Partial(profile)
 
 			validator := func(s string) bool {
@@ -236,7 +236,6 @@ var addonsConfigureCmd = &cobra.Command{
 				out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile})
 			}
 		case "registry-aliases":
-			profile := ClusterFlagValue()
 			_, cfg := mustload.Partial(profile)
 			validator := func(s string) bool {
 				format := regexp.MustCompile(`^([a-zA-Z0-9-_]+\.[a-zA-Z0-9-_]+)+(\ [a-zA-Z0-9-_]+\.[a-zA-Z0-9-_]+)*$`)
@@ -255,7 +254,27 @@ var addonsConfigureCmd = &cobra.Command{
 					out.ErrT(style.Fatal, "Failed to configure registry-aliases {{.profile}}", out.V{"profile": profile})
 				}
 			}
-
+		case "auto-pause":
+			_, cfg := mustload.Partial(profile)
+			intervalInput := AskForStaticValue("-- Enter interval time of auto-pause-interval (ex. 1m0s): ")
+			intervalTime, err := time.ParseDuration(intervalInput)
+			if err != nil {
+				out.ErrT(style.Fatal, "Interval is an invalid duration: {{.error}}", out.V{"error": err})
+			}
+			if intervalTime != intervalTime.Abs() || intervalTime.String() == "0s" {
+				out.ErrT(style.Fatal, "Interval must be greater than 0s")
+			}
+			cfg.AutoPauseInterval = intervalTime
+			if err := config.SaveProfile(profile, cfg); err != nil {
+				out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile})
+			}
+			addon := assets.Addons["auto-pause"]
+			if addon.IsEnabled(cfg) {
+				// Re-enable auto-pause addon in order to update interval time
+				if err := addons.EnableOrDisableAddon(cfg, "auto-pause", "true"); err != nil {
+					out.ErrT(style.Fatal, "Failed to configure auto-pause {{.profile}}", out.V{"profile": profile})
+				}
+			}
 		default:
 			out.FailureT("{{.name}} has no available configuration options", out.V{"name": addon})
 			return

cmd/minikube/cmd/start.go

@@ -31,6 +31,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/Delta456/box-cli-maker/v2"
 	"github.com/blang/semver/v4"
@@ -1249,7 +1250,7 @@ func validateCPUCount(drvName string) {
 }
 
 // validateFlags validates the supplied flags against known bad combinations
-func validateFlags(cmd *cobra.Command, drvName string) {
+func validateFlags(cmd *cobra.Command, drvName string) { //nolint:gocyclo
 	if cmd.Flags().Changed(humanReadableDiskSize) {
 		err := validateDiskSize(viper.GetString(humanReadableDiskSize))
 		if err != nil {
@@ -1315,6 +1316,12 @@ func validateFlags(cmd *cobra.Command, drvName string) {
 		}
 	}
 
+	if cmd.Flags().Changed(autoPauseInterval) {
+		if err := validateAutoPauseInterval(viper.GetDuration(autoPauseInterval)); err != nil {
+			exit.Message(reason.Usage, "{{.err}}", out.V{"err": err})
+		}
+	}
+
 	if driver.IsSSH(drvName) {
 		sshIPAddress := viper.GetString(sshIPAddress)
 		if sshIPAddress == "" {
@@ -1476,6 +1483,13 @@ func validateGPUsArch() error {
 	return errors.Errorf("The GPUs flag is only supported on amd64, arm64 & ppc64le, currently using %s", runtime.GOARCH)
 }
 
+func validateAutoPauseInterval(interval time.Duration) error {
+	if interval != interval.Abs() || interval.String() == "0s" {
+		return errors.New("auto-pause-interval must be greater than 0s")
+	}
+	return nil
+}
+
 func getContainerRuntime(old *config.ClusterConfig) string {
 	paramRuntime := viper.GetString(containerRuntime)
 

cmd/minikube/cmd/start_flags.go

@@ -142,6 +142,7 @@ const (
 	socketVMnetPath         = "socket-vmnet-path"
 	staticIP                = "static-ip"
 	gpus                    = "gpus"
+	autoPauseInterval       = "auto-pause-interval"
 )
 
 var (
@@ -204,6 +205,7 @@ func initMinikubeFlags() {
 	startCmd.Flags().Bool(disableMetrics, false, "If set, disables metrics reporting (CPU and memory usage), this can improve CPU usage. Defaults to false.")
 	startCmd.Flags().String(staticIP, "", "Set a static IP for the minikube cluster, the IP must be: private, IPv4, and the last octet must be between 2 and 254, for example 192.168.200.200 (Docker and Podman drivers only)")
 	startCmd.Flags().StringP(gpus, "g", "", "Allow pods to use your NVIDIA GPUs. Options include: [all,nvidia] (Docker driver with Docker container-runtime only)")
+	startCmd.Flags().Duration(autoPauseInterval, time.Minute*1, "Duration of inactivity before the minikube VM is paused (default 1m0s)")
 }
 
 // initKubernetesFlags inits the commandline flags for Kubernetes related options
@@ -603,6 +605,7 @@ func generateNewConfigFromFlags(cmd *cobra.Command, k8sVersion string, rtime str
 		},
 		MultiNodeRequested: viper.GetInt(nodes) > 1,
 		GPUs:               viper.GetString(gpus),
+		AutoPauseInterval:  viper.GetDuration(autoPauseInterval),
 	}
 	cc.VerifyComponents = interpretWaitFlag(*cmd)
 	if viper.GetBool(createMount) && driver.IsKIC(drvName) {
@@ -817,6 +820,7 @@ func updateExistingConfigFromFlags(cmd *cobra.Command, existing *config.ClusterC
 	updateStringFromFlag(cmd, &cc.CustomQemuFirmwarePath, qemuFirmwarePath)
 	updateStringFromFlag(cmd, &cc.SocketVMnetClientPath, socketVMnetClientPath)
 	updateStringFromFlag(cmd, &cc.SocketVMnetPath, socketVMnetPath)
+	updateDurationFromFlag(cmd, &cc.AutoPauseInterval, autoPauseInterval)
 
 	if cmd.Flags().Changed(kubernetesVersion) {
 		kubeVer, err := getKubernetesVersion(existing)

cmd/minikube/cmd/start_test.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/blang/semver/v4"
 	"github.com/spf13/cobra"
@@ -886,3 +887,29 @@ func TestValidateGPUs(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateAutoPause(t *testing.T) {
+	tests := []struct {
+		interval    string
+		shouldError bool
+	}{
+		{"1m0s", false},
+		{"5m", false},
+		{"1s", false},
+		{"0s", true},
+		{"-2m", true},
+	}
+	for _, tc := range tests {
+		input, err := time.ParseDuration(tc.interval)
+		if err != nil {
+			t.Fatalf("test has an invalid input duration of %q", tc.interval)
+		}
+		err = validateAutoPauseInterval(input)
+		if err != nil && !tc.shouldError {
+			t.Errorf("interval of %q failed validation; expected it to pass: %v", input, err)
+		}
+		if err == nil && tc.shouldError {
+			t.Errorf("interval of %q passed validataion; expected it to fail: %v", input, err)
+		}
+	}
+}

deploy/addons/auto-pause/auto-pause.service.tmpl

@@ -3,7 +3,7 @@ Description=Auto Pause Service
 
 [Service]
 Type=simple
-ExecStart=/bin/auto-pause --container-runtime={{.ContainerRuntime}}
+ExecStart=/bin/auto-pause --container-runtime={{.ContainerRuntime}} --interval={{.AutoPauseInterval}}
 Restart=always
 
 [Install]

pkg/drivers/kic/types.go

@@ -24,10 +24,10 @@ import (
 
 const (
 	// Version is the current version of kic
-	Version = "v0.0.42-1704759386-17866"
+	Version = "v0.0.42-1708008208-17936"
 
 	// SHA of the kic base image
-	baseImageSHA = "8c3c33047f9bc285e1f5f2a5aa14744a2fe04c58478f02f77b06169dea8dd3f0"
+	baseImageSHA = "4ea1136332ba1476cda33a97bf12e2f96995cc120674fbafd3ade22d1118ecdf"
 	// The name of the GCR kicbase repository
 	gcrRepo = "gcr.io/k8s-minikube/kicbase-builds"
 	// The name of the Dockerhub kicbase repository

pkg/minikube/assets/addons.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"runtime"
 	"strings"
+	"time"
 
 	semver "github.com/blang/semver/v4"
 	"github.com/pkg/errors"
@@ -937,6 +938,7 @@ func GenerateTemplateData(addon *Addon, cc *config.ClusterConfig, netInfo Networ
 		Environment             map[string]string
 		LegacyPodSecurityPolicy bool
 		LegacyRuntimeClass      bool
+		AutoPauseInterval       time.Duration
 	}{
 		KubernetesVersion:      make(map[string]uint64),
 		PreOneTwentyKubernetes: false,
@@ -958,6 +960,7 @@ func GenerateTemplateData(addon *Addon, cc *config.ClusterConfig, netInfo Networ
 		},
 		LegacyPodSecurityPolicy: v.LT(semver.Version{Major: 1, Minor: 25}),
 		LegacyRuntimeClass:      v.LT(semver.Version{Major: 1, Minor: 25}),
+		AutoPauseInterval:       cc.AutoPauseInterval,
 	}
 	if opts.ImageRepository != "" && !strings.HasSuffix(opts.ImageRepository, "/") {
 		opts.ImageRepository += "/"

pkg/minikube/config/types.go

@@ -108,6 +108,7 @@ type ClusterConfig struct {
 	SSHAuthSock             string
 	SSHAgentPID             int
 	GPUs                    string
+	AutoPauseInterval       time.Duration // Specifies interval of time to wait before checking if cluster should be paused
 }
 
 // KubernetesConfig contains the parameters used to configure the VM Kubernetes.