Skip to content

Commit

Permalink
Fix linting errors (rancher#2574)
Browse files Browse the repository at this point in the history 
* Refactor cluster import

This reduces complexity of kubeconfig computation to eliminate linter
errors.

* Refactor bundle deployment creation into separate method

This reduces the complexity of reconcile loops for bundles to eliminate
linter errors, while improving readability.
  • Loading branch information
@weyfonk
weyfonk authored Jul 3, 2024
1 parent 6404451 commit c8f246b
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 65 deletions.
26 changes: 11 additions & 15 deletions internal/cmd/controller/agentmanagement/controllers/cluster/import.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,19 +245,7 @@ func (i *importHandler) importCluster(cluster *fleet.Cluster, status fleet.Clust
apiServerCA = cfg.APIServerCA
}

if cfg.AgentTLSMode != config.AgentTLSModeStrict && cfg.AgentTLSMode != config.AgentTLSModeSystemStore {
return status,
fmt.Errorf(
"provided config value for agentTLSMode is none of [%q,%q]",
config.AgentTLSModeStrict,
config.AgentTLSModeSystemStore,
)
}

restConfig, err := i.restConfigFromKubeConfig(
secret.Data[config.KubeConfigSecretValueKey],
cfg.AgentTLSMode == config.AgentTLSModeSystemStore,
)
restConfig, err := i.restConfigFromKubeConfig(secret.Data[config.KubeConfigSecretValueKey], cfg.AgentTLSMode)
if err != nil {
return status, err
}
Expand Down Expand Up @@ -411,7 +399,15 @@ func isLegacyAgentNamespaceSelectedByUser() bool {

// restConfigFromKubeConfig checks kubeconfig data and tries to connect to server. If server is behind public CA, remove
// CertificateAuthorityData in kubeconfig file unless strict TLS mode is enabled.
func (i *importHandler) restConfigFromKubeConfig(data []byte, trustSystemStoreCAs bool) (*rest.Config, error) {
func (i *importHandler) restConfigFromKubeConfig(data []byte, agentTLSMode string) (*rest.Config, error) {
if agentTLSMode != config.AgentTLSModeStrict && agentTLSMode != config.AgentTLSModeSystemStore {
return nil, fmt.Errorf(
"provided config value for agentTLSMode is none of [%q,%q]",
config.AgentTLSModeStrict,
config.AgentTLSModeSystemStore,
)
}

clientConfig, err := clientcmd.NewClientConfigFromBytes(data)
if err != nil {
return nil, err
Expand All @@ -422,7 +418,7 @@ func (i *importHandler) restConfigFromKubeConfig(data []byte, trustSystemStoreCA
return nil, err
}

if trustSystemStoreCAs && raw.Contexts[raw.CurrentContext] != nil {
if agentTLSMode == config.AgentTLSModeSystemStore && raw.Contexts[raw.CurrentContext] != nil {
cluster := raw.Contexts[raw.CurrentContext].Cluster
if raw.Clusters[cluster] != nil {
if _, err := http.Get(raw.Clusters[cluster].Server); err == nil {
Expand Down
131 changes: 81 additions & 50 deletions internal/cmd/controller/reconciler/bundle_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"context"
"fmt"

"github.com/go-logr/logr"
"github.com/rancher/fleet/internal/cmd/controller/finalize"
"github.com/rancher/fleet/internal/cmd/controller/summary"
"github.com/rancher/fleet/internal/cmd/controller/target"
Expand Down Expand Up @@ -130,23 +131,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, client.IgnoreNotFound(err)
}

if bundle.DeletionTimestamp.IsZero() {
if !controllerutil.ContainsFinalizer(bundle, bundleFinalizer) {
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
if err := r.Get(ctx, req.NamespacedName, bundle); err != nil {
return err
}

controllerutil.AddFinalizer(bundle, bundleFinalizer)

return r.Update(ctx, bundle)
})

if client.IgnoreNotFound(err) != nil {
return ctrl.Result{}, err
}
}
} else {
if !bundle.DeletionTimestamp.IsZero() {
if controllerutil.ContainsFinalizer(bundle, bundleFinalizer) {
metrics.BundleCollector.Delete(req.Name, req.Namespace)

Expand Down Expand Up @@ -175,7 +160,29 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, nil
}

logger.V(1).Info("Reconciling bundle, checking targets, calculating changes, building objects", "generation", bundle.Generation, "observedGeneration", bundle.Status.ObservedGeneration)
if !controllerutil.ContainsFinalizer(bundle, bundleFinalizer) {
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
if err := r.Get(ctx, req.NamespacedName, bundle); err != nil {
return err
}

controllerutil.AddFinalizer(bundle, bundleFinalizer)

return r.Update(ctx, bundle)
})

if client.IgnoreNotFound(err) != nil {
return ctrl.Result{}, err
}
}

logger.V(1).Info(
"Reconciling bundle, checking targets, calculating changes, building objects",
"generation",
bundle.Generation,
"observedGeneration",
bundle.Status.ObservedGeneration,
)

contentsInOCI := bundle.Spec.ContentsID != "" && ociwrapper.ExperimentalOCIIsEnabled()
manifestID := bundle.Spec.ContentsID
Expand Down Expand Up @@ -217,8 +224,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
// the `helm.Chart` field, change which resources are used. The
// agents have access to all resources and use their specific
// set of `BundleDeploymentOptions`.
err := r.Store.Store(ctx, resourcesManifest)
if err != nil {
if err := r.Store.Store(ctx, resourcesManifest); err != nil {
return ctrl.Result{}, err
}
}
Expand Down Expand Up @@ -249,37 +255,12 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
// DependsOn with the bundle's DependsOn (pure function) and replacing
// the labels with the bundle's labels
for _, target := range matchedTargets {
if target.Deployment == nil {
continue
}
if target.Deployment.Namespace == "" {
logger.V(1).Info("Skipping bundledeployment with empty namespace, waiting for agentmanagement to set cluster.status.namespace", "bundledeployment", target.Deployment)
continue
}

// NOTE we don't use the existing BundleDeployment, we discard annotations, status, etc
// copy labels from Bundle as they might have changed
bd := target.BundleDeployment()

// No need to check the deletion timestamp here before adding a finalizer, since the bundle has just
// been created.
controllerutil.AddFinalizer(bd, bundleDeploymentFinalizer)

bd.Spec.OCIContents = contentsInOCI

updated := bd.DeepCopy()
op, err := controllerutil.CreateOrUpdate(ctx, r.Client, bd, func() error {
bd.Spec = updated.Spec
bd.Labels = updated.GetLabels()
return nil
})
if err != nil && !apierrors.IsAlreadyExists(err) {
logger.Error(err, "Reconcile failed to create or update bundledeployment", "bundledeployment", bd, "operation", op)
bd, err := r.createBundle(ctx, logger, target, contentsInOCI)
if err != nil {
return ctrl.Result{}, err
}
logger.V(1).Info(upper(op)+" bundledeployment", "bundledeployment", bd, "operation", op)

if contentsInOCI {
if bd != nil && contentsInOCI {
// we need to create the OCI registry credentials secret in the BundleDeployment's namespace
if err := r.createDeploymentOCISecret(ctx, bundle, bd); err != nil {
return ctrl.Result{}, err
Expand All @@ -291,8 +272,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr

err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
t := &fleet.Bundle{}
err := r.Get(ctx, req.NamespacedName, t)
if err != nil {
if err := r.Get(ctx, req.NamespacedName, t); err != nil {
return err
}
t.Status = bundle.Status
Expand Down Expand Up @@ -324,6 +304,57 @@ func upper(op controllerutil.OperationResult) string {
}
}

func (r *BundleReconciler) createBundle(
ctx context.Context,
logger logr.Logger,
target *target.Target,
contentsInOCI bool,
) (*fleet.BundleDeployment, error) {
if target.Deployment == nil {
return nil, nil
}
if target.Deployment.Namespace == "" {
logger.V(1).Info(
"Skipping bundledeployment with empty namespace, "+
"waiting for agentmanagement to set cluster.status.namespace",
"bundledeployment",
target.Deployment,
)
return nil, nil
}

// NOTE we don't use the existing BundleDeployment, we discard annotations, status, etc
// copy labels from Bundle as they might have changed
bd := target.BundleDeployment()

// No need to check the deletion timestamp here before adding a finalizer, since the bundle has just
// been created.
controllerutil.AddFinalizer(bd, bundleDeploymentFinalizer)

bd.Spec.OCIContents = contentsInOCI

updated := bd.DeepCopy()
op, err := controllerutil.CreateOrUpdate(ctx, r.Client, bd, func() error {
bd.Spec = updated.Spec
bd.Labels = updated.GetLabels()
return nil
})
if err != nil && !apierrors.IsAlreadyExists(err) {
logger.Error(
err,
"Reconcile failed to create or update bundledeployment",
"bundledeployment",
bd,
"operation",
op,
)
return nil, err
}
logger.V(1).Info(upper(op)+" bundledeployment", "bundledeployment", bd, "operation", op)

return bd, nil
}

func (r *BundleReconciler) createDeploymentOCISecret(ctx context.Context, bundle *fleet.Bundle, bd *fleet.BundleDeployment) error {
namespacedName := types.NamespacedName{
Namespace: bundle.Namespace,
Expand Down

0 comments on commit c8f246b

Please sign in to comment.