Skip to content

Commit a7cd691

Browse files
sbueringersbueringer
authored
✨ Optimize size of runtime hook requests (#12462)
* Optimize size of runtime hook requests * Fix review findings
1 parent ea6314d commit a7cd691

File tree

11 files changed

+428
-37
lines changed

11 files changed

+428
-37
lines changed

‎exp/topology/desiredstate/desired_state.go

Lines changed: 20 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,19 @@ 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 status, 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+
cluster.Status = clusterv1beta1.ClusterStatus{}
1497+
return cluster
1498+
}

‎exp/topology/desiredstate/desired_state_test.go

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package desiredstate
1818

1919
import (
2020
"encoding/json"
21+
"errors"
2122
"fmt"
2223
"strings"
2324
"testing"
@@ -37,6 +38,7 @@ import (
3738
ctrl "sigs.k8s.io/controller-runtime"
3839
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3940

41+
clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
4042
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
4143
runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1"
4244
runtimev1 "sigs.k8s.io/cluster-api/api/runtime/v1beta2"
@@ -50,6 +52,7 @@ import (
5052
topologynames "sigs.k8s.io/cluster-api/internal/topology/names"
5153
"sigs.k8s.io/cluster-api/internal/topology/ownerrefs"
5254
"sigs.k8s.io/cluster-api/util"
55+
"sigs.k8s.io/cluster-api/util/conversion"
5356
"sigs.k8s.io/cluster-api/util/test/builder"
5457
)
5558

@@ -1185,6 +1188,22 @@ func TestComputeControlPlaneVersion(t *testing.T) {
11851188
ObjectMeta: metav1.ObjectMeta{
11861189
Name: "test-cluster",
11871190
Namespace: "test-ns",
1191+
// Add managedFields and annotations that should be cleaned up before the Cluster is sent to the RuntimeExtension.
1192+
ManagedFields: []metav1.ManagedFieldsEntry{
1193+
{
1194+
APIVersion: builder.InfrastructureGroupVersion.String(),
1195+
Manager: "manager",
1196+
Operation: "op",
1197+
Time: ptr.To(metav1.Now()),
1198+
FieldsType: "FieldsV1",
1199+
FieldsV1: &metav1.FieldsV1{},
1200+
},
1201+
},
1202+
Annotations: map[string]string{
1203+
"fizz": "buzz",
1204+
corev1.LastAppliedConfigAnnotation: "should be cleaned up",
1205+
conversion.DataAnnotation: "should be cleaned up",
1206+
},
11881207
},
11891208
},
11901209
ControlPlane: &scope.ControlPlaneState{Object: tt.controlPlaneObj},
@@ -1207,6 +1226,7 @@ func TestComputeControlPlaneVersion(t *testing.T) {
12071226
WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{
12081227
beforeClusterUpgradeGVH: tt.hookResponse,
12091228
}).
1229+
WithCallAllExtensionValidations(validateCleanupCluster).
12101230
Build()
12111231

12121232
fakeClient := fake.NewClientBuilder().WithScheme(fakeScheme).WithObjects(s.Current.Cluster).Build()
@@ -1505,10 +1525,28 @@ func TestComputeControlPlaneVersion(t *testing.T) {
15051525
t.Run(tt.name, func(t *testing.T) {
15061526
g := NewWithT(t)
15071527

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

@@ -1520,6 +1558,11 @@ func TestComputeControlPlaneVersion(t *testing.T) {
15201558
}
15211559

15221560
_, err := r.computeControlPlaneVersion(ctx, tt.s)
1561+
if tt.wantErr {
1562+
g.Expect(err).To(HaveOccurred())
1563+
} else {
1564+
g.Expect(err).ToNot(HaveOccurred())
1565+
}
15231566

15241567
if tt.wantHookToBeCalled {
15251568
g.Expect(fakeRuntimeClient.CallAllCount(runtimehooksv1.AfterControlPlaneUpgrade)).To(Equal(1), "Expected hook to be called once")
@@ -1528,7 +1571,6 @@ func TestComputeControlPlaneVersion(t *testing.T) {
15281571
}
15291572

15301573
g.Expect(hooks.IsPending(runtimehooksv1.AfterControlPlaneUpgrade, tt.s.Current.Cluster)).To(Equal(tt.wantIntentToCall))
1531-
g.Expect(err != nil).To(Equal(tt.wantErr))
15321574
if tt.wantHookToBeCalled && !tt.wantErr {
15331575
g.Expect(tt.s.HookResponseTracker.IsBlocking(runtimehooksv1.AfterControlPlaneUpgrade)).To(Equal(tt.wantHookToBlock))
15341576
}
@@ -3498,3 +3540,26 @@ func TestCalculateRefDesiredAPIVersion(t *testing.T) {
34983540
})
34993541
}
35003542
}
3543+
3544+
func validateCleanupCluster(req runtimehooksv1.RequestObject) error {
3545+
var cluster clusterv1beta1.Cluster
3546+
switch req := req.(type) {
3547+
case *runtimehooksv1.BeforeClusterUpgradeRequest:
3548+
cluster = req.Cluster
3549+
case *runtimehooksv1.AfterControlPlaneUpgradeRequest:
3550+
cluster = req.Cluster
3551+
default:
3552+
return fmt.Errorf("unhandled request type %T", req)
3553+
}
3554+
3555+
if cluster.GetManagedFields() != nil {
3556+
return errors.New("managedFields should have been cleaned up")
3557+
}
3558+
if _, ok := cluster.Annotations[corev1.LastAppliedConfigAnnotation]; ok {
3559+
return errors.New("last-applied-configuration annotation should have been cleaned up")
3560+
}
3561+
if _, ok := cluster.Annotations[conversion.DataAnnotation]; ok {
3562+
return errors.New("conversion annotation should have been cleaned up")
3563+
}
3564+
return nil
3565+
}

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

Lines changed: 21 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,19 @@ 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 status, 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+
cluster.Status = clusterv1beta1.ClusterStatus{}
566+
return cluster
567+
}

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

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package cluster
1818

1919
import (
20+
"errors"
2021
"fmt"
2122
"testing"
2223
"time"
@@ -33,6 +34,7 @@ import (
3334
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3435
"sigs.k8s.io/controller-runtime/pkg/reconcile"
3536

37+
clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
3638
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
3739
runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1"
3840
runtimev1 "sigs.k8s.io/cluster-api/api/runtime/v1beta2"
@@ -44,6 +46,7 @@ import (
4446
"sigs.k8s.io/cluster-api/internal/hooks"
4547
fakeruntimeclient "sigs.k8s.io/cluster-api/internal/runtime/client/fake"
4648
"sigs.k8s.io/cluster-api/util/conditions"
49+
"sigs.k8s.io/cluster-api/util/conversion"
4750
"sigs.k8s.io/cluster-api/util/kubeconfig"
4851
"sigs.k8s.io/cluster-api/util/patch"
4952
"sigs.k8s.io/cluster-api/util/test/builder"
@@ -557,11 +560,29 @@ func TestClusterReconciler_reconcileDelete(t *testing.T) {
557560
t.Run(tt.name, func(t *testing.T) {
558561
g := NewWithT(t)
559562

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

@@ -712,14 +733,34 @@ func TestReconciler_callBeforeClusterCreateHook(t *testing.T) {
712733
WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{
713734
gvh: tt.hookResponse,
714735
}).
736+
WithCallAllExtensionValidations(validateCleanupCluster).
715737
Build()
716738

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

0 commit comments

Comments
 (0)