From ad531d2e88b4c5725b55f03de875bad5984dda30 Mon Sep 17 00:00:00 2001 From: Mario Manno Date: Thu, 2 Oct 2025 14:24:22 +0200 Subject: [PATCH] Limit PerClusterState to less than 1mb --- internal/resourcestatus/resourcekey.go | 23 +++++- internal/resourcestatus/resourcekey_test.go | 89 +++++++++++++++++++++ 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/internal/resourcestatus/resourcekey.go b/internal/resourcestatus/resourcekey.go index b29a3ee093..f3b2dbd587 100644 --- a/internal/resourcestatus/resourcekey.go +++ b/internal/resourcestatus/resourcekey.go @@ -1,6 +1,9 @@ package resourcestatus import ( + "cmp" + "maps" + "slices" "sort" "strings" @@ -50,10 +53,10 @@ func clusterID(bd fleet.BundleDeployment) string { // fromResources inspects a list of BundleDeployments and returns a list of per-cluster states per resource keys. // It also returns a list of errors messages produced that may have occurred during processing func fromResources(items []fleet.BundleDeployment) resourceStatesByResourceKey { + // ensure cluster order is stable, so we do not trigger reconciles sort.Slice(items, func(i, j int) bool { return clusterID(items[i]) < clusterID(items[j]) }) - resources := make(resourceStatesByResourceKey) for _, bd := range items { for key, entry := range bundleDeploymentResources(bd) { @@ -155,8 +158,18 @@ func bundleDeploymentResources(bd fleet.BundleDeployment) map[fleet.ResourceKey] } func aggregateResourceStatesClustersMap(resourceKeyStates resourceStatesByResourceKey) []fleet.Resource { + // sort to make the result stable in case of truncation + keys := slices.SortedFunc(maps.Keys(resourceKeyStates), func(a, b fleet.ResourceKey) int { + return cmp.Compare( + (b.Kind + b.APIVersion + " " + b.Namespace + "/" + b.Name), + (a.Kind + a.APIVersion + " " + a.Namespace + "/" + a.Name), + ) + }) + + size := 0 result := make([]fleet.Resource, 0, len(resourceKeyStates)) - for resourceKey, entries := range resourceKeyStates { + for _, resourceKey := range keys { + entries := resourceKeyStates[resourceKey] resource := &fleet.Resource{ Kind: resourceKey.Kind, APIVersion: resourceKey.APIVersion, @@ -172,7 +185,10 @@ func aggregateResourceStatesClustersMap(resourceKeyStates resourceStatesByResour resource.IncompleteState = true } - appendToPerClusterState(&resource.PerClusterState, entry.state, entry.clusterID) + size += len(entry.clusterID) + if size < 1024*1024 { // 1mb + appendToPerClusterState(&resource.PerClusterState, entry.state, entry.clusterID) + } // top-level state is set from first non "Ready" per-cluster state if resource.State == "Ready" { @@ -183,6 +199,7 @@ func aggregateResourceStatesClustersMap(resourceKeyStates resourceStatesByResour result = append(result, *resource) } + // sort to make result easier to read sort.Slice(result, func(i, j int) bool { return key(result[i]) < key(result[j]) }) diff --git a/internal/resourcestatus/resourcekey_test.go b/internal/resourcestatus/resourcekey_test.go index 0c69923901..dbff697774 100644 --- a/internal/resourcestatus/resourcekey_test.go +++ b/internal/resourcestatus/resourcekey_test.go @@ -1,6 +1,9 @@ package resourcestatus import ( + "encoding/json" + "fmt" + "slices" "testing" "github.com/stretchr/testify/assert" @@ -245,6 +248,7 @@ func TestPerClusterState(t *testing.T) { }, } } + tests := []struct { name string bundleDeployments []fleet.BundleDeployment @@ -305,3 +309,88 @@ func TestPerClusterState(t *testing.T) { }) } } + +func TestPerClusterStateTruncation(t *testing.T) { + percluster := func(b, c int) fleet.BundleDeployment { + workload := fmt.Sprintf("workload%02d", b) + bd := fleet.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("bundlename%d", b), + Namespace: fmt.Sprintf("ns-cluster%d", c), + Labels: map[string]string{ + fleet.ClusterLabel: fmt.Sprintf("d0-k3k-downstream%04d-downstream%04d", c, c), + fleet.ClusterNamespaceLabel: "fleet-default", + }, + }, + Spec: fleet.BundleDeploymentSpec{ + DeploymentID: "fakeid", + }, + Status: fleet.BundleDeploymentStatus{ + AppliedDeploymentID: "fakeid", + Resources: []fleet.BundleDeploymentResource{ + {Kind: "ConfigMap", APIVersion: "v1", Namespace: workload, Name: "cm-web"}, + {Kind: "Deployment", APIVersion: "v1", Namespace: workload, Name: "web"}, + {Kind: "Service", APIVersion: "v1", Namespace: workload, Name: "web-svc"}, + }, + ModifiedStatus: []fleet.ModifiedStatus{ + {Kind: "Secret", APIVersion: "v1", Namespace: workload, Name: "cm-creds", Create: true}, + }, + NonReadyStatus: []fleet.NonReadyStatus{ + {Kind: "Deployment", APIVersion: "v1", Namespace: workload, Name: "db", Summary: summary.Summary{State: "NotReady"}}, + }, + }, + } + return bd + } + // we are not comparing the whole struct + sizeOf := func(res []fleet.Resource) int { + size := 0 + for _, r := range res { + for _, s := range r.PerClusterState.Ready { + size = size + len(s) + } + for _, s := range r.PerClusterState.NotReady { + size = size + len(s) + } + for _, s := range r.PerClusterState.Missing { + size = size + len(s) + } + } + return size + } + + n := 0 + maxBundle := 50 + maxCluster := 800 + var items = make([]fleet.BundleDeployment, maxBundle*maxCluster) + for c := range maxCluster { + for b := range maxBundle { + items[n] = percluster(b, c) + n = n + 1 + } + } + + // different order should produce the same truncation + ritems := slices.Clone(items) + slices.Reverse(ritems) + + var status fleet.GitRepoStatus + SetResources(items, &status.StatusBase) + + assert.Less(t, sizeOf(status.Resources), 1024*1024, "resources should be truncated to be less than 1MB") + + js, err := json.Marshal(status.Resources) + assert.NoError(t, err) + + // and the truncation is stable + SetResources(items, &status.StatusBase) + js2, err := json.Marshal(status.Resources) + assert.NoError(t, err) + // avoid the long diff from assert.Equal + assert.True(t, string(js) == string(js2), "truncation should produce stable json for the same input") + + SetResources(ritems, &status.StatusBase) + js2, err = json.Marshal(status.Resources) + assert.NoError(t, err) + assert.True(t, string(js) == string(js2), "truncation should produce stable json, when items are in a different order") +}