Skip to content

Commit f3d93fe

Browse files
medyaghmedyagh
authored
Merge pull request #17936 from spowelljr/reimplementAutoPause
Addons auto-pause: Remove memory leak & add configurable interval
2 parents 04a79bd + 37a485e commit f3d93fe

File tree

21 files changed

+147
-49
lines changed

21 files changed

+147
-49
lines changed

Diff for: ‎Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ KIC_VERSION ?= $(shell grep -E "Version =" pkg/drivers/kic/types.go | cut -d \"
2424
HUGO_VERSION ?= $(shell grep -E "HUGO_VERSION = \"" netlify.toml | cut -d \" -f2)
2525

2626
# Default to .0 for higher cache hit rates, as build increments typically don't require new ISO versions
27-
ISO_VERSION ?= v1.32.1-1703784139-17866
27+
ISO_VERSION ?= v1.32.1-1708020063-17936
2828

2929
# Dashes are valid in semver, but not Linux packaging. Use ~ to delimit alpha/beta
3030
DEB_VERSION ?= $(subst -,~,$(RAW_VERSION))

Diff for: ‎cmd/auto-pause/auto-pause.go

+26-23
Original file line numberDiff line numberDiff line change
@@ -28,42 +28,40 @@ import (
2828
"k8s.io/minikube/pkg/minikube/command"
2929
"k8s.io/minikube/pkg/minikube/cruntime"
3030
"k8s.io/minikube/pkg/minikube/exit"
31-
"k8s.io/minikube/pkg/minikube/out"
3231
"k8s.io/minikube/pkg/minikube/reason"
33-
"k8s.io/minikube/pkg/minikube/style"
3432
)
3533

36-
var unpauseRequests = make(chan struct{})
37-
var done = make(chan struct{})
38-
var mu sync.Mutex
34+
var (
35+
unpauseRequests = make(chan struct{})
36+
done = make(chan struct{})
37+
mu sync.Mutex
38+
runtimePaused bool
39+
version = "0.0.1"
3940

40-
var runtimePaused bool
41-
var version = "0.0.1"
42-
43-
var runtime = flag.String("container-runtime", "docker", "Container runtime to use for (un)pausing")
41+
runtime = flag.String("container-runtime", "docker", "Container runtime to use for (un)pausing")
42+
interval = flag.Duration("interval", time.Minute*1, "Interval of inactivity for pause to occur")
43+
)
4444

4545
func main() {
4646
flag.Parse()
4747

48-
// TODO: #10595 make this configurable
49-
const interval = time.Minute * 1
48+
tickerChannel := time.NewTicker(*interval)
5049

5150
// Check current state
5251
alreadyPaused()
5352

5453
// channel for incoming messages
5554
go func() {
5655
for {
57-
// On each iteration new timer is created
5856
select {
59-
// TODO: #10596 make it memory-leak proof
60-
case <-time.After(interval):
57+
case <-tickerChannel.C:
58+
tickerChannel.Stop()
6159
runPause()
6260
case <-unpauseRequests:
63-
fmt.Printf("Got request\n")
64-
if runtimePaused {
65-
runUnpause()
66-
}
61+
tickerChannel.Stop()
62+
log.Println("Got request")
63+
runUnpause()
64+
tickerChannel.Reset(*interval)
6765

6866
done <- struct{}{}
6967
}
@@ -86,9 +84,10 @@ func runPause() {
8684
mu.Lock()
8785
defer mu.Unlock()
8886
if runtimePaused {
89-
out.Styled(style.AddonEnable, "Auto-pause is already enabled.")
87+
log.Println("Already paused, skipping")
9088
return
9189
}
90+
log.Println("Pausing...")
9291

9392
r := command.NewExecRunner(true)
9493

@@ -104,13 +103,17 @@ func runPause() {
104103

105104
runtimePaused = true
106105

107-
out.Step(style.Unpause, "Paused {{.count}} containers", out.V{"count": len(uids)})
106+
log.Printf("Paused %d containers", len(uids))
108107
}
109108

110109
func runUnpause() {
111-
fmt.Println("unpausing...")
112110
mu.Lock()
113111
defer mu.Unlock()
112+
if !runtimePaused {
113+
log.Println("Already unpaused, skipping")
114+
return
115+
}
116+
log.Println("Unpausing...")
114117

115118
r := command.NewExecRunner(true)
116119

@@ -125,7 +128,7 @@ func runUnpause() {
125128
}
126129
runtimePaused = false
127130

128-
out.Step(style.Unpause, "Unpaused {{.count}} containers", out.V{"count": len(uids)})
131+
log.Printf("Unpaused %d containers", len(uids))
129132
}
130133

131134
func alreadyPaused() {
@@ -142,5 +145,5 @@ func alreadyPaused() {
142145
if err != nil {
143146
exit.Error(reason.GuestCheckPaused, "Fail check if container paused", err)
144147
}
145-
out.Step(style.Check, "containers paused status: {{.paused}}", out.V{"paused": runtimePaused})
148+
log.Printf("containers paused status: %t", runtimePaused)
146149
}

Diff for: ‎cmd/minikube/cmd/config/configure.go

+28-9
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"net"
2121
"os"
2222
"regexp"
23+
"time"
2324

2425
"github.com/spf13/cobra"
2526
"k8s.io/minikube/pkg/addons"
@@ -45,6 +46,8 @@ var addonsConfigureCmd = &cobra.Command{
4546
exit.Message(reason.Usage, "usage: minikube addons configure ADDON_NAME")
4647
}
4748

49+
profile := ClusterFlagValue()
50+
4851
addon := args[0]
4952
// allows for additional prompting of information when enabling addons
5053
switch addon {
@@ -109,12 +112,11 @@ var addonsConfigureCmd = &cobra.Command{
109112
acrPassword = AskForPasswordValue("-- Enter service principal password to access Azure Container Registry: ")
110113
}
111114

112-
cname := ClusterFlagValue()
113115
namespace := "kube-system"
114116

115117
// Create ECR Secret
116118
err := service.CreateSecret(
117-
cname,
119+
profile,
118120
namespace,
119121
"registry-creds-ecr",
120122
map[string]string{
@@ -136,7 +138,7 @@ var addonsConfigureCmd = &cobra.Command{
136138

137139
// Create GCR Secret
138140
err = service.CreateSecret(
139-
cname,
141+
profile,
140142
namespace,
141143
"registry-creds-gcr",
142144
map[string]string{
@@ -155,7 +157,7 @@ var addonsConfigureCmd = &cobra.Command{
155157

156158
// Create Docker Secret
157159
err = service.CreateSecret(
158-
cname,
160+
profile,
159161
namespace,
160162
"registry-creds-dpr",
161163
map[string]string{
@@ -175,7 +177,7 @@ var addonsConfigureCmd = &cobra.Command{
175177

176178
// Create Azure Container Registry Secret
177179
err = service.CreateSecret(
178-
cname,
180+
profile,
179181
namespace,
180182
"registry-creds-acr",
181183
map[string]string{
@@ -194,7 +196,6 @@ var addonsConfigureCmd = &cobra.Command{
194196
}
195197

196198
case "metallb":
197-
profile := ClusterFlagValue()
198199
_, cfg := mustload.Partial(profile)
199200

200201
validator := func(s string) bool {
@@ -214,7 +215,6 @@ var addonsConfigureCmd = &cobra.Command{
214215
out.ErrT(style.Fatal, "Failed to configure metallb IP {{.profile}}", out.V{"profile": profile})
215216
}
216217
case "ingress":
217-
profile := ClusterFlagValue()
218218
_, cfg := mustload.Partial(profile)
219219

220220
validator := func(s string) bool {
@@ -236,7 +236,6 @@ var addonsConfigureCmd = &cobra.Command{
236236
out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile})
237237
}
238238
case "registry-aliases":
239-
profile := ClusterFlagValue()
240239
_, cfg := mustload.Partial(profile)
241240
validator := func(s string) bool {
242241
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{
255254
out.ErrT(style.Fatal, "Failed to configure registry-aliases {{.profile}}", out.V{"profile": profile})
256255
}
257256
}
258-
257+
case "auto-pause":
258+
_, cfg := mustload.Partial(profile)
259+
intervalInput := AskForStaticValue("-- Enter interval time of auto-pause-interval (ex. 1m0s): ")
260+
intervalTime, err := time.ParseDuration(intervalInput)
261+
if err != nil {
262+
out.ErrT(style.Fatal, "Interval is an invalid duration: {{.error}}", out.V{"error": err})
263+
}
264+
if intervalTime != intervalTime.Abs() || intervalTime.String() == "0s" {
265+
out.ErrT(style.Fatal, "Interval must be greater than 0s")
266+
}
267+
cfg.AutoPauseInterval = intervalTime
268+
if err := config.SaveProfile(profile, cfg); err != nil {
269+
out.ErrT(style.Fatal, "Failed to save config {{.profile}}", out.V{"profile": profile})
270+
}
271+
addon := assets.Addons["auto-pause"]
272+
if addon.IsEnabled(cfg) {
273+
// Re-enable auto-pause addon in order to update interval time
274+
if err := addons.EnableOrDisableAddon(cfg, "auto-pause", "true"); err != nil {
275+
out.ErrT(style.Fatal, "Failed to configure auto-pause {{.profile}}", out.V{"profile": profile})
276+
}
277+
}
259278
default:
260279
out.FailureT("{{.name}} has no available configuration options", out.V{"name": addon})
261280
return

Diff for: ‎cmd/minikube/cmd/start.go

+15-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"sort"
3232
"strconv"
3333
"strings"
34+
"time"
3435

3536
"github.com/Delta456/box-cli-maker/v2"
3637
"github.com/blang/semver/v4"
@@ -1249,7 +1250,7 @@ func validateCPUCount(drvName string) {
12491250
}
12501251

12511252
// validateFlags validates the supplied flags against known bad combinations
1252-
func validateFlags(cmd *cobra.Command, drvName string) {
1253+
func validateFlags(cmd *cobra.Command, drvName string) { //nolint:gocyclo
12531254
if cmd.Flags().Changed(humanReadableDiskSize) {
12541255
err := validateDiskSize(viper.GetString(humanReadableDiskSize))
12551256
if err != nil {
@@ -1315,6 +1316,12 @@ func validateFlags(cmd *cobra.Command, drvName string) {
13151316
}
13161317
}
13171318

1319+
if cmd.Flags().Changed(autoPauseInterval) {
1320+
if err := validateAutoPauseInterval(viper.GetDuration(autoPauseInterval)); err != nil {
1321+
exit.Message(reason.Usage, "{{.err}}", out.V{"err": err})
1322+
}
1323+
}
1324+
13181325
if driver.IsSSH(drvName) {
13191326
sshIPAddress := viper.GetString(sshIPAddress)
13201327
if sshIPAddress == "" {
@@ -1476,6 +1483,13 @@ func validateGPUsArch() error {
14761483
return errors.Errorf("The GPUs flag is only supported on amd64, arm64 & ppc64le, currently using %s", runtime.GOARCH)
14771484
}
14781485

1486+
func validateAutoPauseInterval(interval time.Duration) error {
1487+
if interval != interval.Abs() || interval.String() == "0s" {
1488+
return errors.New("auto-pause-interval must be greater than 0s")
1489+
}
1490+
return nil
1491+
}
1492+
14791493
func getContainerRuntime(old *config.ClusterConfig) string {
14801494
paramRuntime := viper.GetString(containerRuntime)
14811495

Diff for: ‎cmd/minikube/cmd/start_flags.go

+4
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ const (
142142
socketVMnetPath = "socket-vmnet-path"
143143
staticIP = "static-ip"
144144
gpus = "gpus"
145+
autoPauseInterval = "auto-pause-interval"
145146
)
146147

147148
var (
@@ -204,6 +205,7 @@ func initMinikubeFlags() {
204205
startCmd.Flags().Bool(disableMetrics, false, "If set, disables metrics reporting (CPU and memory usage), this can improve CPU usage. Defaults to false.")
205206
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)")
206207
startCmd.Flags().StringP(gpus, "g", "", "Allow pods to use your NVIDIA GPUs. Options include: [all,nvidia] (Docker driver with Docker container-runtime only)")
208+
startCmd.Flags().Duration(autoPauseInterval, time.Minute*1, "Duration of inactivity before the minikube VM is paused (default 1m0s)")
207209
}
208210

209211
// initKubernetesFlags inits the commandline flags for Kubernetes related options
@@ -603,6 +605,7 @@ func generateNewConfigFromFlags(cmd *cobra.Command, k8sVersion string, rtime str
603605
},
604606
MultiNodeRequested: viper.GetInt(nodes) > 1,
605607
GPUs: viper.GetString(gpus),
608+
AutoPauseInterval: viper.GetDuration(autoPauseInterval),
606609
}
607610
cc.VerifyComponents = interpretWaitFlag(*cmd)
608611
if viper.GetBool(createMount) && driver.IsKIC(drvName) {
@@ -817,6 +820,7 @@ func updateExistingConfigFromFlags(cmd *cobra.Command, existing *config.ClusterC
817820
updateStringFromFlag(cmd, &cc.CustomQemuFirmwarePath, qemuFirmwarePath)
818821
updateStringFromFlag(cmd, &cc.SocketVMnetClientPath, socketVMnetClientPath)
819822
updateStringFromFlag(cmd, &cc.SocketVMnetPath, socketVMnetPath)
823+
updateDurationFromFlag(cmd, &cc.AutoPauseInterval, autoPauseInterval)
820824

821825
if cmd.Flags().Changed(kubernetesVersion) {
822826
kubeVer, err := getKubernetesVersion(existing)

Diff for: ‎cmd/minikube/cmd/start_test.go

+27
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"fmt"
2121
"strings"
2222
"testing"
23+
"time"
2324

2425
"github.com/blang/semver/v4"
2526
"github.com/spf13/cobra"
@@ -886,3 +887,29 @@ func TestValidateGPUs(t *testing.T) {
886887
}
887888
}
888889
}
890+
891+
func TestValidateAutoPause(t *testing.T) {
892+
tests := []struct {
893+
interval string
894+
shouldError bool
895+
}{
896+
{"1m0s", false},
897+
{"5m", false},
898+
{"1s", false},
899+
{"0s", true},
900+
{"-2m", true},
901+
}
902+
for _, tc := range tests {
903+
input, err := time.ParseDuration(tc.interval)
904+
if err != nil {
905+
t.Fatalf("test has an invalid input duration of %q", tc.interval)
906+
}
907+
err = validateAutoPauseInterval(input)
908+
if err != nil && !tc.shouldError {
909+
t.Errorf("interval of %q failed validation; expected it to pass: %v", input, err)
910+
}
911+
if err == nil && tc.shouldError {
912+
t.Errorf("interval of %q passed validataion; expected it to fail: %v", input, err)
913+
}
914+
}
915+
}

Diff for: ‎deploy/addons/auto-pause/auto-pause.service.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ Description=Auto Pause Service
33

44
[Service]
55
Type=simple
6-
ExecStart=/bin/auto-pause --container-runtime={{.ContainerRuntime}}
6+
ExecStart=/bin/auto-pause --container-runtime={{.ContainerRuntime}} --interval={{.AutoPauseInterval}}
77
Restart=always
88

99
[Install]

Diff for: ‎pkg/drivers/kic/types.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ import (
2424

2525
const (
2626
// Version is the current version of kic
27-
Version = "v0.0.42-1704759386-17866"
27+
Version = "v0.0.42-1708008208-17936"
2828

2929
// SHA of the kic base image
30-
baseImageSHA = "8c3c33047f9bc285e1f5f2a5aa14744a2fe04c58478f02f77b06169dea8dd3f0"
30+
baseImageSHA = "4ea1136332ba1476cda33a97bf12e2f96995cc120674fbafd3ade22d1118ecdf"
3131
// The name of the GCR kicbase repository
3232
gcrRepo = "gcr.io/k8s-minikube/kicbase-builds"
3333
// The name of the Dockerhub kicbase repository

Diff for: ‎pkg/minikube/assets/addons.go

+3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"os"
2222
"runtime"
2323
"strings"
24+
"time"
2425

2526
semver "github.com/blang/semver/v4"
2627
"github.com/pkg/errors"
@@ -937,6 +938,7 @@ func GenerateTemplateData(addon *Addon, cc *config.ClusterConfig, netInfo Networ
937938
Environment map[string]string
938939
LegacyPodSecurityPolicy bool
939940
LegacyRuntimeClass bool
941+
AutoPauseInterval time.Duration
940942
}{
941943
KubernetesVersion: make(map[string]uint64),
942944
PreOneTwentyKubernetes: false,
@@ -958,6 +960,7 @@ func GenerateTemplateData(addon *Addon, cc *config.ClusterConfig, netInfo Networ
958960
},
959961
LegacyPodSecurityPolicy: v.LT(semver.Version{Major: 1, Minor: 25}),
960962
LegacyRuntimeClass: v.LT(semver.Version{Major: 1, Minor: 25}),
963+
AutoPauseInterval: cc.AutoPauseInterval,
961964
}
962965
if opts.ImageRepository != "" && !strings.HasSuffix(opts.ImageRepository, "/") {
963966
opts.ImageRepository += "/"

Diff for: ‎pkg/minikube/config/types.go

+1
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ type ClusterConfig struct {
108108
SSHAuthSock string
109109
SSHAgentPID int
110110
GPUs string
111+
AutoPauseInterval time.Duration // Specifies interval of time to wait before checking if cluster should be paused
111112
}
112113

113114
// KubernetesConfig contains the parameters used to configure the VM Kubernetes.

0 commit comments

Comments
 (0)