diff --git a/cmd/scheduler/main.go b/cmd/scheduler/main.go index f7c68d1585..8da9f507fb 100644 --- a/cmd/scheduler/main.go +++ b/cmd/scheduler/main.go @@ -76,7 +76,10 @@ func init() { rootCmd.Flags().IntVar(&config.Burst, "kube-burst", client.DefaultBurst, "Burst to use while talking with kube-apiserver.") rootCmd.Flags().IntVar(&config.Timeout, "kube-timeout", client.DefaultTimeout, "Timeout to use while talking with kube-apiserver.") rootCmd.Flags().BoolVar(&enableProfiling, "profiling", false, "Enable pprof profiling via HTTP server") - rootCmd.Flags().DurationVar(&config.NodeLockTimeout, "node-lock-timeout", time.Minute*5, "timeout for node locks") + // nodelock's package init runs before this one and has already applied + // HAMI_NODELOCK_EXPIRE, so taking its value as the flag default leaves + // precedence as flag, then environment, then default. + rootCmd.Flags().DurationVar(&config.NodeLockTimeout, "node-lock-timeout", nodelock.NodeLockTimeout, "timeout for node locks") rootCmd.Flags().DurationVar(&config.NodeLockRetryTimeout, "node-lock-retry-timeout", 28*time.Second, "timeout for retrying LockNode when contended by another PodGroup member (0 disables retry). Align the Extender's httpTimeout in KubeSchedulerConfiguration with this value.") rootCmd.Flags().BoolVar(&config.ForceOverwriteDefaultScheduler, "force-overwrite-default-scheduler", true, "Overwrite schedulerName in Pod Spec when set to the const DefaultSchedulerName in https://k8s.io/api/core/v1 package") @@ -110,7 +113,6 @@ func injectProfilingRoute(router *httprouter.Router) { } func start() error { - // Initialize node lock timeout from config nodelock.NodeLockTimeout = config.NodeLockTimeout klog.InfoS("Set node lock timeout", "timeout", nodelock.NodeLockTimeout) client.InitGlobalClient( diff --git a/cmd/scheduler/nodelock_timeout_test.go b/cmd/scheduler/nodelock_timeout_test.go new file mode 100644 index 0000000000..bedfa24f64 --- /dev/null +++ b/cmd/scheduler/nodelock_timeout_test.go @@ -0,0 +1,42 @@ +/* +Copyright 2024 The HAMi Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "testing" + + "github.com/Project-HAMi/HAMi/pkg/scheduler/config" + "github.com/Project-HAMi/HAMi/pkg/util/nodelock" +) + +// TestNodeLockTimeoutFlagDefault guards the precedence of flag, then +// environment, then default. nodelock's init applies HAMI_NODELOCK_EXPIRE +// before this package's init registers flags, so the flag default must be +// nodelock.NodeLockTimeout. Hard-coding a duration here instead would discard +// the environment value on every start, which is the bug in #2692. +func TestNodeLockTimeoutFlagDefault(t *testing.T) { + f := rootCmd.Flags().Lookup("node-lock-timeout") + if f == nil { + t.Fatal("node-lock-timeout flag is not registered") + } + if want := nodelock.NodeLockTimeout.String(); f.DefValue != want { + t.Errorf("node-lock-timeout default = %s, want %s (nodelock.NodeLockTimeout)", f.DefValue, want) + } + if config.NodeLockTimeout != nodelock.NodeLockTimeout { + t.Errorf("config.NodeLockTimeout = %v, want %v", config.NodeLockTimeout, nodelock.NodeLockTimeout) + } +}