🌱 Conversion of references should use a CR client

This changeset introduces some deprecations for functions that are still creating the metadata client through a restconfig each time the function is called. The controller runtime client supports retrieving metadata only objects; this change deprecates the old functions and creates new ones that use a CR client. Signed-off-by: Vince Prignano <[email protected]>
kubernetes-sigs · Aug 26, 2021 · 2f42881 · 2f42881
1 parent 103997d
commit 2f42881
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 13 deletions.
diff --git a/controllers/cluster_controller_phases.go b/controllers/cluster_controller_phases.go
@@ -66,7 +66,7 @@ func (r *ClusterReconciler) reconcilePhase(_ context.Context, cluster *clusterv1
 func (r *ClusterReconciler) reconcileExternal(ctx context.Context, cluster *clusterv1.Cluster, ref *corev1.ObjectReference) (external.ReconcileOutput, error) {
 	log := ctrl.LoggerFrom(ctx)
 
-	if err := utilconversion.ConvertReferenceAPIContract(ctx, r.Client, r.restConfig, ref); err != nil {
+	if err := utilconversion.UpdateReferenceAPIContract(ctx, r.Client, ref); err != nil {
 		return external.ReconcileOutput{}, err
 	}
 

diff --git a/controllers/machine_controller_phases.go b/controllers/machine_controller_phases.go
@@ -89,7 +89,7 @@ func (r *MachineReconciler) reconcilePhase(_ context.Context, m *clusterv1.Machi
 func (r *MachineReconciler) reconcileExternal(ctx context.Context, cluster *clusterv1.Cluster, m *clusterv1.Machine, ref *corev1.ObjectReference) (external.ReconcileOutput, error) {
 	log := ctrl.LoggerFrom(ctx, "cluster", cluster.Name)
 
-	if err := utilconversion.ConvertReferenceAPIContract(ctx, r.Client, r.restConfig, ref); err != nil {
+	if err := utilconversion.UpdateReferenceAPIContract(ctx, r.Client, ref); err != nil {
 		return external.ReconcileOutput{}, err
 	}
 

diff --git a/controllers/machineset_controller.go b/controllers/machineset_controller.go
@@ -671,7 +671,7 @@ func reconcileExternalTemplateReference(ctx context.Context, c client.Client, re
 		return nil
 	}
 
-	if err := utilconversion.ConvertReferenceAPIContract(ctx, c, restConfig, ref); err != nil {
+	if err := utilconversion.UpdateReferenceAPIContract(ctx, c, ref); err != nil {
 		return err
 	}
 

diff --git a/controllers/topology/util.go b/controllers/topology/util.go
@@ -48,7 +48,7 @@ func (r *ClusterReconciler) getReference(ctx context.Context, ref *corev1.Object
 	if ref == nil {
 		return nil, errors.New("reference is not set")
 	}
-	if err := utilconversion.ConvertReferenceAPIContract(ctx, r.Client, r.restConfig, ref); err != nil {
+	if err := utilconversion.UpdateReferenceAPIContract(ctx, r.Client, ref); err != nil {
 		return nil, err
 	}
 

diff --git a/util/conversion/conversion.go b/util/conversion/conversion.go
@@ -58,6 +58,44 @@ var (
 // the Custom Resource Definition and looks which one is the stored version available.
 //
 // The object passed as input is modified in place if an updated compatible version is found.
+func UpdateReferenceAPIContract(ctx context.Context, c client.Client, ref *corev1.ObjectReference) error {
+	log := ctrl.LoggerFrom(ctx)
+	gvk := ref.GroupVersionKind()
+
+	metadata, err := util.GetGVKMetadata(ctx, c, gvk)
+	if err != nil {
+		log.Info("Cannot retrieve CRD with metadata only client, falling back to slower listing", "err", err.Error())
+		// Fallback to slower and more memory intensive method to get the full CRD.
+		crd, err := util.GetCRDWithContract(ctx, c, gvk, contract)
+		if err != nil {
+			return err
+		}
+		metadata = &metav1.PartialObjectMetadata{
+			TypeMeta:   crd.TypeMeta,
+			ObjectMeta: crd.ObjectMeta,
+		}
+	}
+
+	chosen, err := getLatestAPIVersionFromContract(metadata)
+	if err != nil {
+		return err
+	}
+
+	// Modify the GroupVersionKind with the new version.
+	if gvk.Version != chosen {
+		gvk.Version = chosen
+		ref.SetGroupVersionKind(gvk)
+	}
+
+	return nil
+}
+
+// ConvertReferenceAPIContract takes a client and object reference, queries the API Server for
+// the Custom Resource Definition and looks which one is the stored version available.
+//
+// The object passed as input is modified in place if an updated compatible version is found.
+//
+// Deprecated: Use UpdateReferenceAPIContract instead.
 func ConvertReferenceAPIContract(ctx context.Context, c client.Client, restConfig *rest.Config, ref *corev1.ObjectReference) error {
 	log := ctrl.LoggerFrom(ctx)
 	gvk := ref.GroupVersionKind()
@@ -76,17 +114,11 @@ func ConvertReferenceAPIContract(ctx context.Context, c client.Client, restConfi
 		}
 	}
 
-	// If there is no label, return early without changing the reference.
-	supportedVersions, ok := metadata.Labels[contract]
-	if !ok || supportedVersions == "" {
-		return errors.Errorf("cannot find any versions matching contract %q for CRD %v as contract version label(s) are either missing or empty", contract, metadata.Name)
+	chosen, err := getLatestAPIVersionFromContract(metadata)
+	if err != nil {
+		return err
 	}
 
-	// Pick the latest version in the slice and validate it.
-	kubeVersions := util.KubeAwareAPIVersions(strings.Split(supportedVersions, "_"))
-	sort.Sort(kubeVersions)
-	chosen := kubeVersions[len(kubeVersions)-1]
-
 	// Modify the GroupVersionKind with the new version.
 	if gvk.Version != chosen {
 		gvk.Version = chosen
@@ -96,6 +128,21 @@ func ConvertReferenceAPIContract(ctx context.Context, c client.Client, restConfi
 	return nil
 }
 
+func getLatestAPIVersionFromContract(metadata metav1.Object) (string, error) {
+	labels := metadata.GetLabels()
+
+	// If there is no label, return early without changing the reference.
+	supportedVersions, ok := labels[contract]
+	if !ok || supportedVersions == "" {
+		return "", errors.Errorf("cannot find any versions matching contract %q for GVK %v as contract version label(s) are either missing or empty", contract, metadata.GetName())
+	}
+
+	// Pick the latest version in the slice and validate it.
+	kubeVersions := util.KubeAwareAPIVersions(strings.Split(supportedVersions, "_"))
+	sort.Sort(kubeVersions)
+	return kubeVersions[len(kubeVersions)-1], nil
+}
+
 // MarshalData stores the source object as json data in the destination object annotations map.
 // It ignores the metadata of the source object.
 func MarshalData(src metav1.Object, dst metav1.Object) error {

diff --git a/util/util.go b/util/util.go
@@ -416,6 +416,18 @@ func HasOwner(refList []metav1.OwnerReference, apiVersion string, kinds []string
 	return false
 }
 
+// GetGVKMetadata retrieves a CustomResourceDefinition metadata from the API server using partial object metadata.
+//
+// This function is greatly more efficient than GetCRDWithContract and should be preferred in most cases.
+func GetGVKMetadata(ctx context.Context, c client.Client, gvk schema.GroupVersionKind) (*metav1.PartialObjectMetadata, error) {
+	meta := &metav1.PartialObjectMetadata{}
+	meta.SetName(fmt.Sprintf("%s.%s", flect.Pluralize(strings.ToLower(gvk.Kind)), gvk.Group))
+	if err := c.Get(ctx, client.ObjectKeyFromObject(meta), meta); err != nil {
+		return meta, errors.Wrap(err, "failed to retrieve metadata from GVK resource")
+	}
+	return meta, nil
+}
+
 // GetCRDWithContract retrieves a list of CustomResourceDefinitions from using controller-runtime Client,
 // filtering with the `contract` label passed in.
 // Returns the first CRD in the list that matches the GroupVersionKind, otherwise returns an error.
@@ -445,6 +457,8 @@ func GetCRDWithContract(ctx context.Context, c client.Client, gvk schema.GroupVe
 // GetCRDMetadataFromGVK retrieves a CustomResourceDefinition metadata from the API server using client-go's metadata only client.
 //
 // This function is greatly more efficient than GetCRDWithContract and should be preferred in most cases.
+//
+// Deprecated: Use GetGVKMetadata instead.
 func GetCRDMetadataFromGVK(ctx context.Context, restConfig *rest.Config, gvk schema.GroupVersionKind) (*metav1.PartialObjectMetadata, error) {
 	// Make sure a rest config is available.
 	if restConfig == nil {