From 5e3386d5b45cc45558c182f397e123a1ef4dead1 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Thu, 11 Apr 2024 22:21:45 +0200 Subject: [PATCH 1/3] Use standard RunDir/DataDir for Supervisor That's how the other components are configuring the supervisor. Don't deviate from the standards unless there's a reason for it. Signed-off-by: Tom Wieczorek --- pkg/component/controller/cplb_unix.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/component/controller/cplb_unix.go b/pkg/component/controller/cplb_unix.go index eed18c44e846..da0b8c67c405 100644 --- a/pkg/component/controller/cplb_unix.go +++ b/pkg/component/controller/cplb_unix.go @@ -110,8 +110,8 @@ func (k *Keepalived) Start(_ context.Context) error { Name: "keepalived", BinPath: assets.BinPath("keepalived", k.K0sVars.BinDir), Args: args, - RunDir: filepath.Dir(k.K0sVars.KeepalivedConfigFile), - DataDir: filepath.Dir(k.K0sVars.KeepalivedConfigFile), + RunDir: k.K0sVars.RunDir, + DataDir: k.K0sVars.DataDir, UID: k.uid, } return k.supervisor.Supervise() From daebdf1c5dd665260c0e1879d5f1ea09c9cadbdd Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Thu, 11 Apr 2024 22:31:13 +0200 Subject: [PATCH 2/3] Use k0s's RunDir for keepalived config file The file is ephemeral and doesn't need to live on a persistent path. This also saves us from ensuring the existence of the base directory in the cplb component, as the RunDir's existence is ensured during controller startup. Signed-off-by: Tom Wieczorek --- pkg/component/controller/cplb_unix.go | 20 ++++++-------------- pkg/config/cfgvars.go | 2 -- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/pkg/component/controller/cplb_unix.go b/pkg/component/controller/cplb_unix.go index da0b8c67c405..3a527eb0d691 100644 --- a/pkg/component/controller/cplb_unix.go +++ b/pkg/component/controller/cplb_unix.go @@ -29,7 +29,6 @@ import ( "slices" "text/template" - "github.com/k0sproject/k0s/internal/pkg/dir" "github.com/k0sproject/k0s/internal/pkg/users" k0sAPI "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" "github.com/k0sproject/k0s/pkg/assets" @@ -48,6 +47,7 @@ type Keepalived struct { uid int supervisor *supervisor.Supervisor log *logrus.Entry + configFilePath string } // Init extracts the needed binaries and creates the directories @@ -63,15 +63,7 @@ func (k *Keepalived) Init(_ context.Context) error { k.log.Warnf("Unable to get %s UID running keepalived as root: %v", constant.KeepalivedUser, err) } - basepath := filepath.Dir(k.K0sVars.KeepalivedConfigFile) - if err = dir.Init(basepath, constant.KeepalivedDirMode); err != nil { - return fmt.Errorf("failed to create keepalived data dir: %w", err) - } - - if err = os.Chown(basepath, k.uid, -1); err != nil { - return fmt.Errorf("failed to chown keepalived data dir: %w", err) - } - + k.configFilePath = filepath.Join(k.K0sVars.RunDir, "keepalived.conf") return assets.Stage(k.K0sVars.BinDir, "keepalived", constant.BinDirMode) } @@ -96,7 +88,7 @@ func (k *Keepalived) Start(_ context.Context) error { args := []string{ "--dont-fork", "--use-file", - k.K0sVars.KeepalivedConfigFile, + k.configFilePath, "--no-syslog", "--log-console", } @@ -274,7 +266,7 @@ func (*Keepalived) getLinkAddresses(link netlink.Link) ([]netlink.Addr, []string } func (k *Keepalived) generateKeepalivedTemplate() error { - f, err := os.OpenFile(k.K0sVars.KeepalivedConfigFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, fs.FileMode(0500)) + f, err := os.OpenFile(k.configFilePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, fs.FileMode(0500)) if err != nil { return fmt.Errorf("failed to open keepalived config file: %w", err) } @@ -293,10 +285,10 @@ func (k *Keepalived) generateKeepalivedTemplate() error { } // TODO: Do we really need to this every single time? - if err = os.Chown(k.K0sVars.KeepalivedConfigFile, k.uid, -1); err != nil { + if err = os.Chown(k.configFilePath, k.uid, -1); err != nil { return fmt.Errorf("failed to chown keepalived config file: %w", err) } - if err = os.Chmod(k.K0sVars.KeepalivedConfigFile, fs.FileMode(0400)); err != nil { + if err = os.Chmod(k.configFilePath, fs.FileMode(0400)); err != nil { return fmt.Errorf("failed to chmod keepalived config file: %w", err) } return nil diff --git a/pkg/config/cfgvars.go b/pkg/config/cfgvars.go index 006e8b5ffc07..3257ac050193 100644 --- a/pkg/config/cfgvars.go +++ b/pkg/config/cfgvars.go @@ -49,7 +49,6 @@ type CfgVars struct { EtcdCertDir string // EtcdCertDir contains etcd certificates EtcdDataDir string // EtcdDataDir contains etcd state KineSocketPath string // The unix socket path for kine - KeepalivedConfigFile string // location for keepalived data KonnectivitySocketDir string // location of konnectivity's socket path KubeletAuthConfigPath string // KubeletAuthConfigPath defines the default kubelet auth config path KubeletVolumePluginDir string // location for kubelet plugins volume executables @@ -179,7 +178,6 @@ func NewCfgVars(cobraCmd command, dirs ...string) (*CfgVars, error) { EtcdCertDir: filepath.Join(certDir, "etcd"), EtcdDataDir: filepath.Join(dataDir, "etcd"), KineSocketPath: filepath.Join(runDir, constant.KineSocket), - KeepalivedConfigFile: filepath.Join(dataDir, "keepalived", "keepalived.conf"), KonnectivitySocketDir: filepath.Join(runDir, "konnectivity-server"), KubeletAuthConfigPath: filepath.Join(dataDir, "kubelet.conf"), KubeletVolumePluginDir: constant.KubeletVolumePluginDir, From 4c0a8204a8198241c7a62793f851c3d5a536e98b Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Fri, 12 Apr 2024 00:14:24 +0200 Subject: [PATCH 3/3] Use file.WriteAtomically to write keepalived config file This aligns with other file generation routines in k0s. Also, use template.Must(...) to parse the template string, as it's a hard coded constant. Signed-off-by: Tom Wieczorek --- pkg/component/controller/cplb_unix.go | 38 ++++++++++++--------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/pkg/component/controller/cplb_unix.go b/pkg/component/controller/cplb_unix.go index 3a527eb0d691..1781894c6abd 100644 --- a/pkg/component/controller/cplb_unix.go +++ b/pkg/component/controller/cplb_unix.go @@ -19,16 +19,18 @@ limitations under the License. package controller import ( + "bufio" "context" "errors" "fmt" - "io/fs" + "io" "net" "os" "path/filepath" "slices" "text/template" + "github.com/k0sproject/k0s/internal/pkg/file" "github.com/k0sproject/k0s/internal/pkg/users" k0sAPI "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" "github.com/k0sproject/k0s/pkg/assets" @@ -266,31 +268,25 @@ func (*Keepalived) getLinkAddresses(link netlink.Link) ([]netlink.Addr, []string } func (k *Keepalived) generateKeepalivedTemplate() error { - f, err := os.OpenFile(k.configFilePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, fs.FileMode(0500)) - if err != nil { - return fmt.Errorf("failed to open keepalived config file: %w", err) - } - defer f.Close() - - template, err := template.New("keepalived").Parse(keepalivedConfigTemplate) - if err != nil { - return fmt.Errorf("failed to parse keepalived template: %w", err) - } - + template := template.Must(template.New("keepalived").Parse(keepalivedConfigTemplate)) kc := keepalivedConfig{ VRRPInstances: k.Config.VRRPInstances, } - if err = template.Execute(f, kc); err != nil { - return fmt.Errorf("failed to execute keepalived template: %w", err) - } - // TODO: Do we really need to this every single time? - if err = os.Chown(k.configFilePath, k.uid, -1); err != nil { - return fmt.Errorf("failed to chown keepalived config file: %w", err) - } - if err = os.Chmod(k.configFilePath, fs.FileMode(0400)); err != nil { - return fmt.Errorf("failed to chmod keepalived config file: %w", err) + if err := file.WriteAtomically(k.configFilePath, 0400, func(file io.Writer) error { + if err := file.(*os.File).Chown(k.uid, -1); err != nil { + return err + } + + w := bufio.NewWriter(file) + if err := template.Execute(w, kc); err != nil { + return err + } + return w.Flush() + }); err != nil { + return fmt.Errorf("failed to write keepalived config file: %w", err) } + return nil }