Skip to content

Commit

Permalink
robot test was flaky, fixed it.
Browse files Browse the repository at this point in the history
  • Loading branch information
guettli committed Dec 16, 2024
1 parent 15237fd commit 5e14043
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 21 deletions.
2 changes: 1 addition & 1 deletion hcloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (lt *LoggingTransport) RoundTrip(req *http.Request) (resp *http.Response, e

func newHcloudClient(rootDir string) (*hcloud.Client, error) {
secretDir := filepath.Join(rootDir, "etc", "hetzner-secret")
token, err := hotreload.ReadHcloudCredentialsFromDirectory(secretDir)
token, err := hotreload.GetInitialHcloudCredentialsFromDirectory(secretDir)
if err != nil {
klog.V(1).Infof("reading Hetzner Cloud token from directory failed. Will try env var: %s", err.Error())
token = os.Getenv(hcloudTokenENVVar)
Expand Down
52 changes: 38 additions & 14 deletions internal/hotreload/hotreload.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,23 @@ import (
)

var (
// fsnotify creates several events for a single update of a mounted secret.
// To avoid multiple reloads, we store the old values and only reload when
// the values have changed.
oldRobotUser string
oldRobotPassword string
oldHcloudToken string

// RobotReloadCounter gets incremented when the credentials get reloaded.
// Mosty used for testing.
RobotReloadCounter uint64
robotMutex sync.Mutex

oldHcloudToken string
// HcloudTokenReloadCounter gets incremented when the credentials get reloaded.
// Mosty used for testing.
HcloudTokenReloadCounter uint64
hcloudMutex sync.Mutex

hcloudMutex sync.Mutex
robotMutex sync.Mutex
)

// Watch the mounted secrets. Reload the credentials, when the files get updated. The robotClient can be nil.
Expand All @@ -48,19 +53,19 @@ func Watch(hetznerSecretDirectory string, hcloudClient *hcloud.Client, robotClie
var err error
switch baseName {
case "robot-user":
err = LoadRobotCredentials(hetznerSecretDirectory, robotClient)
err = loadRobotCredentials(hetznerSecretDirectory, robotClient)
case "robot-password":
err = LoadRobotCredentials(hetznerSecretDirectory, robotClient)
err = loadRobotCredentials(hetznerSecretDirectory, robotClient)
case "hcloud":
err = LoadHcloudCredentials(hetznerSecretDirectory, hcloudClient)
err = loadHcloudCredentials(hetznerSecretDirectory, hcloudClient)
case "..data":
// The files (for example hcloud) are symlinks to ..data/. For example to ../data/hcloud
// This means the files/symlinks don't change. When the secrets get changed, then
// a new ..data directory gets created. This is done by Kubernetes to make the
// update atomic.
err = LoadHcloudCredentials(hetznerSecretDirectory, hcloudClient)
err = loadHcloudCredentials(hetznerSecretDirectory, hcloudClient)
if robotClient != nil {
err = errors.Join(err, LoadRobotCredentials(hetznerSecretDirectory, robotClient))
err = errors.Join(err, loadRobotCredentials(hetznerSecretDirectory, robotClient))
}
default:
klog.Infof("Ignoring fsnotify event for %q: %s", baseName, event.String())
Expand Down Expand Up @@ -95,10 +100,10 @@ func isValidEvent(event fsnotify.Event) bool {
return false
}

func LoadRobotCredentials(hetznerSecretDirectory string, robotClient robotclient.Client) error {
func loadRobotCredentials(hetznerSecretDirectory string, robotClient robotclient.Client) error {
robotMutex.Lock()
defer robotMutex.Unlock()
username, password, err := ReadRobotCredentialsFromDirectory(hetznerSecretDirectory)
username, password, err := readRobotCredentials(hetznerSecretDirectory)
if err != nil {
return fmt.Errorf("reading robot credentials from secret: %w", err)
}
Expand All @@ -116,7 +121,17 @@ func LoadRobotCredentials(hetznerSecretDirectory string, robotClient robotclient
return nil
}

func ReadRobotCredentialsFromDirectory(hetznerSecretDirectory string) (username, password string, err error) {
func GetInitialRobotCredentials(hetznerSecretDirectory string) (username, password string, err error) {
u, p, err := readRobotCredentials(hetznerSecretDirectory)
if err != nil {
return "", "", fmt.Errorf("readRobotCredentials: %w", err)
}
oldRobotUser = u
oldRobotPassword = p
return u, p, nil
}

func readRobotCredentials(hetznerSecretDirectory string) (username, password string, err error) {
robotUserNameFile := filepath.Join(hetznerSecretDirectory, "robot-user")
robotPasswordFile := filepath.Join(hetznerSecretDirectory, "robot-password")
u, err := os.ReadFile(robotUserNameFile)
Expand All @@ -130,11 +145,11 @@ func ReadRobotCredentialsFromDirectory(hetznerSecretDirectory string) (username,
return strings.TrimSpace(string(u)), strings.TrimSpace(string(p)), nil
}

func LoadHcloudCredentials(hetznerSecretDirectory string, hcloudClient *hcloud.Client) error {
func loadHcloudCredentials(hetznerSecretDirectory string, hcloudClient *hcloud.Client) error {
hcloudMutex.Lock()
defer hcloudMutex.Unlock()
op := "hcloud/updateHcloudToken"
token, err := ReadHcloudCredentialsFromDirectory(hetznerSecretDirectory)
token, err := readHcloudCredentials(hetznerSecretDirectory)
if err != nil {
return fmt.Errorf("%s: %w", op, err)
}
Expand All @@ -151,7 +166,16 @@ func LoadHcloudCredentials(hetznerSecretDirectory string, hcloudClient *hcloud.C
return nil
}

func ReadHcloudCredentialsFromDirectory(hetznerSecretDirectory string) (string, error) {
func GetInitialHcloudCredentialsFromDirectory(hetznerSecretDirectory string) (string, error) {
token, err := readHcloudCredentials(hetznerSecretDirectory)
if err != nil {
return "", fmt.Errorf("readHcloudCredentials: %w", err)
}
oldHcloudToken = token
return token, nil
}

func readHcloudCredentials(hetznerSecretDirectory string) (string, error) {
hcloudTokenFile := filepath.Join(hetznerSecretDirectory, "hcloud")
data, err := os.ReadFile(hcloudTokenFile)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/robot/client/cache/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func NewCachedRobotClient(rootDir string, httpClient *http.Client, baseURL strin
return nil, nil
}
} else {
robotUser, robotPassword, err = hotreload.ReadRobotCredentialsFromDirectory(secretsDir)
robotUser, robotPassword, err = hotreload.GetInitialRobotCredentials(secretsDir)
if err != nil {
return nil, fmt.Errorf("%s: %w", op, err)
}
Expand Down
31 changes: 26 additions & 5 deletions internal/robot/client/cache/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http/httptest"
"os"
"path/filepath"
"sync/atomic"
"testing"
"time"

Expand All @@ -30,6 +31,12 @@ func Test_updateRobotCredentials(t *testing.T) {
err = os.MkdirAll(filepath.Join(tmp, "etc", "hetzner-secret"), 0o755)
require.NoError(t, err)

err = os.Symlink("..data/robot-user", filepath.Join(tmp, "etc", "hetzner-secret", "robot-user"))
require.NoError(t, err)

err = os.Symlink("..data/robot-password", filepath.Join(tmp, "etc", "hetzner-secret", "robot-password"))
require.NoError(t, err)

err = writeCredentials(tmp, "my-robot-user", "my-robot-password")
require.NoError(t, err)

Expand All @@ -42,7 +49,7 @@ func Test_updateRobotCredentials(t *testing.T) {
json.NewEncoder(w).Encode([]models.ServerResponse{
{
Server: models.Server{
ServerIP: "123.123.123.123",
ServerIP: "123.123.123.12",
ServerIPv6Net: "2a01:f48:111:4221::",
ServerNumber: 321,
Name: "bm-server1",
Expand All @@ -61,12 +68,15 @@ func Test_updateRobotCredentials(t *testing.T) {
require.NoError(t, err)
require.Len(t, servers, 1)

oldCount := hotreload.RobotReloadCounter
var oldCount uint64

atomic.LoadUint64(&hotreload.RobotReloadCounter)
err = writeCredentials(tmp, "user2", "password2")
require.NoError(t, err)
start := time.Now()
for {
if hotreload.RobotReloadCounter > oldCount {
// if hotreload.RobotReloadCounter > oldCount {
if atomic.LoadUint64(&hotreload.RobotReloadCounter) > oldCount {
break
}
if time.Since(start) > time.Second*3 {
Expand All @@ -82,16 +92,27 @@ func Test_updateRobotCredentials(t *testing.T) {
}

func writeCredentials(tmpDir, user, password string) error {
err := os.WriteFile(filepath.Join(tmpDir, "etc", "hetzner-secret", "robot-user"),
newDir := filepath.Join(tmpDir, "etc", "hetzner-secret", "..dataNew")
if err := os.MkdirAll(newDir, 0o700); err != nil {
return err
}
err := os.WriteFile(filepath.Join(newDir, "robot-user"),
[]byte(user), 0o600)
if err != nil {
return err
}

err = os.WriteFile(filepath.Join(tmpDir, "etc", "hetzner-secret", "robot-password"),
err = os.WriteFile(filepath.Join(newDir, "robot-password"),
[]byte(password), 0o600)
if err != nil {
return err
}
targetDir := filepath.Join(tmpDir, "etc", "hetzner-secret", "..data")
if err := os.RemoveAll(targetDir); err != nil {
return err
}
if err := os.Rename(newDir, targetDir); err != nil {
return err
}
return nil
}

0 comments on commit 5e14043

Please sign in to comment.