Skip to content

Commit 45cddad

Browse files
sbueringersbueringer
committed
Optimize size of runtime hook requests
1 parent c3e752a commit 45cddad

File tree

11 files changed

+409
-34
lines changed

11 files changed

+409
-34
lines changed

‎exp/topology/desiredstate/desired_state.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package desiredstate
2020
import (
2121
"context"
2222
"fmt"
23+
"maps"
2324
"slices"
2425
"strings"
2526
"time"
@@ -52,6 +53,7 @@ import (
5253
"sigs.k8s.io/cluster-api/internal/topology/selectors"
5354
"sigs.k8s.io/cluster-api/internal/webhooks"
5455
"sigs.k8s.io/cluster-api/util"
56+
"sigs.k8s.io/cluster-api/util/conversion"
5557
)
5658

5759
// Generator is a generator to generate the desired state.
@@ -535,7 +537,7 @@ func (g *generator) computeControlPlaneVersion(ctx context.Context, s *scope.Sco
535537

536538
// Call all the registered extension for the hook.
537539
hookRequest := &runtimehooksv1.AfterControlPlaneUpgradeRequest{
538-
Cluster: *v1beta1Cluster,
540+
Cluster: *cleanupCluster(v1beta1Cluster),
539541
KubernetesVersion: desiredVersion,
540542
}
541543
hookResponse := &runtimehooksv1.AfterControlPlaneUpgradeResponse{}
@@ -608,7 +610,7 @@ func (g *generator) computeControlPlaneVersion(ctx context.Context, s *scope.Sco
608610
}
609611

610612
hookRequest := &runtimehooksv1.BeforeClusterUpgradeRequest{
611-
Cluster: *v1beta1Cluster,
613+
Cluster: *cleanupCluster(v1beta1Cluster),
612614
FromKubernetesVersion: *currentVersion,
613615
ToKubernetesVersion: desiredVersion,
614616
}
@@ -1478,3 +1480,18 @@ func getOwnerReferenceFrom(obj, owner client.Object) *metav1.OwnerReference {
14781480
}
14791481
return nil
14801482
}
1483+
1484+
func cleanupCluster(cluster *clusterv1beta1.Cluster) *clusterv1beta1.Cluster {
1485+
// Optimize size of Cluster by not sending the managedFields and some specific annotations.
1486+
cluster.SetManagedFields(nil)
1487+
1488+
// The conversion that we run before calling cleanupCluster does not clone annotations
1489+
// So we have to do it here to not modify the original Cluster.
1490+
if cluster.Annotations != nil {
1491+
annotations := maps.Clone(cluster.Annotations)
1492+
delete(annotations, corev1.LastAppliedConfigAnnotation)
1493+
delete(annotations, conversion.DataAnnotation)
1494+
cluster.Annotations = annotations
1495+
}
1496+
return cluster
1497+
}

‎exp/topology/desiredstate/desired_state_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
ctrl "sigs.k8s.io/controller-runtime"
3838
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3939

40+
clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
4041
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
4142
runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1"
4243
runtimev1 "sigs.k8s.io/cluster-api/api/runtime/v1beta2"
@@ -50,6 +51,7 @@ import (
5051
topologynames "sigs.k8s.io/cluster-api/internal/topology/names"
5152
"sigs.k8s.io/cluster-api/internal/topology/ownerrefs"
5253
"sigs.k8s.io/cluster-api/util"
54+
"sigs.k8s.io/cluster-api/util/conversion"
5355
"sigs.k8s.io/cluster-api/util/test/builder"
5456
)
5557

@@ -1185,6 +1187,22 @@ func TestComputeControlPlaneVersion(t *testing.T) {
11851187
ObjectMeta: metav1.ObjectMeta{
11861188
Name: "test-cluster",
11871189
Namespace: "test-ns",
1190+
// Add managedFields and annotations that should be cleaned up before the Cluster is sent to the RuntimeExtension.
1191+
ManagedFields: []metav1.ManagedFieldsEntry{
1192+
{
1193+
APIVersion: builder.InfrastructureGroupVersion.String(),
1194+
Manager: "manager",
1195+
Operation: "op",
1196+
Time: ptr.To(metav1.Now()),
1197+
FieldsType: "FieldsV1",
1198+
FieldsV1: &metav1.FieldsV1{},
1199+
},
1200+
},
1201+
Annotations: map[string]string{
1202+
"fizz": "buzz",
1203+
corev1.LastAppliedConfigAnnotation: "should be cleaned up",
1204+
conversion.DataAnnotation: "should be cleaned up",
1205+
},
11881206
},
11891207
},
11901208
ControlPlane: &scope.ControlPlaneState{Object: tt.controlPlaneObj},
@@ -1207,6 +1225,7 @@ func TestComputeControlPlaneVersion(t *testing.T) {
12071225
WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{
12081226
beforeClusterUpgradeGVH: tt.hookResponse,
12091227
}).
1228+
WithCallAllExtensionValidations(validateCleanupCluster).
12101229
Build()
12111230

12121231
fakeClient := fake.NewClientBuilder().WithScheme(fakeScheme).WithObjects(s.Current.Cluster).Build()
@@ -1505,10 +1524,28 @@ func TestComputeControlPlaneVersion(t *testing.T) {
15051524
t.Run(tt.name, func(t *testing.T) {
15061525
g := NewWithT(t)
15071526

1527+
// Add managedFields and annotations that should be cleaned up before the Cluster is sent to the RuntimeExtension.
1528+
tt.s.Current.Cluster.SetManagedFields([]metav1.ManagedFieldsEntry{
1529+
{
1530+
APIVersion: builder.InfrastructureGroupVersion.String(),
1531+
Manager: "manager",
1532+
Operation: "op",
1533+
Time: ptr.To(metav1.Now()),
1534+
FieldsType: "FieldsV1",
1535+
FieldsV1: &metav1.FieldsV1{},
1536+
},
1537+
})
1538+
if tt.s.Current.Cluster.Annotations == nil {
1539+
tt.s.Current.Cluster.Annotations = map[string]string{}
1540+
}
1541+
tt.s.Current.Cluster.Annotations[corev1.LastAppliedConfigAnnotation] = "should be cleaned up"
1542+
tt.s.Current.Cluster.Annotations[conversion.DataAnnotation] = "should be cleaned up"
1543+
15081544
fakeRuntimeClient := fakeruntimeclient.NewRuntimeClientBuilder().
15091545
WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{
15101546
afterControlPlaneUpgradeGVH: tt.hookResponse,
15111547
}).
1548+
WithCallAllExtensionValidations(validateCleanupCluster).
15121549
WithCatalog(catalog).
15131550
Build()
15141551

@@ -3498,3 +3535,26 @@ func TestCalculateRefDesiredAPIVersion(t *testing.T) {
34983535
})
34993536
}
35003537
}
3538+
3539+
func validateCleanupCluster(req runtimehooksv1.RequestObject) error {
3540+
var cluster clusterv1beta1.Cluster
3541+
switch req := req.(type) {
3542+
case *runtimehooksv1.BeforeClusterUpgradeRequest:
3543+
cluster = req.Cluster
3544+
case *runtimehooksv1.AfterControlPlaneUpgradeRequest:
3545+
cluster = req.Cluster
3546+
default:
3547+
panic(fmt.Sprintf("unhandled request type %T", req))
3548+
}
3549+
3550+
if cluster.GetManagedFields() != nil {
3551+
panic("managedFields should have been cleaned up")
3552+
}
3553+
if _, ok := cluster.Annotations[corev1.LastAppliedConfigAnnotation]; ok {
3554+
panic("last-applied-configuration annotation should have been cleaned up")
3555+
}
3556+
if _, ok := cluster.Annotations[conversion.DataAnnotation]; ok {
3557+
panic("conversion annotation should have been cleaned up")
3558+
}
3559+
return nil
3560+
}

