fix(config): avoid deadlock saving empty nodes config
This commit is contained in:
parent
941e9ed098
commit
e9fd6c197b
2 changed files with 48 additions and 2 deletions
|
|
@ -912,8 +912,26 @@ func (c *ConfigPersistence) saveNodesConfig(pveInstances []PVEInstance, pbsInsta
|
||||||
// CRITICAL: Never save empty nodes configuration
|
// CRITICAL: Never save empty nodes configuration
|
||||||
// This prevents data loss from accidental wipes
|
// This prevents data loss from accidental wipes
|
||||||
if !allowEmpty && len(pveInstances) == 0 && len(pbsInstances) == 0 && len(pmgInstances) == 0 {
|
if !allowEmpty && len(pveInstances) == 0 && len(pbsInstances) == 0 && len(pmgInstances) == 0 {
|
||||||
// Check if we're replacing existing non-empty config
|
// If we're replacing an existing non-empty config, block the wipe.
|
||||||
if existing, err := c.LoadNodesConfig(); err == nil && existing != nil {
|
// We must not call LoadNodesConfig here because it acquires c.mu again.
|
||||||
|
if _, err := os.Stat(c.nodesFile); err == nil {
|
||||||
|
data, err := os.ReadFile(c.nodesFile)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("refusing to save empty nodes config: failed to read existing nodes config: %w", err)
|
||||||
|
}
|
||||||
|
if c.crypto != nil {
|
||||||
|
decrypted, err := c.crypto.Decrypt(data)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("refusing to save empty nodes config: existing nodes config is not decryptable: %w", err)
|
||||||
|
}
|
||||||
|
data = decrypted
|
||||||
|
}
|
||||||
|
|
||||||
|
var existing NodesConfig
|
||||||
|
if err := json.Unmarshal(data, &existing); err != nil {
|
||||||
|
return fmt.Errorf("refusing to save empty nodes config: existing nodes config is not parseable: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
if len(existing.PVEInstances) > 0 || len(existing.PBSInstances) > 0 || len(existing.PMGInstances) > 0 {
|
if len(existing.PVEInstances) > 0 || len(existing.PBSInstances) > 0 || len(existing.PMGInstances) > 0 {
|
||||||
log.Error().
|
log.Error().
|
||||||
Int("existing_pve", len(existing.PVEInstances)).
|
Int("existing_pve", len(existing.PVEInstances)).
|
||||||
|
|
|
||||||
28
internal/config/persistence_deadlock_test.go
Normal file
28
internal/config/persistence_deadlock_test.go
Normal file
|
|
@ -0,0 +1,28 @@
|
||||||
|
package config
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestSaveNodesConfig_EmptyDoesNotDeadlockWhenExistingNodesPresent(t *testing.T) {
|
||||||
|
cp := NewConfigPersistence(t.TempDir())
|
||||||
|
|
||||||
|
if err := cp.SaveNodesConfig([]PVEInstance{{Name: "pve1", Host: "https://example.invalid:8006", User: "root@pam"}}, nil, nil); err != nil {
|
||||||
|
t.Fatalf("SaveNodesConfig seed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
done := make(chan error, 1)
|
||||||
|
go func() {
|
||||||
|
done <- cp.SaveNodesConfig(nil, nil, nil)
|
||||||
|
}()
|
||||||
|
|
||||||
|
select {
|
||||||
|
case err := <-done:
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected error when saving empty over existing nodes")
|
||||||
|
}
|
||||||
|
case <-time.After(2 * time.Second):
|
||||||
|
t.Fatalf("SaveNodesConfig appears to have deadlocked")
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue