Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Conversion of references should use a CR client #5160

Merged
merged 1 commit into from
Aug 26, 2021
Merged

🌱 Conversion of references should use a CR client #5160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions controllers/cluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -68,7 +67,6 @@ type ClusterReconciler struct {
Client client.Client
WatchFilterValue string

restConfig *rest.Config
recorder record.EventRecorder
externalTracker external.ObjectTracker
}
Expand All @@ -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,
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/cluster_controller_phases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
3 changes: 0 additions & 3 deletions controllers/machine_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -81,7 +80,6 @@ type MachineReconciler struct {
WatchFilterValue string

controller controller.Controller
restConfig *rest.Config
recorder record.EventRecorder
externalTracker external.ObjectTracker
}
Expand Down Expand Up @@ -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,
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/machine_controller_phases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
9 changes: 3 additions & 6 deletions controllers/machinedeployment_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -94,7 +92,6 @@ func (r *MachineDeploymentReconciler) SetupWithManager(ctx context.Context, mgr
}

r.recorder = mgr.GetEventRecorderFor("machinedeployment-controller")
r.restConfig = mgr.GetConfig()
return nil
}

Expand Down Expand Up @@ -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
}
}
Expand Down
13 changes: 5 additions & 8 deletions controllers/machineset_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}

Expand Down
3 changes: 0 additions & 3 deletions controllers/topology/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand All @@ -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,
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/topology/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
3 changes: 0 additions & 3 deletions exp/controllers/machinepool_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -61,7 +60,6 @@ type MachinePoolReconciler struct {
Client client.Client
WatchFilterValue string

config *rest.Config
controller controller.Controller
recorder record.EventRecorder
externalWatchers sync.Map
Expand Down Expand Up @@ -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
}

Expand Down
65 changes: 56 additions & 9 deletions util/conversion/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down