From 487569d8a13f2dd79948ecfdc938c5cb549178fb Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Sun, 18 Oct 2026 23:28:44 +0100 Subject: [PATCH] Keep guest HTTP tests on namespaced fixture identities PR3063 landed endpoint namespaces while PR3068 was being prepared from an older release base. Keep that existing fixture identity boundary and its uncertainty-fence regression, and remove the redundant global-registry resets and helper move added by PR3068. The Proxmox files return to the first parent of PR3068. The installer reader-readiness correction remains in place. This keeps one HTTP fixture isolation mechanism and preserves earlier uncertainty fences throughout the tests. Production guest admission and release controls are unchanged. Refs #3063, #3068 --- .../cluster_client_endpoint_ownership_test.go | 2 +- pkg/proxmox/guest_agent_capacity_test.go | 23 +++++++-- pkg/proxmox/guest_agent_fixture_test.go | 24 --------- pkg/proxmox/guest_agent_http_status_test.go | 50 ------------------- pkg/proxmox/guest_agent_ipv4mapped_test.go | 4 +- .../guest_agent_typed_response_test.go | 2 +- 6 files changed, 24 insertions(+), 81 deletions(-) delete mode 100644 pkg/proxmox/guest_agent_fixture_test.go 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) {