‎internal/controllers/topology/cluster/cluster_controller.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ package cluster
1919
import (
2020
"context"
2121
"fmt"
22+
"maps"
2223
"reflect"
2324
"time"
2425

2526
"github.com/go-logr/logr"
2627
"github.com/pkg/errors"
28+
corev1 "k8s.io/api/core/v1"
2729
apierrors "k8s.io/apimachinery/pkg/api/errors"
2830
"k8s.io/apimachinery/pkg/runtime"
2931
"k8s.io/apimachinery/pkg/types"
@@ -58,6 +60,7 @@ import (
5860
"sigs.k8s.io/cluster-api/util"
5961
"sigs.k8s.io/cluster-api/util/annotations"
6062
"sigs.k8s.io/cluster-api/util/conditions"
63+
"sigs.k8s.io/cluster-api/util/conversion"
6164
"sigs.k8s.io/cluster-api/util/patch"
6265
"sigs.k8s.io/cluster-api/util/predicates"
6366
)
@@ -434,7 +437,7 @@ func (r *Reconciler) callBeforeClusterCreateHook(ctx context.Context, s *scope.S
434437
}
435438

436439
hookRequest := &runtimehooksv1.BeforeClusterCreateRequest{
437-
Cluster: *v1beta1Cluster,
440+
Cluster: *cleanupCluster(v1beta1Cluster),
438441
}
439442
hookResponse := &runtimehooksv1.BeforeClusterCreateResponse{}
440443
if err := r.RuntimeClient.CallAllExtensions(ctx, runtimehooksv1.BeforeClusterCreate, s.Current.Cluster, hookRequest, hookResponse); err != nil {
@@ -527,7 +530,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
527530
}
528531

529532
hookRequest := &runtimehooksv1.BeforeClusterDeleteRequest{
530-
Cluster: *v1beta1Cluster,
533+
Cluster: *cleanupCluster(v1beta1Cluster),
531534
}
532535
hookResponse := &runtimehooksv1.BeforeClusterDeleteResponse{}
533536
if err := r.RuntimeClient.CallAllExtensions(ctx, runtimehooksv1.BeforeClusterDelete, cluster, hookRequest, hookResponse); err != nil {
@@ -546,3 +549,18 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
546549
}
547550
return ctrl.Result{}, nil
548551
}
552+
553+
func cleanupCluster(cluster *clusterv1beta1.Cluster) *clusterv1beta1.Cluster {
554+
// Optimize size of Cluster by not sending the managedFields and some specific annotations.
555+
cluster.SetManagedFields(nil)
556+
557+
// The conversion that we run before calling cleanupCluster does not clone annotations
558+
// So we have to do it here to not modify the original Cluster.
559+
if cluster.Annotations != nil {
560+
annotations := maps.Clone(cluster.Annotations)
561+
delete(annotations, corev1.LastAppliedConfigAnnotation)
562+
delete(annotations, conversion.DataAnnotation)
563+
cluster.Annotations = annotations
564+
}
565+
return cluster
566+
}

