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

Fix linting errors #2574

Merged
merged 3 commits into from
Jul 3, 2024
Merged

Fix linting errors #2574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b186054
Refactor cluster import
weyfonk Jul 2, 2024
b4908a6
Refactor bundle deployment creation into separate method
weyfonk Jul 2, 2024
b641524
Check for nil bd
manno Jul 3, 2024
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
26 changes: 11 additions & 15 deletions internal/cmd/controller/agentmanagement/controllers/cluster/import.go
View file
Open in desktop
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
Open in desktop
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)
manno marked this conversation as resolved.
Show resolved Hide resolved
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
Loading