Skip to content

Commit

Permalink
Merge pull request #619 from Danil-Grigorev/re-import-caching-issue
Browse files Browse the repository at this point in the history
Perform cache-less check on imported annotation for CAPI cluster
  • Loading branch information
salasberryfin authored Jul 29, 2024
2 parents 9a7fcfe + 10a4a2d commit 2ae0681
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 13 deletions.
27 changes: 20 additions & 7 deletions internal/controllers/import_controller_v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const (
// CAPIImportManagementV3Reconciler represents a reconciler for importing CAPI clusters in Rancher.
type CAPIImportManagementV3Reconciler struct {
Client client.Client
UncachedClient client.Client
RancherClient client.Client
recorder record.EventRecorder
WatchFilterValue string
Expand Down Expand Up @@ -262,16 +263,10 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context,

err := r.RancherClient.Get(ctx, client.ObjectKeyFromObject(rancherCluster), rancherCluster)
if apierrors.IsNotFound(err) {
shouldImport, err := util.ShouldAutoImport(ctx, log, r.Client, capiCluster, importLabelName)
if err != nil {
if autoImport, err := r.shouldAutoImportUncached(ctx, capiCluster); err != nil || !autoImport {
return ctrl.Result{}, err
}

if !shouldImport {
log.Info("not auto importing cluster as namespace or cluster isn't marked auto import")
return ctrl.Result{}, nil
}

newCluster := &managementv3.Cluster{
ObjectMeta: metav1.ObjectMeta{
Namespace: capiCluster.Namespace,
Expand Down Expand Up @@ -372,6 +367,24 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context,
return ctrl.Result{}, nil
}

func (r *CAPIImportManagementV3Reconciler) shouldAutoImportUncached(ctx context.Context, capiCluster *clusterv1.Cluster) (bool, error) {
log := log.FromContext(ctx)

if err := r.UncachedClient.Get(ctx, client.ObjectKeyFromObject(capiCluster), capiCluster); err != nil {
return false, client.IgnoreNotFound(err)
}

if shouldImport, err := util.ShouldAutoImport(ctx, log, r.Client, capiCluster, importLabelName); err != nil {
return false, err
} else if !shouldImport {
log.Info("not auto importing cluster as namespace or cluster isn't marked auto import")

return false, nil
}

return true, nil
}

func (r *CAPIImportManagementV3Reconciler) rancherV3ClusterToCapiCluster(ctx context.Context, clusterPredicate predicate.Funcs) handler.MapFunc {
log := log.FromContext(ctx)

Expand Down
1 change: 1 addition & 0 deletions internal/controllers/import_controller_v3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ var _ = Describe("reconcile CAPI Cluster", func() {

r = &CAPIImportManagementV3Reconciler{
Client: cl,
UncachedClient: cl,
RancherClient: cl, // rancher and rancher-turtles deployed in the same cluster
remoteClientGetter: remote.NewClusterClient,
Scheme: testEnv.GetScheme(),
Expand Down
37 changes: 33 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package main

import (
"cmp"
"context"
"flag"
"fmt"
Expand Down Expand Up @@ -190,17 +191,45 @@ func setupChecks(mgr ctrl.Manager) {
}

func setupReconcilers(ctx context.Context, mgr ctrl.Manager) {
rancherClient, err := setupRancherClient(mgr)
rancherClient, err := setupRancherClient(client.Options{Scheme: mgr.GetClient().Scheme()})
if err != nil {
setupLog.Error(err, "failed to create client")
os.Exit(1)
}

rancherClient = cmp.Or(rancherClient, mgr.GetClient())

options := client.Options{
Scheme: mgr.GetClient().Scheme(),
Cache: &client.CacheOptions{
DisableFor: []client.Object{
&clusterv1.Cluster{},
},
},
}

uncachedClient, err := setupRancherClient(options)
if err != nil {
setupLog.Error(err, "failed to create uncached rancher client")
os.Exit(1)
}

if uncachedClient == nil {
cl, err := client.New(mgr.GetConfig(), options)
if err != nil {
setupLog.Error(err, "failed to create uncached rancher client (same cluster)")
os.Exit(1)
}

uncachedClient = cl
}

if feature.Gates.Enabled(feature.ManagementV3Cluster) {
setupLog.Info("enabling CAPI cluster import controller for `management.cattle.io/v3` resources")

if err := (&controllers.CAPIImportManagementV3Reconciler{
Client: mgr.GetClient(),
UncachedClient: uncachedClient,
RancherClient: rancherClient,
WatchFilterValue: watchFilterValue,
InsecureSkipVerify: insecureSkipVerify,
Expand Down Expand Up @@ -263,7 +292,7 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) {
// setupRancherClient can either create a client for an in-cluster installation (rancher and rancher-turtles in the same cluster)
// or create a client for an out-of-cluster installation (rancher and rancher-turtles in different clusters) based on the
// existence of Rancher kubeconfig file.
func setupRancherClient(mgr ctrl.Manager) (client.Client, error) {
func setupRancherClient(options client.Options) (client.Client, error) {
if len(rancherKubeconfig) > 0 {
setupLog.Info("out-of-cluster installation of rancher-turtles", "using kubeconfig from path", rancherKubeconfig)

Expand All @@ -272,7 +301,7 @@ func setupRancherClient(mgr ctrl.Manager) (client.Client, error) {
return nil, fmt.Errorf("unable to load kubeconfig from file: %w", err)
}

rancherClient, err := client.New(restConfig, client.Options{Scheme: mgr.GetClient().Scheme()})
rancherClient, err := client.New(restConfig, options)
if err != nil {
return nil, err
}
Expand All @@ -282,7 +311,7 @@ func setupRancherClient(mgr ctrl.Manager) (client.Client, error) {

setupLog.Info("in-cluster installation of rancher-turtles")

return mgr.GetClient(), nil
return nil, nil
}

// loadConfigWithContext loads a REST Config from a path using a logic similar to the one used in controller-runtime.
Expand Down
6 changes: 4 additions & 2 deletions util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"

turtlesannotations "github.com/rancher/turtles/util/annotations"
)

// ShouldImport checks if the object has the label set to true.
Expand All @@ -49,8 +51,8 @@ func ShouldAutoImport(ctx context.Context, logger logr.Logger, cl client.Client,

// Check CAPI cluster for label first
hasLabel, autoImport := ShouldImport(capiCluster, label)
if hasLabel && autoImport {
logger.V(2).Info("Cluster contains import annotation")
if hasLabel && autoImport && !turtlesannotations.HasClusterImportAnnotation(capiCluster) {
logger.V(2).Info("Cluster contains import label and has no `imported` annotation")

return true, nil
}
Expand Down

0 comments on commit 2ae0681

Please sign in to comment.