diff --git a/pkg/proxmox/cluster_client_endpoint_ownership_test.go b/pkg/proxmox/cluster_client_endpoint_ownership_test.go index f8e447668e..5ad31ac18e 100644 --- a/pkg/proxmox/cluster_client_endpoint_ownership_test.go +++ b/pkg/proxmox/cluster_client_endpoint_ownership_test.go @@ -68,7 +68,7 @@ func testClusterClientEndpointOwnershipRoutingAndTrust(t *testing.T) { } func testClusterClientEndpointOwnershipGuestAdmission(t *testing.T) { - isolateGuestAgentRegistry(t) + isolateGuestAgentCapacity(t) entered, finish := make(chan struct{}), make(chan struct{}) var firstCommand atomic.Bool var originalCommands, otherRequests atomic.Int32 diff --git a/pkg/proxmox/guest_agent_capacity_test.go b/pkg/proxmox/guest_agent_capacity_test.go index e4a90f123f..fc09a1bdc7 100644 --- a/pkg/proxmox/guest_agent_capacity_test.go +++ b/pkg/proxmox/guest_agent_capacity_test.go @@ -12,6 +12,23 @@ import ( "time" ) +// These tests are serial, as are the other guest-guard tests. Restore the +// process-wide state before the package's parallel HTTP tests can start. +func isolateGuestAgentCapacity(t *testing.T) { + t.Helper() + guestAgentGuards.Lock() + aliases, entries, next := guestAgentGuards.aliases, guestAgentGuards.entries, guestAgentGuards.nextCleanup + guestAgentGuards.aliases = make(map[string][]string) + guestAgentGuards.entries = make(map[guestAgentGuardKey]guestAgentGuardEntry) + guestAgentGuards.nextCleanup = time.Now().Add(time.Hour) + guestAgentGuards.Unlock() + t.Cleanup(func() { + guestAgentGuards.Lock() + guestAgentGuards.aliases, guestAgentGuards.entries, guestAgentGuards.nextCleanup = aliases, entries, next + guestAgentGuards.Unlock() + }) +} + // Half the protected entries are busy; half retain uncertain completion. func seedProtectedGuestAgentCapacity(count int) map[guestAgentGuardKey]guestAgentGuardEntry { protected := make(map[guestAgentGuardKey]guestAgentGuardEntry, count) @@ -44,7 +61,7 @@ func assertProtectedGuestAgentCapacity(t *testing.T, protected map[guestAgentGua func TestGuestAgentCapacityReusesExpiredAliasEntries(t *testing.T) { for _, expiredAliases := range []int{1, 2} { t.Run(fmt.Sprintf("expired-%d", expiredAliases), func(t *testing.T) { - isolateGuestAgentRegistry(t) + isolateGuestAgentCapacity(t) first, second := "https://first.capacity.invalid/api2/json", "https://second.capacity.invalid/api2/json" registerGuestAgentEndpoints("https://first.capacity.invalid", []string{"https://second.capacity.invalid"}) protected := seedProtectedGuestAgentCapacity(maxGuestAgentGuardEntries - 1 - expiredAliases) @@ -79,7 +96,7 @@ func TestGuestAgentCapacityReusesExpiredAliasEntries(t *testing.T) { func TestGuestAgentCapacityReclaimsExpiredBeforeEveryHTTPRead(t *testing.T) { for name, read := range backupAgentReads() { t.Run(name, func(t *testing.T) { - isolateGuestAgentRegistry(t) + isolateGuestAgentCapacity(t) var commands, configs atomic.Int32 var locked atomic.Bool server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -131,7 +148,7 @@ func TestGuestAgentCapacityReclaimsExpiredBeforeEveryHTTPRead(t *testing.T) { func TestGuestAgentCapacityNeverEvictsProtectedWork(t *testing.T) { for _, slotsLeft := range []int{0, 1} { t.Run(fmt.Sprintf("slots-%d", slotsLeft), func(t *testing.T) { - isolateGuestAgentRegistry(t) + isolateGuestAgentCapacity(t) var requests atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requests.Add(1) diff --git a/pkg/proxmox/guest_agent_fixture_test.go b/pkg/proxmox/guest_agent_fixture_test.go deleted file mode 100644 index 76e30cb8b1..0000000000 --- a/pkg/proxmox/guest_agent_fixture_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package proxmox - -import ( - "testing" - "time" -) - -// Serial guest-agent fixtures own one registry for their whole scenario. -// Keep admission shared across clients and aliases within that fixture, then -// restore prior state before the package's parallel HTTP tests can start. -func isolateGuestAgentRegistry(t *testing.T) { - t.Helper() - guestAgentGuards.Lock() - aliases, entries, next := guestAgentGuards.aliases, guestAgentGuards.entries, guestAgentGuards.nextCleanup - guestAgentGuards.aliases = make(map[string][]string) - guestAgentGuards.entries = make(map[guestAgentGuardKey]guestAgentGuardEntry) - guestAgentGuards.nextCleanup = time.Now().Add(time.Hour) - guestAgentGuards.Unlock() - t.Cleanup(func() { - guestAgentGuards.Lock() - guestAgentGuards.aliases, guestAgentGuards.entries, guestAgentGuards.nextCleanup = aliases, entries, next - guestAgentGuards.Unlock() - }) -} diff --git a/pkg/proxmox/guest_agent_http_status_test.go b/pkg/proxmox/guest_agent_http_status_test.go index a43ce2eb14..d7617d5420 100644 --- a/pkg/proxmox/guest_agent_http_status_test.go +++ b/pkg/proxmox/guest_agent_http_status_test.go @@ -39,7 +39,6 @@ func TestGuestAgentHTTPFailureDefersEveryRead(t *testing.T) { t.Run(tc.name, func(t *testing.T) { for name, first := range backupAgentReads() { t.Run(name, func(t *testing.T) { - isolateGuestAgentRegistry(t) var calls atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { @@ -87,7 +86,6 @@ func TestGuestAgentUnrecognisedHTTPStatusDefersEveryRead(t *testing.T) { t.Run(fmt.Sprintf("status-%d", status), func(t *testing.T) { for name, first := range backupAgentReads() { t.Run(name, func(t *testing.T) { - isolateGuestAgentRegistry(t) var commands, configs atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { @@ -131,51 +129,9 @@ func TestGuestAgentUnrecognisedHTTPStatusDefersEveryRead(t *testing.T) { } } -func TestGuestAgentHTTPFixtureDoesNotInheritPriorAdmission(t *testing.T) { - isolateGuestAgentRegistry(t) - var commands atomic.Int32 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.HasSuffix(r.URL.Path, "/config") { - fmt.Fprint(w, `{"data":{}}`) - return - } - commands.Add(1) - w.WriteHeader(http.StatusUpgradeRequired) - fmt.Fprint(w, "upstream unavailable") - })) - defer server.Close() - client := backupTestClient(t, server.URL) - // A closed fixture can leave this same origin in the process-wide guard - // until its one-minute uncertainty interval expires. Kernel port reuse must - // not turn the first request of a new HTTP case into an old cooldown. - release, err := acquireGuestAgent(client.baseURL, 105) - if err != nil { - t.Fatal(err) - } - release(true) - t.Run("new-http-fixture", func(t *testing.T) { - isolateGuestAgentRegistry(t) - var response *apiResponseError - _, err := backupTestClient(t, server.URL).GetVMAgentVersion(context.Background(), "node", 105) - if GuestAgentDeferredReason(err) != "agent-completion-unverified" || !errors.As(err, &response) || response.statusCode != http.StatusUpgradeRequired { - t.Errorf("new fixture inherited an earlier origin's admission: %v", err) - } - if _, err := backupTestClient(t, server.URL).GetVMAgentVersion(context.Background(), "node", 105); GuestAgentDeferredReason(err) != "agent-cooldown" { - t.Errorf("clients inside one fixture no longer share uncertainty: %v", err) - } - if commands.Load() != 1 { - t.Errorf("new fixture dispatched %d commands, want one", commands.Load()) - } - }) - if _, err := acquireGuestAgent(client.baseURL, 105); GuestAgentDeferredReason(err) != "agent-cooldown" { - t.Errorf("fixture isolation discarded the outer uncertainty fence: %v", err) - } -} - func TestGuestAgentUnrecognisedHTTPStatusSharesAliasesAndResumes(t *testing.T) { for _, status := range []int{409, 425, 499} { t.Run(fmt.Sprintf("status-%d", status), func(t *testing.T) { - isolateGuestAgentRegistry(t) var commands atomic.Int32 var healthy atomic.Bool handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -250,7 +206,6 @@ func TestGuestAgentUnrecognisedHTTPStatusSharesAliasesAndResumes(t *testing.T) { } func TestGuestAgentHTTPFailureDoesNotFailOver(t *testing.T) { - isolateGuestAgentRegistry(t) var calls atomic.Int32 handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { @@ -308,7 +263,6 @@ func TestGuestAgentHTTPExplicitRefusalsRemainErrors(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - isolateGuestAgentRegistry(t) var calls atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.HasSuffix(r.URL.Path, "/config") { @@ -357,7 +311,6 @@ func TestGuestAgentHTTPAmbiguousFailuresDoNotClearAdmission(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - isolateGuestAgentRegistry(t) var calls atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.HasSuffix(r.URL.Path, "/config") { @@ -394,7 +347,6 @@ func TestGuestAgentHTTPTerminalFailuresRemainCommandBound(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - isolateGuestAgentRegistry(t) var calls atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.HasSuffix(r.URL.Path, "/config") { @@ -424,7 +376,6 @@ func TestGuestAgentHTTPTerminalFailuresRemainCommandBound(t *testing.T) { } func TestGuestAgentHTTPOrdinaryRequestPreservesStatusAndError(t *testing.T) { - isolateGuestAgentRegistry(t) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(502) fmt.Fprint(w, "API error 403: permission denied") @@ -442,7 +393,6 @@ func TestGuestAgentHTTPOrdinaryRequestPreservesStatusAndError(t *testing.T) { } func TestGuestAgentHTTPUncertaintyExpiresAndKeepsOtherVMsIndependent(t *testing.T) { - isolateGuestAgentRegistry(t) var calls atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.HasSuffix(r.URL.Path, "/config") { diff --git a/pkg/proxmox/guest_agent_ipv4mapped_test.go b/pkg/proxmox/guest_agent_ipv4mapped_test.go index d760fe2631..400b05b685 100644 --- a/pkg/proxmox/guest_agent_ipv4mapped_test.go +++ b/pkg/proxmox/guest_agent_ipv4mapped_test.go @@ -89,7 +89,7 @@ func mappedEndpointReadNames() []string { func TestGuestAgentIPv4MappedInflight(t *testing.T) { for _, reverse := range []bool{false, true} { t.Run(fmt.Sprintf("mapped-first-%t", reverse), func(t *testing.T) { - isolateGuestAgentRegistry(t) + isolateGuestAgentCapacity(t) entered, finish := make(chan struct{}), make(chan struct{}) var commands, configs atomic.Int32 _, ipv4, mapped := mappedEndpointServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -165,7 +165,7 @@ func TestGuestAgentIPv4MappedUncertainty(t *testing.T) { for _, reverse := range []bool{false, true} { for _, incomplete := range []bool{false, true} { t.Run(fmt.Sprintf("mapped-first-%t/incomplete-%t", reverse, incomplete), func(t *testing.T) { - isolateGuestAgentRegistry(t) + isolateGuestAgentCapacity(t) var commands, configs atomic.Int32 _, ipv4, mapped := mappedEndpointServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { diff --git a/pkg/proxmox/guest_agent_typed_response_test.go b/pkg/proxmox/guest_agent_typed_response_test.go index 1f152a0e0b..6b992a51d5 100644 --- a/pkg/proxmox/guest_agent_typed_response_test.go +++ b/pkg/proxmox/guest_agent_typed_response_test.go @@ -88,7 +88,7 @@ func TestGuestAgentTypedSuccessDefersEveryRead(t *testing.T) { } func TestGuestAgentTypedSuccessSharesUnrenewedFenceAndResumes(t *testing.T) { - isolateGuestAgentRegistry(t) + isolateGuestAgentCapacity(t) var commands atomic.Int32 var healthy atomic.Bool handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {