Skip to content

Commit

Permalink
pkg/endpoint: simplify state directory synchronization
Browse files Browse the repository at this point in the history
BPF regeneration writes state into a new temporary directory. Once it
has succeeded we need to swap the old and new directory. This is currently
achieved by "backing up" the current state by renaming the directory.
This code has a bunch of corner cases around cleaning up old directories
and so on.

Instead, use the RENAME_EXCHANGE flag to atomically exchange the two existing
directories.

Signed-off-by: Lorenz Bauer <[email protected]>
  • Loading branch information
lmb committed May 9, 2024
1 parent d9ea218 commit 1e78f20
Showing 1 changed file with 6 additions and 44 deletions.
50 changes: 6 additions & 44 deletions pkg/endpoint/directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import (
"path/filepath"

"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"

"github.com/cilium/cilium/pkg/logging"
"github.com/cilium/cilium/pkg/logging/logfields"
"github.com/cilium/cilium/pkg/option"
)

Expand Down Expand Up @@ -43,10 +43,6 @@ func (e *Endpoint) NextDirectoryPath() string {
return filepath.Join(".", fmt.Sprintf("%d%s", e.ID, nextDirectorySuffix))
}

func (e *Endpoint) backupDirectoryPath() string {
return e.DirectoryPath() + backupDirectorySuffix
}

// moveNewFilesTo copies all files, that do not exist in newDir, from oldDir.
// It assumes that oldDir and newDir are an endpoint's old and new state
// directories (see synchronizeDirectories below).
Expand Down Expand Up @@ -115,52 +111,19 @@ func (e *Endpoint) synchronizeDirectories(origDir string, stateDirComplete bool)
// An endpoint directory already exists. We need to back it up before attempting
// to move the new directory in its place so we can attempt recovery.
case !os.IsNotExist(err):
scopedLog.Debug("endpoint directory exists; backing it up")
backupDir := e.backupDirectoryPath()

// Remove any eventual old backup directory. This may fail if
// the directory does not exist. The error is deliberately
// ignored.
e.removeDirectory(backupDir)

// Move the current endpoint directory to a backup location
if debugLogEnabled {
scopedLog.WithFields(logrus.Fields{
"originalDirectory": origDir,
"backupDirectory": backupDir,
}).Debug("moving current directory to backup location")
}

if err := os.Rename(origDir, backupDir); err != nil {
return fmt.Errorf("unable to rename current endpoint directory: %w", err)
}

// Regarldess of whether the atomic replace succeeds or not,
// ensure that the backup directory is removed when the
// function returns.
defer e.removeDirectory(backupDir)

// Make temporary directory the new endpoint directory
if err := os.Rename(tmpDir, origDir); err != nil {
if err2 := os.Rename(backupDir, origDir); err2 != nil {
scopedLog.WithFields(logrus.Fields{
logfields.Path: backupDir,
}).Warn("restoring directory for endpoint failed, endpoint " +
"is in inconsistent state. Keeping stale directory.")
return err2
}

return fmt.Errorf("restored original endpoint directory, atomic directory move failed: %w", err)
// Atomically exchange the two directories.
if err := unix.Renameat2(unix.AT_FDCWD, tmpDir, unix.AT_FDCWD, origDir, unix.RENAME_EXCHANGE); err != nil {
return fmt.Errorf("unable to exchange %s with %s: %w", origDir, tmpDir, err)
}

// If the compilation was skipped then we need to copy the old
// bpf objects into the new directory
if !stateDirComplete {
scopedLog.Debug("some BPF state files were not recreated; moving old BPF objects into new directory")
err := moveNewFilesTo(backupDir, origDir)
err := moveNewFilesTo(tmpDir, origDir)
if err != nil {
log.WithError(err).Debugf("unable to copy old bpf object "+
"files from %s into the new directory %s.", backupDir, origDir)
"files from %s into the new directory %s.", tmpDir, origDir)
}
}

Expand Down Expand Up @@ -198,5 +161,4 @@ func (e *Endpoint) removeDirectories() {
e.removeDirectory(e.DirectoryPath())
e.removeDirectory(e.FailedDirectoryPath())
e.removeDirectory(e.NextDirectoryPath())
e.removeDirectory(e.backupDirectoryPath())
}

0 comments on commit 1e78f20

Please sign in to comment.