Skip to content

Commit

Permalink
Cleanup separate unstructuredCachingClient
Browse files Browse the repository at this point in the history
Signed-off-by: Stefan Büringer buringerst@vmware.com
  • Loading branch information
sbueringer committed May 28, 2024
1 parent 2568aa2 commit 4714c48
Show file tree
Hide file tree
Showing 41 changed files with 193 additions and 371 deletions.
2 changes: 2 additions & 0 deletions bootstrap/kubeadm/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ func main() {
&corev1.ConfigMap{},
&corev1.Secret{},
},
// Use the cache for all Unstructured get/list calls.
Unstructured: true,
},
},
WebhookServer: webhook.NewServer(
Expand Down
8 changes: 3 additions & 5 deletions cmd/clusterctl/client/cluster/topology.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,8 @@ func (t *topologyClient) Plan(ctx context.Context, in *TopologyPlanInput) (*Topo

res.ReconciledCluster = targetCluster
reconciler := &clustertopologycontroller.Reconciler{
Client: dryRunClient,
APIReader: dryRunClient,
UnstructuredCachingClient: dryRunClient,
Client: dryRunClient,
APIReader: dryRunClient,
}
reconciler.SetupForDryRun(&noOpRecorder{})
request := reconcile.Request{NamespacedName: *targetCluster}
Expand Down Expand Up @@ -515,8 +514,7 @@ func reconcileClusterClass(ctx context.Context, apiReader client.Reader, class c
reconcilerClient := dryrun.NewClient(apiReader, reconciliationObjects)

clusterClassReconciler := &clusterclasscontroller.Reconciler{
Client: reconcilerClient,
UnstructuredCachingClient: reconcilerClient,
Client: reconcilerClient,
}

if _, err := clusterClassReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: targetClusterClass}); err != nil {
Expand Down
76 changes: 29 additions & 47 deletions controllers/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,26 @@ import (

// ClusterReconciler reconciles a Cluster object.
type ClusterReconciler struct {
Client client.Client
UnstructuredCachingClient client.Client
APIReader client.Reader
Client client.Client
APIReader client.Reader

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
}

func (r *ClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&clustercontroller.Reconciler{
Client: r.Client,
UnstructuredCachingClient: r.UnstructuredCachingClient,
APIReader: r.APIReader,
WatchFilterValue: r.WatchFilterValue,
Client: r.Client,
APIReader: r.APIReader,
WatchFilterValue: r.WatchFilterValue,
}).SetupWithManager(ctx, mgr, options)
}

// MachineReconciler reconciles a Machine object.
type MachineReconciler struct {
Client client.Client
UnstructuredCachingClient client.Client
APIReader client.Reader
Tracker *remote.ClusterCacheTracker
Client client.Client
APIReader client.Reader
Tracker *remote.ClusterCacheTracker

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Expand All @@ -75,21 +72,19 @@ type MachineReconciler struct {

func (r *MachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&machinecontroller.Reconciler{
Client: r.Client,
UnstructuredCachingClient: r.UnstructuredCachingClient,
APIReader: r.APIReader,
Tracker: r.Tracker,
WatchFilterValue: r.WatchFilterValue,
NodeDrainClientTimeout: r.NodeDrainClientTimeout,
Client: r.Client,
APIReader: r.APIReader,
Tracker: r.Tracker,
WatchFilterValue: r.WatchFilterValue,
NodeDrainClientTimeout: r.NodeDrainClientTimeout,
}).SetupWithManager(ctx, mgr, options)
}

// MachineSetReconciler reconciles a MachineSet object.
type MachineSetReconciler struct {
Client client.Client
UnstructuredCachingClient client.Client
APIReader client.Reader
Tracker *remote.ClusterCacheTracker
Client client.Client
APIReader client.Reader
Tracker *remote.ClusterCacheTracker

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Expand All @@ -101,7 +96,6 @@ type MachineSetReconciler struct {
func (r *MachineSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&machinesetcontroller.Reconciler{
Client: r.Client,
UnstructuredCachingClient: r.UnstructuredCachingClient,
APIReader: r.APIReader,
Tracker: r.Tracker,
WatchFilterValue: r.WatchFilterValue,
Expand All @@ -111,20 +105,18 @@ func (r *MachineSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Ma

// MachineDeploymentReconciler reconciles a MachineDeployment object.
type MachineDeploymentReconciler struct {
Client client.Client
UnstructuredCachingClient client.Client
APIReader client.Reader
Client client.Client
APIReader client.Reader

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
}

func (r *MachineDeploymentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&machinedeploymentcontroller.Reconciler{
Client: r.Client,
UnstructuredCachingClient: r.UnstructuredCachingClient,
APIReader: r.APIReader,
WatchFilterValue: r.WatchFilterValue,
Client: r.Client,
APIReader: r.APIReader,
WatchFilterValue: r.WatchFilterValue,
}).SetupWithManager(ctx, mgr, options)
}

Expand Down Expand Up @@ -157,20 +149,15 @@ type ClusterTopologyReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string

// UnstructuredCachingClient provides a client that forces caching of unstructured objects,
// thus allowing to optimize reads for templates or provider specific objects in a managed topology.
UnstructuredCachingClient client.Client
}

func (r *ClusterTopologyReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&clustertopologycontroller.Reconciler{
Client: r.Client,
APIReader: r.APIReader,
Tracker: r.Tracker,
RuntimeClient: r.RuntimeClient,
UnstructuredCachingClient: r.UnstructuredCachingClient,
WatchFilterValue: r.WatchFilterValue,
Client: r.Client,
APIReader: r.APIReader,
Tracker: r.Tracker,
RuntimeClient: r.RuntimeClient,
WatchFilterValue: r.WatchFilterValue,
}).SetupWithManager(ctx, mgr, options)
}

Expand Down Expand Up @@ -227,18 +214,13 @@ type ClusterClassReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string

// UnstructuredCachingClient provides a client that forces caching of unstructured objects,
// thus allowing to optimize reads for templates or provider specific objects.
UnstructuredCachingClient client.Client
}

func (r *ClusterClassReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
r.internalReconciler = &clusterclasscontroller.Reconciler{
Client: r.Client,
RuntimeClient: r.RuntimeClient,
UnstructuredCachingClient: r.UnstructuredCachingClient,
WatchFilterValue: r.WatchFilterValue,
Client: r.Client,
RuntimeClient: r.RuntimeClient,
WatchFilterValue: r.WatchFilterValue,
}
return r.internalReconciler.SetupWithManager(ctx, mgr, options)
}
Expand Down
2 changes: 1 addition & 1 deletion controlplane/kubeadm/internal/controllers/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ func TestCloneConfigsAndGenerateMachine(t *testing.T) {
},
},
}
g.Expect(env.Create(ctx, genericInfrastructureMachineTemplate)).To(Succeed())
g.Expect(env.CreateAndWait(ctx, genericInfrastructureMachineTemplate)).To(Succeed())

kcp := &controlplanev1.KubeadmControlPlane{
ObjectMeta: metav1.ObjectMeta{
Expand Down
2 changes: 1 addition & 1 deletion controlplane/kubeadm/internal/controllers/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestKubeadmControlPlaneReconciler_RolloutStrategy_ScaleUp(t *testing.T) {
timeout := 30 * time.Second

cluster, kcp, genericInfrastructureMachineTemplate := createClusterWithControlPlane(namespace.Name)
g.Expect(env.Create(ctx, genericInfrastructureMachineTemplate, client.FieldOwner("manager"))).To(Succeed())
g.Expect(env.CreateAndWait(ctx, genericInfrastructureMachineTemplate, client.FieldOwner("manager"))).To(Succeed())
cluster.UID = types.UID(util.RandomString(10))
cluster.Spec.ControlPlaneEndpoint.Host = Host
cluster.Spec.ControlPlaneEndpoint.Port = 6443
Expand Down
2 changes: 1 addition & 1 deletion controlplane/kubeadm/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func main() {
&corev1.ConfigMap{},
&corev1.Secret{},
},
// This config ensures that the default client caches Unstructured objects.
// This config ensures that the default client uses the cache for all Unstructured get/list calls.
// KCP is only using Unstructured to retrieve InfraMachines and InfraMachineTemplates.
// As the cache should be used in those cases, caching is configured globally instead of
// creating a separate client that caches Unstructured.
Expand Down
1 change: 0 additions & 1 deletion docs/proposals/20220221-runtime-SDK.md
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,6 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) {
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
RuntimeClient: runtimeClient,
UnstructuredCachingClient: unstructuredCachingClient,
WatchFilterValue: watchFilterValue,
}).SetupWithManager(ctx, mgr, concurrency(clusterTopologyConcurrency)); err != nil {
setupLog.Error(err, "Unable to create controller", "controller", "ClusterTopology")
Expand Down
9 changes: 4 additions & 5 deletions internal/controllers/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,8 @@ const (

// Reconciler reconciles a Cluster object.
type Reconciler struct {
Client client.Client
UnstructuredCachingClient client.Client
APIReader client.Reader
Client client.Client
APIReader client.Reader

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Expand Down Expand Up @@ -279,7 +278,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
}

if cluster.Spec.ControlPlaneRef != nil {
obj, err := external.Get(ctx, r.UnstructuredCachingClient, cluster.Spec.ControlPlaneRef, cluster.Namespace)
obj, err := external.Get(ctx, r.Client, cluster.Spec.ControlPlaneRef, cluster.Namespace)
switch {
case apierrors.IsNotFound(errors.Cause(err)):
// All good - the control plane resource has been deleted
Expand Down Expand Up @@ -310,7 +309,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
}

if cluster.Spec.InfrastructureRef != nil {
obj, err := external.Get(ctx, r.UnstructuredCachingClient, cluster.Spec.InfrastructureRef, cluster.Namespace)
obj, err := external.Get(ctx, r.Client, cluster.Spec.InfrastructureRef, cluster.Namespace)
switch {
case apierrors.IsNotFound(errors.Cause(err)):
// All good - the infra resource has been deleted
Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/cluster/cluster_controller_phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (r *Reconciler) reconcileExternal(ctx context.Context, cluster *clusterv1.C
return external.ReconcileOutput{}, err
}

obj, err := external.Get(ctx, r.UnstructuredCachingClient, ref, cluster.Namespace)
obj, err := external.Get(ctx, r.Client, ref, cluster.Namespace)
if err != nil {
if apierrors.IsNotFound(errors.Cause(err)) {
log.Info("Could not find external object for cluster, requeuing", "refGroupVersionKind", ref.GroupVersionKind(), "refName", ref.Name)
Expand Down
20 changes: 8 additions & 12 deletions internal/controllers/cluster/cluster_controller_phases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,8 @@ func TestClusterReconcilePhases(t *testing.T) {
Build()
}
r := &Reconciler{
Client: c,
UnstructuredCachingClient: c,
recorder: record.NewFakeRecorder(32),
Client: c,
recorder: record.NewFakeRecorder(32),
}

res, err := r.reconcileInfrastructure(ctx, tt.cluster)
Expand Down Expand Up @@ -218,9 +217,8 @@ func TestClusterReconcilePhases(t *testing.T) {
Build()
}
r := &Reconciler{
Client: c,
UnstructuredCachingClient: c,
recorder: record.NewFakeRecorder(32),
Client: c,
recorder: record.NewFakeRecorder(32),
}
res, err := r.reconcileKubeconfig(ctx, tt.cluster)
if tt.wantErr {
Expand Down Expand Up @@ -370,9 +368,8 @@ func TestClusterReconciler_reconcilePhase(t *testing.T) {
Build()

r := &Reconciler{
Client: c,
UnstructuredCachingClient: c,
recorder: record.NewFakeRecorder(32),
Client: c,
recorder: record.NewFakeRecorder(32),
}
r.reconcilePhase(ctx, tt.cluster)
g.Expect(tt.cluster.Status.GetTypedPhase()).To(Equal(tt.wantPhase))
Expand Down Expand Up @@ -488,9 +485,8 @@ func TestClusterReconcilePhases_reconcileFailureDomains(t *testing.T) {

c := fake.NewClientBuilder().WithObjects(objs...).Build()
r := &Reconciler{
Client: c,
UnstructuredCachingClient: c,
recorder: record.NewFakeRecorder(32),
Client: c,
recorder: record.NewFakeRecorder(32),
}

_, err := r.reconcileInfrastructure(ctx, tt.cluster)
Expand Down
8 changes: 3 additions & 5 deletions internal/controllers/cluster/cluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,8 @@ func TestClusterReconciler_reconcileDelete(t *testing.T) {
g := NewWithT(t)
fakeClient := fake.NewClientBuilder().WithObjects(fakeInfraCluster, tt.cluster).Build()
r := &Reconciler{
Client: fakeClient,
UnstructuredCachingClient: fakeClient,
APIReader: fakeClient,
Client: fakeClient,
APIReader: fakeClient,
}

_, _ = r.reconcileDelete(ctx, tt.cluster)
Expand Down Expand Up @@ -527,8 +526,7 @@ func TestClusterReconcilerNodeRef(t *testing.T) {

c := fake.NewClientBuilder().WithObjects(cluster, controlPlaneWithNoderef, controlPlaneWithoutNoderef, nonControlPlaneWithNoderef, nonControlPlaneWithoutNoderef).Build()
r := &Reconciler{
Client: c,
UnstructuredCachingClient: c,
Client: c,
}
requests := r.controlPlaneMachineToCluster(ctx, tt.o)
g.Expect(requests).To(BeComparableTo(tt.want))
Expand Down
24 changes: 5 additions & 19 deletions internal/controllers/cluster/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
Expand Down Expand Up @@ -81,29 +80,16 @@ func TestMain(m *testing.M) {
panic(fmt.Sprintf("Failed to start ClusterCacheReconciler: %v", err))
}

unstructuredCachingClient, err := client.New(mgr.GetConfig(), client.Options{
HTTPClient: mgr.GetHTTPClient(),
Cache: &client.CacheOptions{
Reader: mgr.GetCache(),
Unstructured: true,
},
})
if err != nil {
panic(fmt.Sprintf("unable to create unstructuredCachineClient: %v", err))
}

if err := (&Reconciler{
Client: mgr.GetClient(),
UnstructuredCachingClient: unstructuredCachingClient,
APIReader: mgr.GetClient(),
Client: mgr.GetClient(),
APIReader: mgr.GetClient(),
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
panic(fmt.Sprintf("Failed to start ClusterReconciler: %v", err))
}
if err := (&machinecontroller.Reconciler{
Client: mgr.GetClient(),
UnstructuredCachingClient: unstructuredCachingClient,
APIReader: mgr.GetAPIReader(),
Tracker: tracker,
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
Tracker: tracker,
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
panic(fmt.Sprintf("Failed to start MachineReconciler: %v", err))
}
Expand Down
6 changes: 1 addition & 5 deletions internal/controllers/clusterclass/clusterclass_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ type Reconciler struct {

// RuntimeClient is a client for calling runtime extensions.
RuntimeClient runtimeclient.Client

// UnstructuredCachingClient provides a client that forces caching of unstructured objects,
// thus allowing to optimize reads for templates or provider specific objects.
UnstructuredCachingClient client.Client
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
Expand Down Expand Up @@ -358,7 +354,7 @@ func refString(ref *corev1.ObjectReference) string {
func (r *Reconciler) reconcileExternal(ctx context.Context, clusterClass *clusterv1.ClusterClass, ref *corev1.ObjectReference) error {
log := ctrl.LoggerFrom(ctx)

obj, err := external.Get(ctx, r.UnstructuredCachingClient, ref, clusterClass.Namespace)
obj, err := external.Get(ctx, r.Client, ref, clusterClass.Namespace)
if err != nil {
if apierrors.IsNotFound(errors.Cause(err)) {
return errors.Wrapf(err, "Could not find external object for the ClusterClass. refGroupVersionKind: %s, refName: %s", ref.GroupVersionKind(), ref.Name)
Expand Down
Loading

0 comments on commit 4714c48

Please sign in to comment.