fix review findings

Signed-off-by: sivchari <shibuuuu5@gmail.com>

Author: sivchari

exp/topology/desiredstate/desired_state.go

@@ -1635,7 +1635,7 @@ func getOwnerReferenceFrom(obj, owner client.Object) *metav1.OwnerReference {
 	return nil
 }
 
-func cleanupCluster(cluster *clusterv1beta1.Cluster) *clusterv1beta1.Cluster {
+func cleanupV1Beta1Cluster(cluster *clusterv1beta1.Cluster) *clusterv1beta1.Cluster {
 	// Optimize size of Cluster by not sending status, the managedFields and some specific annotations.
 	cluster.SetManagedFields(nil)
 
@@ -1650,3 +1650,19 @@ func cleanupCluster(cluster *clusterv1beta1.Cluster) *clusterv1beta1.Cluster {
 	cluster.Status = clusterv1beta1.ClusterStatus{}
 	return cluster
 }
+
+func cleanupCluster(cluster *clusterv1.Cluster) *clusterv1.Cluster {
+	// Optimize size of Cluster by not sending status, the managedFields and some specific annotations.
+	cluster.SetManagedFields(nil)
+
+	// The conversion that we run before calling cleanupCluster does not clone annotations
+	// So we have to do it here to not modify the original Cluster.
+	if cluster.Annotations != nil {
+		annotations := maps.Clone(cluster.Annotations)
+		delete(annotations, corev1.LastAppliedConfigAnnotation)
+		delete(annotations, conversion.DataAnnotation)
+		cluster.Annotations = annotations
+	}
+	cluster.Status = clusterv1.ClusterStatus{}
+	return cluster
+}

exp/topology/desiredstate/lifecycle_hooks.go

@@ -79,7 +79,7 @@ func (g *generator) callBeforeClusterUpgradeHook(ctx context.Context, s *scope.S
 		}
 
 		hookRequest := &runtimehooksv1.BeforeClusterUpgradeRequest{
-			Cluster:               *cleanupCluster(v1beta1Cluster),
+			Cluster:               *cleanupV1Beta1Cluster(v1beta1Cluster),
 			FromKubernetesVersion: *currentVersion,
 			ToKubernetesVersion:   topologyVersion,
 			ControlPlaneUpgrades:  toUpgradeStep(s.UpgradeTracker.ControlPlane.UpgradePlan),
@@ -119,7 +119,7 @@ func (g *generator) callBeforeControlPlaneUpgradeHook(ctx context.Context, s *sc
 	}
 
 	hookRequest := &runtimehooksv1.BeforeControlPlaneUpgradeRequest{
-		Cluster:               *cleanupCluster(v1beta1Cluster),
+		Cluster:               *cleanupV1Beta1Cluster(v1beta1Cluster),
 		FromKubernetesVersion: *currentVersion,
 		ToKubernetesVersion:   nextVersion,
 		ControlPlaneUpgrades:  toUpgradeStep(s.UpgradeTracker.ControlPlane.UpgradePlan),
@@ -161,7 +161,7 @@ func (g *generator) callAfterControlPlaneUpgradeHook(ctx context.Context, s *sco
 
 		// Call all the registered extension for the hook.
 		hookRequest := &runtimehooksv1.AfterControlPlaneUpgradeRequest{
-			Cluster:              *cleanupCluster(v1beta1Cluster),
+			Cluster:              *cleanupV1Beta1Cluster(v1beta1Cluster),
 			KubernetesVersion:    *currentVersion,
 			ControlPlaneUpgrades: toUpgradeStep(s.UpgradeTracker.ControlPlane.UpgradePlan),
 			WorkersUpgrades:      toUpgradeStep(s.UpgradeTracker.MachineDeployments.UpgradePlan, s.UpgradeTracker.MachinePools.UpgradePlan),
@@ -204,7 +204,7 @@ func (g *generator) callBeforeWorkersUpgradeHook(ctx context.Context, s *scope.S
 		}
 
 		hookRequest := &runtimehooksv1.BeforeWorkersUpgradeRequest{
-			Cluster:               *cleanupCluster(v1beta1Cluster),
+			Cluster:               *cleanupV1Beta1Cluster(v1beta1Cluster),
 			FromKubernetesVersion: *currentVersion,
 			ToKubernetesVersion:   nextVersion,
 			ControlPlaneUpgrades:  toUpgradeStep(s.UpgradeTracker.ControlPlane.UpgradePlan),
@@ -251,7 +251,7 @@ func (g *generator) callAfterWorkersUpgradeHook(ctx context.Context, s *scope.Sc
 
 		// Call all the registered extension for the hook.
 		hookRequest := &runtimehooksv1.AfterWorkersUpgradeRequest{
-			Cluster:              *cleanupCluster(v1beta1Cluster),
+			Cluster:              *cleanupV1Beta1Cluster(v1beta1Cluster),
 			KubernetesVersion:    *currentVersion,
 			ControlPlaneUpgrades: toUpgradeStep(s.UpgradeTracker.ControlPlane.UpgradePlan),
 			WorkersUpgrades:      toUpgradeStep(s.UpgradeTracker.MachineDeployments.UpgradePlan, s.UpgradeTracker.MachinePools.UpgradePlan),

exp/topology/desiredstate/upgrade_plan.go

@@ -419,7 +419,7 @@ func GetUpgradePlanFromExtension(runtimeClient runtimeclient.Client, cluster *cl
 
 		// Prepare the request.
 		req := &runtimehooksv1.GenerateUpgradePlanRequest{
-			Cluster:                           *cluster,
+			Cluster:                           *cleanupCluster(cluster.DeepCopy()),
 			FromControlPlaneKubernetesVersion: currentControlPlaneVersion,
 			FromWorkersKubernetesVersion:      currentMinWorkersVersion,
 			ToKubernetesVersion:               desiredVersion,
@@ -428,11 +428,7 @@ func GetUpgradePlanFromExtension(runtimeClient runtimeclient.Client, cluster *cl
 		// Call the extension.
 		resp := &runtimehooksv1.GenerateUpgradePlanResponse{}
 		if err := runtimeClient.CallExtension(ctx, runtimehooksv1.GenerateUpgradePlan, cluster, extensionName, req, resp); err != nil {
-			return nil, nil, errors.Wrapf(err, "failed to call GenerateUpgradePlan extension %q", extensionName)
-		}
-
-		if resp.GetStatus() != runtimehooksv1.ResponseStatusSuccess {
-			return nil, nil, errors.Errorf("GenerateUpgradePlan extension %q returned failure: %s", extensionName, resp.GetMessage())
+			return nil, nil, errors.Wrap(err, "failed to get upgrade plan from extension")
 		}
 
 		// Convert UpgradeStep to string slice.

exp/topology/desiredstate/upgrade_plan_test.go

@@ -1206,7 +1206,7 @@ func TestGetUpgradePlanFromExtension_Errors(t *testing.T) {
 			currentControlPlaneVersion: "v1.31.0",
 			currentMinWorkersVersion:   "v1.31.0",
 			extensionError:             errors.New("extension call error"),
-			wantErrMessage:             "failed to call GenerateUpgradePlan extension \"test-extension\": extension call error",
+			wantErrMessage:             "failed to get upgrade plan from extension: extension call error",
 		},
 	}