From 97349aee27d8905375ad26e24466bfee4891e63b Mon Sep 17 00:00:00 2001 From: Patryk Matuszak Date: Mon, 27 Jan 2025 10:52:31 +0100 Subject: [PATCH 1/3] Move function GenerateUniqueTempPath to util pkg --- pkg/admin/autorecovery/state.go | 2 +- pkg/admin/data/atomic_dir_copy.go | 20 +------------------- pkg/util/util.go | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/admin/autorecovery/state.go b/pkg/admin/autorecovery/state.go index f91c5958c5..3314755cd2 100644 --- a/pkg/admin/autorecovery/state.go +++ b/pkg/admin/autorecovery/state.go @@ -32,7 +32,7 @@ func (s *state) SaveToIntermediate() error { if err != nil { return err } - path, err := data.GenerateUniqueTempPath(filepath.Join(s.storagePath, stateFilename)) + path, err := util.GenerateUniqueTempPath(filepath.Join(s.storagePath, stateFilename)) if err != nil { return err } diff --git a/pkg/admin/data/atomic_dir_copy.go b/pkg/admin/data/atomic_dir_copy.go index 17c673001f..b4339a8220 100644 --- a/pkg/admin/data/atomic_dir_copy.go +++ b/pkg/admin/data/atomic_dir_copy.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "math/rand/v2" "os" "os/exec" "strings" @@ -42,7 +41,7 @@ func (c *AtomicDirCopy) CopyToIntermediate() error { return nil } var err error - c.intermediatePath, err = GenerateUniqueTempPath(c.Destination) + c.intermediatePath, err = util.GenerateUniqueTempPath(c.Destination) if err != nil { return err } @@ -131,20 +130,3 @@ func removeDirIfExists(path string) error { return nil } - -// GenerateUniqueTempPath returns a filepath from given path with extra suffix -// which doesn't exist. -func GenerateUniqueTempPath(path string) (string, error) { - // 1000 tries - for i := 0; i < 1000; i++ { - //nolint:gosec - rnd := rand.IntN(100000) - newPath := fmt.Sprintf("%s.tmp.%d", path, rnd) - if exists, err := util.PathExists(newPath); err != nil { - return "", err - } else if !exists { - return newPath, nil - } - } - return "", fmt.Errorf("attempted to generate unique temporary path (%q) for 1000 tries - giving up", path) -} diff --git a/pkg/util/util.go b/pkg/util/util.go index 7237a94da2..bdfe88111e 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io/fs" + "math/rand/v2" "net/http" "os" "path/filepath" @@ -142,3 +143,20 @@ func (l LogFilePath) Remove() error { } return err } + +// GenerateUniqueTempPath returns a filepath from given path with extra suffix +// which doesn't exist. +func GenerateUniqueTempPath(path string) (string, error) { + // 1000 tries + for i := 0; i < 1000; i++ { + //nolint:gosec + rnd := rand.IntN(100000) + newPath := fmt.Sprintf("%s.tmp.%d", path, rnd) + if exists, err := PathExists(newPath); err != nil { + return "", err + } else if !exists { + return newPath, nil + } + } + return "", fmt.Errorf("attempted to generate unique temporary path (%q) for 1000 tries - giving up", path) +} From 75839be9a24259515ee564950d9083a948e2e4db Mon Sep 17 00:00:00 2001 From: Patryk Matuszak Date: Mon, 27 Jan 2025 10:52:40 +0100 Subject: [PATCH 2/3] Atomically create .nodename file --- pkg/config/node.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/config/node.go b/pkg/config/node.go index 76d6f65123..72084179e2 100644 --- a/pkg/config/node.go +++ b/pkg/config/node.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" + "github.com/openshift/microshift/pkg/util" "k8s.io/klog/v2" ) @@ -49,9 +50,19 @@ func (c *Config) establishNodeName(dataDir string) (string, error) { if err := os.MkdirAll(dataDir, 0700); err != nil { return "", fmt.Errorf("failed to create data dir: %w", err) } - if err := os.WriteFile(filePath, []byte(name), 0400); err != nil { + tmpPath, err := util.GenerateUniqueTempPath(filePath) + if err != nil { + return "", fmt.Errorf("failed to generate temp path for %s: %w", filePath, err) + } + if err := os.WriteFile(tmpPath, []byte(name), 0400); err != nil { return "", fmt.Errorf("failed to write nodename file %q: %v", filePath, err) } + if err := os.Rename(tmpPath, filePath); err != nil { + if err := os.RemoveAll(tmpPath); err != nil { + klog.Errorf("Failed to remove intermediate path %q: %v", tmpPath, err) + } + return "", fmt.Errorf("failed to rename %s to %s: %w", tmpPath, filePath, err) + } return name, nil } else if err != nil { return "", err From 452621e08cba3cd4c59956bd9d44f41c02226605 Mon Sep 17 00:00:00 2001 From: Patryk Matuszak Date: Mon, 3 Feb 2025 13:46:12 +0100 Subject: [PATCH 3/3] Extract .nodename creation logic to separate function Satisfy linter (complexity) --- pkg/config/node.go | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/pkg/config/node.go b/pkg/config/node.go index 72084179e2..5cdd07fbe1 100644 --- a/pkg/config/node.go +++ b/pkg/config/node.go @@ -46,22 +46,8 @@ func (c *Config) establishNodeName(dataDir string) (string, error) { filePath := filepath.Join(dataDir, ".nodename") contents, err := os.ReadFile(filePath) if os.IsNotExist(err) { - // ensure that dataDir exists - if err := os.MkdirAll(dataDir, 0700); err != nil { - return "", fmt.Errorf("failed to create data dir: %w", err) - } - tmpPath, err := util.GenerateUniqueTempPath(filePath) - if err != nil { - return "", fmt.Errorf("failed to generate temp path for %s: %w", filePath, err) - } - if err := os.WriteFile(tmpPath, []byte(name), 0400); err != nil { - return "", fmt.Errorf("failed to write nodename file %q: %v", filePath, err) - } - if err := os.Rename(tmpPath, filePath); err != nil { - if err := os.RemoveAll(tmpPath); err != nil { - klog.Errorf("Failed to remove intermediate path %q: %v", tmpPath, err) - } - return "", fmt.Errorf("failed to rename %s to %s: %w", tmpPath, filePath, err) + if err := c.createNodeNameFile(name, filePath, dataDir); err != nil { + return "", err } return name, nil } else if err != nil { @@ -70,6 +56,27 @@ func (c *Config) establishNodeName(dataDir string) (string, error) { return string(contents), nil } +func (c *Config) createNodeNameFile(nodeName, filePath, dataDir string) error { + // ensure that dataDir exists + if err := os.MkdirAll(dataDir, 0700); err != nil { + return fmt.Errorf("failed to create data dir: %w", err) + } + tmpPath, err := util.GenerateUniqueTempPath(filePath) + if err != nil { + return fmt.Errorf("failed to generate temp path for %s: %w", filePath, err) + } + if err := os.WriteFile(tmpPath, []byte(nodeName), 0400); err != nil { + return fmt.Errorf("failed to write nodename file %q: %v", filePath, err) + } + if err := os.Rename(tmpPath, filePath); err != nil { + if err := os.RemoveAll(tmpPath); err != nil { + klog.Errorf("Failed to remove intermediate path %q: %v", tmpPath, err) + } + return fmt.Errorf("failed to rename %s to %s: %w", tmpPath, filePath, err) + } + return nil +} + // Validate the NodeName to be used for this MicroShift instances func (c *Config) validateNodeName(isDefaultNodeName bool, dataDir string) error { currentNodeName := c.CanonicalNodeName()