‎internal/controllers/topology/cluster/cluster_controller_test.go

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3434
"sigs.k8s.io/controller-runtime/pkg/reconcile"
3535

36+
clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
3637
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
3738
runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1"
3839
runtimev1 "sigs.k8s.io/cluster-api/api/runtime/v1beta2"
@@ -44,6 +45,7 @@ import (
4445
"sigs.k8s.io/cluster-api/internal/hooks"
4546
fakeruntimeclient "sigs.k8s.io/cluster-api/internal/runtime/client/fake"
4647
"sigs.k8s.io/cluster-api/util/conditions"
48+
"sigs.k8s.io/cluster-api/util/conversion"
4749
"sigs.k8s.io/cluster-api/util/kubeconfig"
4850
"sigs.k8s.io/cluster-api/util/patch"
4951
"sigs.k8s.io/cluster-api/util/test/builder"
@@ -557,11 +559,29 @@ func TestClusterReconciler_reconcileDelete(t *testing.T) {
557559
t.Run(tt.name, func(t *testing.T) {
558560
g := NewWithT(t)
559561

562+
// Add managedFields and annotations that should be cleaned up before the Cluster is sent to the RuntimeExtension.
563+
tt.cluster.SetManagedFields([]metav1.ManagedFieldsEntry{
564+
{
565+
APIVersion: builder.InfrastructureGroupVersion.String(),
566+
Manager: "manager",
567+
Operation: "op",
568+
Time: ptr.To(metav1.Now()),
569+
FieldsType: "FieldsV1",
570+
FieldsV1: &metav1.FieldsV1{},
571+
},
572+
})
573+
if tt.cluster.Annotations == nil {
574+
tt.cluster.Annotations = map[string]string{}
575+
}
576+
tt.cluster.Annotations[corev1.LastAppliedConfigAnnotation] = "should be cleaned up"
577+
tt.cluster.Annotations[conversion.DataAnnotation] = "should be cleaned up"
578+
560579
fakeClient := fake.NewClientBuilder().WithObjects(tt.cluster).Build()
561580
fakeRuntimeClient := fakeruntimeclient.NewRuntimeClientBuilder().
562581
WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{
563582
beforeClusterDeleteGVH: tt.hookResponse,
564583
}).
584+
WithCallAllExtensionValidations(validateCleanupCluster).
565585
WithCatalog(catalog).
566586
Build()
567587

@@ -712,14 +732,34 @@ func TestReconciler_callBeforeClusterCreateHook(t *testing.T) {
712732
WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{
713733
gvh: tt.hookResponse,
714734
}).
735+
WithCallAllExtensionValidations(validateCleanupCluster).
715736
Build()
716737

717738
r := &Reconciler{
718739
RuntimeClient: runtimeClient,
719740
}
720741
s := &scope.Scope{
721742
Current: &scope.ClusterState{
722-
Cluster: &clusterv1.Cluster{},
743+
Cluster: &clusterv1.Cluster{
744+
ObjectMeta: metav1.ObjectMeta{
745+
// Add managedFields and annotations that should be cleaned up before the Cluster is sent to the RuntimeExtension.
746+
ManagedFields: []metav1.ManagedFieldsEntry{
747+
{
748+
APIVersion: builder.InfrastructureGroupVersion.String(),
749+
Manager: "manager",
750+
Operation: "op",
751+
Time: ptr.To(metav1.Now()),
752+
FieldsType: "FieldsV1",
753+
FieldsV1: &metav1.FieldsV1{},
754+
},
755+
},
756+
Annotations: map[string]string{
757+
"fizz": "buzz",
758+
corev1.LastAppliedConfigAnnotation: "should be cleaned up",
759+
conversion.DataAnnotation: "should be cleaned up",
760+
},
761+
},
762+
},
723763
},
724764
HookResponseTracker: scope.NewHookResponseTracker(),
725765
}
@@ -1674,3 +1714,30 @@ func TestClusterClassToCluster(t *testing.T) {
16741714
})
16751715
}
16761716
}
1717+
1718+
func validateCleanupCluster(req runtimehooksv1.RequestObject) error {
1719+
var cluster clusterv1beta1.Cluster
1720+
switch req := req.(type) {
1721+
case *runtimehooksv1.BeforeClusterCreateRequest:
1722+
cluster = req.Cluster
1723+
case *runtimehooksv1.AfterControlPlaneInitializedRequest:
1724+
cluster = req.Cluster
1725+
case *runtimehooksv1.AfterClusterUpgradeRequest:
1726+
cluster = req.Cluster
1727+
case *runtimehooksv1.BeforeClusterDeleteRequest:
1728+
cluster = req.Cluster
1729+
default:
1730+
panic(fmt.Sprintf("unhandled request type %T", req))
1731+
}
1732+
1733+
if cluster.GetManagedFields() != nil {
1734+
panic("managedFields should have been cleaned up")
1735+
}
1736+
if _, ok := cluster.Annotations[corev1.LastAppliedConfigAnnotation]; ok {
1737+
panic("last-applied-configuration annotation should have been cleaned up")
1738+
}
1739+
if _, ok := cluster.Annotations[conversion.DataAnnotation]; ok {
1740+
panic("conversion annotation should have been cleaned up")
1741+
}
1742+
return nil
1743+
}

