Skip to content

Commit

Permalink
Remove *os.File hack
Browse files Browse the repository at this point in the history
Use file.AtomicOpener directly when writing the keepalived config file.
This was the only hack that relied on using *os.File directly.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
  • Loading branch information
twz123 committed Apr 26, 2024
1 parent e66cbae commit 1d73bf2
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 26 deletions.
5 changes: 1 addition & 4 deletions internal/pkg/file/atomic.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,7 @@ func remove(fd *os.File) error {
// https://github.com/golang/go/issues/22397#issuecomment-498856679
func WriteAtomically(fileName string, perm os.FileMode, write func(file io.Writer) error) (err error) {
return AtomicWithTarget(fileName).WithPermissions(perm).Do(func(unbuffered AtomicWriter) error {
// Pass the real file to the callback, so that callers that are cheating
// and relied on this implementation detail won't break.
fd := unbuffered.(*Atomic).fd
return write(fd)
return write(unbuffered)
})
}

Expand Down
18 changes: 9 additions & 9 deletions internal/pkg/file/atomic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,9 @@ func TestWriteAtomically(t *testing.T) {
file := filepath.Join(dir, "file")

errs := flatten(WriteAtomically(file, 0644, func(file io.Writer) error {
c, ok := file.(io.Closer)
require.True(t, ok, "Not closeable: %T", file)
require.NoError(t, c.Close())
a, ok := file.(*Atomic)
require.True(t, ok, "Not an Atomic: %T", file)
require.NoError(t, a.fd.Close())
return nil
}))

Expand Down Expand Up @@ -169,9 +169,9 @@ func TestWriteAtomically(t *testing.T) {

var tempPath string
err := WriteAtomically(file, 0644, func(file io.Writer) error {
n, ok := file.(interface{ Name() string })
require.True(t, ok, "Doesn't have a name: %T", file)
tempPath = n.Name()
a, ok := file.(*Atomic)
require.True(t, ok, "Not an Atomic: %T", file)
tempPath = a.fd.Name()
require.Equal(t, dir, filepath.Dir(tempPath))
if runtime.GOOS == "windows" {
t.Skip("Cannot remove a file which is still opened on Windows")
Expand Down Expand Up @@ -245,9 +245,9 @@ func TestWriteAtomically(t *testing.T) {

var tempPath string
errs := flatten(WriteAtomically(file, 0755, func(file io.Writer) error {
n, ok := file.(interface{ Name() string })
require.True(t, ok, "Doesn't have a name: %T", file)
tempPath = n.Name()
a, ok := file.(*Atomic)
require.True(t, ok, "Not an Atomic: %T", file)
tempPath = a.fd.Name()
require.Equal(t, dir, filepath.Dir(tempPath))

if runtime.GOOS == "windows" {
Expand Down
23 changes: 10 additions & 13 deletions pkg/component/controller/cplb_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ import (
"context"
"errors"
"fmt"
"io"
"net"
"os"
"path/filepath"
"slices"
"text/template"
Expand Down Expand Up @@ -273,17 +271,16 @@ func (k *Keepalived) generateKeepalivedTemplate() error {
VRRPInstances: k.Config.VRRPInstances,
}

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 {
if err := file.AtomicWithTarget(k.configFilePath).
WithPermissions(0400).
WithOwner(k.uid).
Do(func(unbuffered file.AtomicWriter) error {
w := bufio.NewWriter(unbuffered)
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)
}

Expand Down

0 comments on commit 1d73bf2

Please sign in to comment.