From 97958d0520bc060962d39c0d92d136e06430bafe Mon Sep 17 00:00:00 2001 From: codex Date: Thu, 16 Jul 2026 01:22:07 -0300 Subject: [PATCH] backup: skip detached longhorn volumes --- internal/longhorn/client.go | 3 ++ internal/server/backup_handlers.go | 9 ++++- internal/server/backup_handlers_test.go | 18 ++++++++++ internal/server/backup_safety.go | 12 +++++++ internal/server/policy_cycle_test.go | 46 +++++++++++++++++++++++++ internal/server/policy_runtime.go | 13 +++++++ internal/server/server_test.go | 8 +++++ 7 files changed, 108 insertions(+), 1 deletion(-) diff --git a/internal/longhorn/client.go b/internal/longhorn/client.go index ba3a629..b4d1fa6 100644 --- a/internal/longhorn/client.go +++ b/internal/longhorn/client.go @@ -46,6 +46,9 @@ func (e *APIError) Error() string { type Volume struct { Name string `json:"name"` Size string `json:"size"` + State string `json:"state"` + Robustness string `json:"robustness"` + CurrentNodeID string `json:"currentNodeID"` NumberOfReplicas int `json:"numberOfReplicas"` BackupStatus []BackupStatus `json:"backupStatus"` LastBackup string `json:"lastBackup"` diff --git a/internal/server/backup_handlers.go b/internal/server/backup_handlers.go index 24ba90d..4d1cd3c 100644 --- a/internal/server/backup_handlers.go +++ b/internal/server/backup_handlers.go @@ -225,6 +225,13 @@ func (s *Server) executeBackup(ctx context.Context, req api.BackupRequest, reque if req.DryRun { return response, "dry_run", nil } + ready, reason, err := s.longhornVolumeReady(ctx, volumeName) + if err != nil { + return api.BackupResponse{}, "backend_error", err + } + if !ready { + return api.BackupResponse{}, "not_ready", errors.New(reason) + } labels := map[string]string{ "soteria.bstein.dev/namespace": req.Namespace, @@ -312,7 +319,7 @@ func backupStatusCode(result string) int { switch result { case "validation_error", "unsupported_driver": return http.StatusBadRequest - case "in_progress": + case "in_progress", "not_ready": return http.StatusConflict case "backend_error": return http.StatusBadGateway diff --git a/internal/server/backup_handlers_test.go b/internal/server/backup_handlers_test.go index 5ec1886..efb110c 100644 --- a/internal/server/backup_handlers_test.go +++ b/internal/server/backup_handlers_test.go @@ -475,6 +475,24 @@ func TestExecuteBackupAndStatusHelpers(t *testing.T) { } }) + t.Run("longhorn detached volume", func(t *testing.T) { + srv := newBackupTestServer( + &config.Config{AuthRequired: false, BackupDriver: "longhorn"}, + &backupTestKubeClient{restoreTestKubeClient: &restoreTestKubeClient{fakeKubeClient: &fakeKubeClient{}}}, + &backupTestLonghornClient{ + restoreTestLonghornClient: &restoreTestLonghornClient{fakeLonghornClient: &fakeLonghornClient{ + volumes: map[string]longhorn.Volume{ + "apps-data-pv": {Name: "apps-data-pv", State: "detached"}, + }, + }}, + }, + ) + _, result, err := srv.executeBackup(context.Background(), api.BackupRequest{Namespace: "apps", PVC: "data"}, "brad") + if result != "not_ready" || err == nil || !strings.Contains(err.Error(), "is detached") { + t.Fatalf("expected detached volume not_ready error, got result=%q err=%v", result, err) + } + }) + t.Run("restic backend error", func(t *testing.T) { srv := newBackupTestServer( &config.Config{AuthRequired: false, BackupDriver: "restic"}, diff --git a/internal/server/backup_safety.go b/internal/server/backup_safety.go index eecd1de..77fa5c8 100644 --- a/internal/server/backup_safety.go +++ b/internal/server/backup_safety.go @@ -85,6 +85,18 @@ func pvcAccessModes(pvc *corev1.PersistentVolumeClaim) []string { return modes } +func (s *Server) longhornVolumeReady(ctx context.Context, volumeName string) (bool, string, error) { + volume, err := s.longhorn.GetVolume(ctx, volumeName) + if err != nil { + return false, "", err + } + state := strings.ToLower(strings.TrimSpace(volume.State)) + if state != "" && state != "attached" { + return false, fmt.Sprintf("Longhorn volume %s is %s; backup waits until the volume is attached", volumeName, state), nil + } + return true, "", nil +} + func (s *Server) activeResticRepositories(ctx context.Context, namespaces []string) (map[string]struct{}, error) { active := map[string]struct{}{} for _, namespace := range uniqueSortedStrings(namespaces) { diff --git a/internal/server/policy_cycle_test.go b/internal/server/policy_cycle_test.go index 291c774..f51f878 100644 --- a/internal/server/policy_cycle_test.go +++ b/internal/server/policy_cycle_test.go @@ -3,12 +3,14 @@ package server import ( "context" "errors" + "strings" "testing" "time" "scm.bstein.dev/bstein/soteria/internal/api" "scm.bstein.dev/bstein/soteria/internal/config" "scm.bstein.dev/bstein/soteria/internal/k8s" + "scm.bstein.dev/bstein/soteria/internal/longhorn" ) type policyCycleTestKubeClient struct { @@ -279,3 +281,47 @@ func TestRunPolicyCycleSkipsLiveExclusivePVCs(t *testing.T) { t.Fatalf("expected one successful policy backup, got %f", got) } } + +func TestRunPolicyCycleSkipsDetachedLonghornVolumesWithoutConsumingLimit(t *testing.T) { + client := &policyCycleTestKubeClient{ + inventoryTestKubeClient: &inventoryTestKubeClient{ + fakeKubeClient: &fakeKubeClient{ + pvcs: []k8s.PVCSummary{ + {Namespace: "apps", Name: "detached", VolumeName: "vol-detached", Phase: "Bound"}, + {Namespace: "apps", Name: "ready", VolumeName: "vol-ready", Phase: "Bound"}, + }, + }, + }, + } + longhornClient := &fakeLonghornClient{ + volumes: map[string]longhorn.Volume{ + "vol-detached": {Name: "vol-detached", State: "detached"}, + "vol-ready": {Name: "vol-ready", State: "attached"}, + }, + } + srv := &Server{ + cfg: &config.Config{ + BackupDriver: "longhorn", + BackupMaxAge: 24 * time.Hour, + PolicyBackupsPerCycle: 1, + }, + client: client, + longhorn: longhornClient, + metrics: newTelemetry(), + policies: map[string]api.BackupPolicy{ + "apps__all": {ID: "apps__all", Namespace: "apps", IntervalHours: 1, Enabled: true, Dedupe: true}, + }, + } + + srv.runPolicyCycle(context.Background()) + + if longhornClient.createSnapshotName == "" || !strings.Contains(longhornClient.createSnapshotName, "apps-ready") { + t.Fatalf("expected ready volume snapshot backup after detached skip, got snapshot %q", longhornClient.createSnapshotName) + } + if got := metricCount(srv.metrics.policyBackups, map[string]string{"result": "volume_not_ready"}); got != 1 { + t.Fatalf("expected volume_not_ready metric, got %f", got) + } + if got := metricCount(srv.metrics.policyBackups, map[string]string{"result": "success"}); got != 1 { + t.Fatalf("expected one successful policy backup, got %f", got) + } +} diff --git a/internal/server/policy_runtime.go b/internal/server/policy_runtime.go index 98c47e5..a7ba619 100644 --- a/internal/server/policy_runtime.go +++ b/internal/server/policy_runtime.go @@ -131,6 +131,19 @@ func (s *Server) runPolicyCycle(ctx context.Context) { s.metrics.RecordPolicyBackup("not_due") continue } + if s.cfg.BackupDriver == "longhorn" { + ready, reason, err := s.longhornVolumeReady(runCtx, pvc.Volume) + if err != nil { + log.Printf("policy cycle Longhorn volume lookup failed for %s/%s: %v", pvc.Namespace, pvc.PVC, err) + s.metrics.RecordPolicyBackup("active_lookup_error") + continue + } + if !ready { + log.Printf("policy backup skipped for %s/%s: %s", pvc.Namespace, pvc.PVC, reason) + s.metrics.RecordPolicyBackup("volume_not_ready") + continue + } + } if s.cfg.PolicyBackupsPerCycle > 0 && started >= s.cfg.PolicyBackupsPerCycle { s.metrics.RecordPolicyBackup("cycle_limit") break diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 8fcd0b2..51bbe22 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -175,6 +175,7 @@ func (f *secretErrorKubeClient) LoadSecretData(_ context.Context, _, _, _ string type fakeLonghornClient struct { backups []longhorn.Backup + volumes map[string]longhorn.Volume createSnapshotName string snapshotBackupName string } @@ -190,6 +191,13 @@ func (f *fakeLonghornClient) SnapshotBackup(_ context.Context, volume, name stri } func (f *fakeLonghornClient) GetVolume(_ context.Context, volume string) (*longhorn.Volume, error) { + if f.volumes != nil { + item := f.volumes[volume] + if item.Name == "" { + item.Name = volume + } + return &item, nil + } return &longhorn.Volume{Name: volume, Size: "1073741824", NumberOfReplicas: 2}, nil }