From 7fdfc0a6466e8733c56b61d02b2dc66a1936ddb0 Mon Sep 17 00:00:00 2001 From: Natanael Copa Date: Tue, 14 Jan 2025 13:18:53 +0100 Subject: [PATCH] Replace pid with flock for runtime config loading Use lock file and flock(2) to ensure there is only a single instance of k0s running. This is more reliable than storing the pid in the runtime config. This solves false positives with k0s runtime config leftovers. Fixes: https://github.com/k0sproject/k0s/issues/5399 Signed-off-by: Natanael Copa --- go.mod | 1 + go.sum | 2 ++ pkg/config/runtime.go | 49 +++++++++++++++++++++-------------- pkg/config/runtime_test.go | 13 ++++++---- pkg/config/runtime_unix.go | 38 --------------------------- pkg/config/runtime_windows.go | 33 ----------------------- 6 files changed, 41 insertions(+), 95 deletions(-) delete mode 100644 pkg/config/runtime_unix.go delete mode 100644 pkg/config/runtime_windows.go diff --git a/go.mod b/go.mod index 36fde7132274..55fec83412d8 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( github.com/go-logr/logr v1.4.2 github.com/go-openapi/jsonpointer v0.21.0 github.com/go-playground/validator/v10 v10.24.0 + github.com/gofrs/flock v0.8.1 github.com/google/go-cmp v0.6.0 github.com/k0sproject/bootloose v0.9.0 github.com/k0sproject/version v0.6.0 diff --git a/go.sum b/go.sum index 00da6e193328..bd007490acbe 100644 --- a/go.sum +++ b/go.sum @@ -235,6 +235,8 @@ github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJA github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk= github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= +github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw= +github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= diff --git a/pkg/config/runtime.go b/pkg/config/runtime.go index a94c67f8869f..96232550b546 100644 --- a/pkg/config/runtime.go +++ b/pkg/config/runtime.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" + "github.com/gofrs/flock" "github.com/k0sproject/k0s/internal/pkg/dir" "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" "github.com/k0sproject/k0s/pkg/constant" @@ -56,7 +57,7 @@ type RuntimeConfig struct { type RuntimeConfigSpec struct { NodeConfig *v1beta1.ClusterConfig `json:"nodeConfig"` K0sVars *CfgVars `json:"k0sVars"` - Pid int `json:"pid"` + lock *flock.Flock } func LoadRuntimeConfig(path string) (*RuntimeConfigSpec, error) { @@ -83,22 +84,28 @@ func LoadRuntimeConfig(path string) (*RuntimeConfigSpec, error) { return nil, fmt.Errorf("%w: spec is nil", ErrInvalidRuntimeConfig) } - // If a pid is defined but there's no process found, the instance of k0s is - // expected to have died, in which case the existing config is removed and - // an error is returned, which allows the controller startup to proceed to - // initialize a new runtime config. - if spec.Pid != 0 { - if err := checkPid(spec.Pid); err != nil { - defer func() { _ = spec.Cleanup() }() - return nil, errors.Join(ErrK0sNotRunning, err) - } - } - return spec, nil } func NewRuntimeConfig(k0sVars *CfgVars) (*RuntimeConfigSpec, error) { - if _, err := LoadRuntimeConfig(k0sVars.RuntimeConfigPath); err == nil { + if err := dir.Init(filepath.Dir(k0sVars.RuntimeConfigPath), constant.RunDirMode); err != nil { + logrus.Warnf("failed to initialize runtime config dir: %v", err) + } + + // A file lock is acquired using `flock(2)` to ensure that only one + // instance of the `k0s` process can modify the runtime configuration + // at a time. The lock is tied to the lifetime of the `k0s` process, + // meaning that if the process terminates unexpectedly, the lock is + // automatically released by the operating system. This ensures that + // subsequent processes can acquire the lock without manual cleanup. + // https://man7.org/linux/man-pages/man2/flock.2.html + + lock := flock.New(k0sVars.RuntimeConfigPath + ".lock") + locked, err := lock.TryLock() + if err != nil { + return nil, fmt.Errorf("failed to aquire lock on runtime config: %w", err) + } + if !locked { return nil, ErrK0sAlreadyRunning } @@ -118,7 +125,7 @@ func NewRuntimeConfig(k0sVars *CfgVars) (*RuntimeConfigSpec, error) { Spec: &RuntimeConfigSpec{ NodeConfig: nodeConfig, K0sVars: k0sVars, - Pid: os.Getpid(), + lock: lock, }, } @@ -127,10 +134,6 @@ func NewRuntimeConfig(k0sVars *CfgVars) (*RuntimeConfigSpec, error) { return nil, err } - if err := dir.Init(filepath.Dir(k0sVars.RuntimeConfigPath), constant.RunDirMode); err != nil { - logrus.Warnf("failed to initialize runtime config dir: %v", err) - } - if err := os.WriteFile(k0sVars.RuntimeConfigPath, content, 0600); err != nil { return nil, fmt.Errorf("failed to write runtime config: %w", err) } @@ -144,7 +147,15 @@ func (r *RuntimeConfigSpec) Cleanup() error { } if err := os.Remove(r.K0sVars.RuntimeConfigPath); err != nil { - return fmt.Errorf("failed to clean up runtime config file: %w", err) + logrus.Warnf("failed to clean up runtime config file: %v", err) + } + + if err := r.lock.Close(); err != nil { + return fmt.Errorf("failed to unlock runtime config: %w", err) + } + + if err := os.Remove(r.lock.Path()); err != nil { + return fmt.Errorf("failed to delete %s: %w", r.lock.Path(), err) } return nil } diff --git a/pkg/config/runtime_test.go b/pkg/config/runtime_test.go index 390a534d6eb1..7532677b56cf 100644 --- a/pkg/config/runtime_test.go +++ b/pkg/config/runtime_test.go @@ -27,7 +27,12 @@ import ( "sigs.k8s.io/yaml" ) -func TestLoadRuntimeConfig_K0sNotRunning(t *testing.T) { +func TestLoadRuntimeConfig(t *testing.T) { + // create a temporary file for runtime config + tmpfile, err := os.CreateTemp("", "runtime-config") + require.NoError(t, err) + defer os.Remove(tmpfile.Name()) + // write some content to the runtime config file rtConfigPath := filepath.Join(t.TempDir(), "runtime-config") content := []byte(`--- @@ -37,14 +42,13 @@ spec: nodeConfig: metadata: name: k0s - pid: -1 `) require.NoError(t, os.WriteFile(rtConfigPath, content, 0644)) // try to load runtime config and check if it returns an error spec, err := LoadRuntimeConfig(rtConfigPath) - assert.Nil(t, spec) - assert.ErrorIs(t, err, ErrK0sNotRunning) + require.NoError(t, err) + assert.NotNil(t, spec) } func TestNewRuntimeConfig(t *testing.T) { @@ -74,7 +78,6 @@ func TestNewRuntimeConfig(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, spec) assert.Same(t, k0sVars, spec.K0sVars) - assert.Equal(t, os.Getpid(), spec.Pid) assert.NotNil(t, spec.NodeConfig) cfg, err := spec.K0sVars.NodeConfig() assert.NoError(t, err) diff --git a/pkg/config/runtime_unix.go b/pkg/config/runtime_unix.go deleted file mode 100644 index 134e6a863ee6..000000000000 --- a/pkg/config/runtime_unix.go +++ /dev/null @@ -1,38 +0,0 @@ -//go:build unix - -/* -Copyright 2023 k0s 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 config - -import ( - "fmt" - "os" - "syscall" -) - -func checkPid(pid int) error { - proc, err := os.FindProcess(pid) - if err != nil { - return fmt.Errorf("failed to find process: %w", err) - } - - if err := proc.Signal(syscall.Signal(0)); err != nil { - return fmt.Errorf("failed to signal process: %w", err) - } - - return nil -} diff --git a/pkg/config/runtime_windows.go b/pkg/config/runtime_windows.go deleted file mode 100644 index dd8903c93c0d..000000000000 --- a/pkg/config/runtime_windows.go +++ /dev/null @@ -1,33 +0,0 @@ -/* -Copyright 2023 k0s 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 config - -import ( - "fmt" - - "golang.org/x/sys/windows" -) - -func checkPid(pid int) error { - procHandle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) - if err != nil { - return fmt.Errorf("failed to find process: %w", err) - } - defer func() { _ = windows.CloseHandle(procHandle) }() - - return nil -}