From 6c138584ed462aeca11dec53ef5caffe22376d00 Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Thu, 26 Aug 2021 07:19:29 -0700 Subject: [PATCH] :seedling: 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 --- controllers/cluster_controller.go | 3 - controllers/cluster_controller_phases.go | 2 +- controllers/machine_controller.go | 3 - controllers/machine_controller_phases.go | 2 +- controllers/machinedeployment_controller.go | 9 +-- controllers/machineset_controller.go | 13 ++--- controllers/topology/controller.go | 3 - controllers/topology/util.go | 2 +- exp/controllers/machinepool_controller.go | 3 - util/conversion/conversion.go | 65 ++++++++++++++++++--- util/util.go | 14 +++++ 11 files changed, 81 insertions(+), 38 deletions(-) diff --git a/controllers/cluster_controller.go b/controllers/cluster_controller.go index 5486ca8974cd..8f38d150af51 100644 --- a/controllers/cluster_controller.go +++ b/controllers/cluster_controller.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/controllers/external" @@ -68,7 +67,6 @@ type ClusterReconciler struct { Client client.Client WatchFilterValue string - restConfig *rest.Config recorder record.EventRecorder externalTracker external.ObjectTracker } @@ -91,7 +89,6 @@ func (r *ClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manag } r.recorder = mgr.GetEventRecorderFor("cluster-controller") - r.restConfig = mgr.GetConfig() r.externalTracker = external.ObjectTracker{ Controller: controller, } diff --git a/controllers/cluster_controller_phases.go b/controllers/cluster_controller_phases.go index 234bc854fdaa..21359c5bd22d 100644 --- 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.go b/controllers/machine_controller.go index e8e09060755a..32873ead0d8d 100644 --- a/controllers/machine_controller.go +++ b/controllers/machine_controller.go @@ -30,7 +30,6 @@ import ( kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" @@ -81,7 +80,6 @@ type MachineReconciler struct { WatchFilterValue string controller controller.Controller - restConfig *rest.Config recorder record.EventRecorder externalTracker external.ObjectTracker } @@ -116,7 +114,6 @@ func (r *MachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manag r.controller = controller r.recorder = mgr.GetEventRecorderFor("machine-controller") - r.restConfig = mgr.GetConfig() r.externalTracker = external.ObjectTracker{ Controller: controller, } diff --git a/controllers/machine_controller_phases.go b/controllers/machine_controller_phases.go index b7f71593fe0e..41ec4a1326f6 100644 --- 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/machinedeployment_controller.go b/controllers/machinedeployment_controller.go index 7bdbf7d644ef..ae36fb9ca682 100644 --- a/controllers/machinedeployment_controller.go +++ b/controllers/machinedeployment_controller.go @@ -26,7 +26,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/util" @@ -57,8 +56,7 @@ type MachineDeploymentReconciler struct { Client client.Client WatchFilterValue string - recorder record.EventRecorder - restConfig *rest.Config + recorder record.EventRecorder } func (r *MachineDeploymentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { @@ -94,7 +92,6 @@ func (r *MachineDeploymentReconciler) SetupWithManager(ctx context.Context, mgr } r.recorder = mgr.GetEventRecorderFor("machinedeployment-controller") - r.restConfig = mgr.GetConfig() return nil } @@ -202,12 +199,12 @@ func (r *MachineDeploymentReconciler) reconcile(ctx context.Context, cluster *cl } // Make sure to reconcile the external infrastructure reference. - if err := reconcileExternalTemplateReference(ctx, r.Client, r.restConfig, cluster, &d.Spec.Template.Spec.InfrastructureRef); err != nil { + if err := reconcileExternalTemplateReference(ctx, r.Client, cluster, &d.Spec.Template.Spec.InfrastructureRef); err != nil { return ctrl.Result{}, err } // Make sure to reconcile the external bootstrap reference, if any. if d.Spec.Template.Spec.Bootstrap.ConfigRef != nil { - if err := reconcileExternalTemplateReference(ctx, r.Client, r.restConfig, cluster, d.Spec.Template.Spec.Bootstrap.ConfigRef); err != nil { + if err := reconcileExternalTemplateReference(ctx, r.Client, cluster, d.Spec.Template.Spec.Bootstrap.ConfigRef); err != nil { return ctrl.Result{}, err } } diff --git a/controllers/machineset_controller.go b/controllers/machineset_controller.go index 2192878a8881..3ff17036eaff 100644 --- a/controllers/machineset_controller.go +++ b/controllers/machineset_controller.go @@ -28,7 +28,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/controllers/external" @@ -71,8 +70,7 @@ type MachineSetReconciler struct { Tracker *remote.ClusterCacheTracker WatchFilterValue string - recorder record.EventRecorder - restConfig *rest.Config + recorder record.EventRecorder } func (r *MachineSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { @@ -108,7 +106,6 @@ func (r *MachineSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Ma } r.recorder = mgr.GetEventRecorderFor("machineset-controller") - r.restConfig = mgr.GetConfig() return nil } @@ -184,12 +181,12 @@ func (r *MachineSetReconciler) reconcile(ctx context.Context, cluster *clusterv1 } // Make sure to reconcile the external infrastructure reference. - if err := reconcileExternalTemplateReference(ctx, r.Client, r.restConfig, cluster, &machineSet.Spec.Template.Spec.InfrastructureRef); err != nil { + if err := reconcileExternalTemplateReference(ctx, r.Client, cluster, &machineSet.Spec.Template.Spec.InfrastructureRef); err != nil { return ctrl.Result{}, err } // Make sure to reconcile the external bootstrap reference, if any. if machineSet.Spec.Template.Spec.Bootstrap.ConfigRef != nil { - if err := reconcileExternalTemplateReference(ctx, r.Client, r.restConfig, cluster, machineSet.Spec.Template.Spec.Bootstrap.ConfigRef); err != nil { + if err := reconcileExternalTemplateReference(ctx, r.Client, cluster, machineSet.Spec.Template.Spec.Bootstrap.ConfigRef); err != nil { return ctrl.Result{}, err } } @@ -668,12 +665,12 @@ func (r *MachineSetReconciler) getMachineNode(ctx context.Context, cluster *clus return node, nil } -func reconcileExternalTemplateReference(ctx context.Context, c client.Client, restConfig *rest.Config, cluster *clusterv1.Cluster, ref *corev1.ObjectReference) error { +func reconcileExternalTemplateReference(ctx context.Context, c client.Client, cluster *clusterv1.Cluster, ref *corev1.ObjectReference) error { if !strings.HasSuffix(ref.Kind, clusterv1.TemplateSuffix) { 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/controller.go b/controllers/topology/controller.go index 9f34529f92b3..66ddfa67f962 100644 --- a/controllers/topology/controller.go +++ b/controllers/topology/controller.go @@ -23,7 +23,6 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/rest" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/api/v1alpha4/index" "sigs.k8s.io/cluster-api/controllers/external" @@ -52,7 +51,6 @@ type ClusterReconciler struct { // thus allowing to optimize reads for templates or provider specific objects in a managed topology. UnstructuredCachingClient client.Client - restConfig *rest.Config externalTracker external.ObjectTracker } @@ -75,7 +73,6 @@ func (r *ClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manag return errors.Wrap(err, "failed setting up with a controller manager") } - r.restConfig = mgr.GetConfig() r.externalTracker = external.ObjectTracker{ Controller: c, } diff --git a/controllers/topology/util.go b/controllers/topology/util.go index f4bd676f3c22..e1a6a2ef020c 100644 --- 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/exp/controllers/machinepool_controller.go b/exp/controllers/machinepool_controller.go index 0eb45ccc1d83..a913303dc334 100644 --- a/exp/controllers/machinepool_controller.go +++ b/exp/controllers/machinepool_controller.go @@ -26,7 +26,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/controllers/external" @@ -61,7 +60,6 @@ type MachinePoolReconciler struct { Client client.Client WatchFilterValue string - config *rest.Config controller controller.Controller recorder record.EventRecorder externalWatchers sync.Map @@ -95,7 +93,6 @@ func (r *MachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M r.controller = c r.recorder = mgr.GetEventRecorderFor("machinepool-controller") - r.config = mgr.GetConfig() return nil } diff --git a/util/conversion/conversion.go b/util/conversion/conversion.go index fd332cbc1e17..42016d7dc975 100644 --- 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 index 12231fc2450c..201ccc5a3d3b 100644 --- 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 {