‎internal/controllers/topology/cluster/patches/template.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"strconv"
2222

2323
"github.com/pkg/errors"
24+
corev1 "k8s.io/api/core/v1"
2425
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2526
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2627
"k8s.io/apimachinery/pkg/runtime"
@@ -32,6 +33,7 @@ import (
3233

3334
runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1"
3435
"sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/variables"
36+
"sigs.k8s.io/cluster-api/util/conversion"
3537
)
3638

3739
// unstructuredDecoder is used to decode byte arrays into Unstructured objects.
@@ -71,7 +73,7 @@ func (t *requestItemBuilder) WithHolder(object client.Object, gvk schema.GroupVe
7173
}
7274

7375
// uuidGenerator is defined as a package variable to enable changing it during testing.
74-
var uuidGenerator func() types.UID = uuid.NewUUID
76+
var uuidGenerator = uuid.NewUUID
7577

7678
// Build builds a GeneratePatchesRequestItem.
7779
func (t *requestItemBuilder) Build() (*runtimehooksv1.GeneratePatchesRequestItem, error) {
@@ -80,6 +82,8 @@ func (t *requestItemBuilder) Build() (*runtimehooksv1.GeneratePatchesRequestItem
8082
UID: uuidGenerator(),
8183
}
8284

85+
cleanupUnstructured(t.template)
86+
8387
jsonObj, err := json.Marshal(t.template)
8488
if err != nil {
8589
return nil, errors.Wrap(err, "failed to marshal template to JSON")
@@ -93,6 +97,16 @@ func (t *requestItemBuilder) Build() (*runtimehooksv1.GeneratePatchesRequestItem
9397
return tpl, nil
9498
}
9599

100+
func cleanupUnstructured(template *unstructured.Unstructured) {
101+
// Optimize size of GeneratePatchesRequest and ValidateTopologyRequest by not sending managedFields and some specific annotations.
102+
template.SetManagedFields(nil)
103+
if annotations := template.GetAnnotations(); annotations != nil {
104+
delete(annotations, corev1.LastAppliedConfigAnnotation)
105+
delete(annotations, conversion.DataAnnotation)
106+
template.SetAnnotations(annotations)
107+
}
108+
}
109+
96110
// getTemplateAsUnstructured is a utility func that returns a template matching the holderKind, holderFieldPath
97111
// and topologyNames from a GeneratePatchesRequest.
98112
func getTemplateAsUnstructured(req *runtimehooksv1.GeneratePatchesRequest, holderKind, holderFieldPath string, topologyNames requestTopologyName) (*unstructured.Unstructured, error) {

0 commit comments

Comments
 (0)