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

Jump to bottom

[release/v1.3.x] Fix incorrect use of format strings with the conditions package. #1199

Merged
merged 1 commit into from
Jul 8, 2024
Merged

[release/v1.3.x] Fix incorrect use of format strings with the conditions package. #1199

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
56 changes: 28 additions & 28 deletions internal/controller/kustomization_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
// Resolve the source reference and requeue the reconciliation if the source is not found.
artifactSource, err := r.getSource(ctx, obj)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", err)

if apierrors.IsNotFound(err) {
msg := fmt.Sprintf("Source '%s' not found", obj.Spec.SourceRef.String())
Expand All @@ -232,7 +232,7 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
}

if acl.IsAccessDenied(err) {
conditions.MarkFalse(obj, meta.ReadyCondition, apiacl.AccessDeniedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, apiacl.AccessDeniedReason, "%s", err)
log.Error(err, "Access denied to cross-namespace source")
r.event(obj, "unknown", eventv1.EventSeverityError, err.Error(), nil)
return ctrl.Result{RequeueAfter: obj.GetRetryInterval()}, nil
Expand All @@ -245,15 +245,15 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
// Requeue the reconciliation if the source artifact is not found.
if artifactSource.GetArtifact() == nil {
msg := fmt.Sprintf("Source artifact not found, retrying in %s", r.requeueDependency.String())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, msg)
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", msg)
log.Info(msg)
return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
}

// Check dependencies and requeue the reconciliation if the check fails.
if len(obj.Spec.DependsOn) > 0 {
if err := r.checkDependencies(ctx, obj, artifactSource); err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.DependencyNotReadyReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.DependencyNotReadyReason, "%s", err)
msg := fmt.Sprintf("Dependencies do not meet ready condition, retrying in %s", r.requeueDependency.String())
log.Info(msg)
r.event(obj, artifactSource.GetArtifact().Revision, eventv1.EventSeverityInfo, msg, nil)
Expand All @@ -268,7 +268,7 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
// Requeue at the specified retry interval if the artifact tarball is not found.
if errors.Is(reconcileErr, fetch.ErrFileNotFound) {
msg := fmt.Sprintf("Source is not ready, artifact not found, retrying in %s", r.requeueDependency.String())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, msg)
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", msg)
log.Info(msg)
return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
}
Expand Down Expand Up @@ -299,8 +299,8 @@ func (r *KustomizationReconciler) reconcile(
// Update status with the reconciliation progress.
revision := src.GetArtifact().Revision
progressingMsg := fmt.Sprintf("Fetching manifests for revision %s with a timeout of %s", revision, obj.GetTimeout().String())
conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "Reconciliation in progress")
conditions.MarkReconciling(obj, meta.ProgressingReason, progressingMsg)
conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "%s", "Reconciliation in progress")
conditions.MarkReconciling(obj, meta.ProgressingReason, "%s", progressingMsg)
if err := r.patch(ctx, obj, patcher); err != nil {
return fmt.Errorf("failed to update status: %w", err)
}
Expand All @@ -315,7 +315,7 @@ func (r *KustomizationReconciler) reconcile(
tmpDir, err := MkdirTempAbs("", "kustomization-")
if err != nil {
err = fmt.Errorf("tmp dir error: %w", err)
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.DirCreationFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.DirCreationFailedReason, "%s", err)
return err
}

Expand All @@ -333,27 +333,27 @@ func (r *KustomizationReconciler) reconcile(
os.Getenv("SOURCE_CONTROLLER_LOCALHOST"),
ctrl.LoggerFrom(ctx),
).Fetch(src.GetArtifact().URL, src.GetArtifact().Digest, tmpDir); err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", err)
return err
}

// check build path exists
dirPath, err := securejoin.SecureJoin(tmpDir, obj.Spec.Path)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", err)
return err
}

if _, err := os.Stat(dirPath); err != nil {
err = fmt.Errorf("kustomization path not found: %w", err)
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", err)
return err
}

// Report progress and set last attempted revision in status.
obj.Status.LastAttemptedRevision = revision
progressingMsg = fmt.Sprintf("Building manifests for revision %s with a timeout of %s", revision, obj.GetTimeout().String())
conditions.MarkReconciling(obj, meta.ProgressingReason, progressingMsg)
conditions.MarkReconciling(obj, meta.ProgressingReason, "%s", progressingMsg)
if err := r.patch(ctx, obj, patcher); err != nil {
return fmt.Errorf("failed to update status: %w", err)
}
Expand All @@ -373,33 +373,33 @@ func (r *KustomizationReconciler) reconcile(
// Create the Kubernetes client that runs under impersonation.
kubeClient, statusPoller, err := impersonation.GetClient(ctx)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, "%s", err)
return fmt.Errorf("failed to build kube client: %w", err)
}

// Generate kustomization.yaml if needed.
k, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, "%s", err)
return err
}
err = r.generate(unstructured.Unstructured{Object: k}, tmpDir, dirPath)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, "%s", err)
return err
}

// Build the Kustomize overlay and decrypt secrets if needed.
resources, err := r.build(ctx, obj, unstructured.Unstructured{Object: k}, tmpDir, dirPath)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, "%s", err)
return err
}

// Convert the build result into Kubernetes unstructured objects.
objects, err := ssautil.ReadObjects(bytes.NewReader(resources))
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, "%s", err)
return err
}

Expand All @@ -413,23 +413,23 @@ func (r *KustomizationReconciler) reconcile(

// Update status with the reconciliation progress.
progressingMsg = fmt.Sprintf("Detecting drift for revision %s with a timeout of %s", revision, obj.GetTimeout().String())
conditions.MarkReconciling(obj, meta.ProgressingReason, progressingMsg)
conditions.MarkReconciling(obj, meta.ProgressingReason, "%s", progressingMsg)
if err := r.patch(ctx, obj, patcher); err != nil {
return fmt.Errorf("failed to update status: %w", err)
}

// Validate and apply resources in stages.
drifted, changeSet, err := r.apply(ctx, resourceManager, obj, revision, objects)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, "%s", err)
return err
}

// Create an inventory from the reconciled resources.
newInventory := inventory.New()
err = inventory.AddChangeSet(newInventory, changeSet)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, "%s", err)
return err
}

Expand All @@ -439,13 +439,13 @@ func (r *KustomizationReconciler) reconcile(
// Detect stale resources which are subject to garbage collection.
staleObjects, err := inventory.Diff(oldInventory, newInventory)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, "%s", err)
return err
}

// Run garbage collection for stale resources that do not have pruning disabled.
if _, err := r.prune(ctx, resourceManager, obj, revision, staleObjects); err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.PruneFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.PruneFailedReason, "%s", err)
return err
}

Expand All @@ -459,7 +459,7 @@ func (r *KustomizationReconciler) reconcile(
isNewRevision,
drifted,
changeSet.ToObjMetadataSet()); err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.HealthCheckFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.HealthCheckFailedReason, "%s", err)
return err
}

Expand Down Expand Up @@ -890,8 +890,8 @@ func (r *KustomizationReconciler) checkHealth(ctx context.Context,

// Update status with the reconciliation progress.
message := fmt.Sprintf("Running health checks for revision %s with a timeout of %s", revision, obj.GetTimeout().String())
conditions.MarkReconciling(obj, meta.ProgressingReason, message)
conditions.MarkUnknown(obj, meta.HealthyCondition, meta.ProgressingReason, message)
conditions.MarkReconciling(obj, meta.ProgressingReason, "%s", message)
conditions.MarkUnknown(obj, meta.HealthyCondition, meta.ProgressingReason, "%s", message)
if err := r.patch(ctx, obj, patcher); err != nil {
return fmt.Errorf("unable to update the healthy status to progressing: %w", err)
}
Expand All @@ -902,8 +902,8 @@ func (r *KustomizationReconciler) checkHealth(ctx context.Context,
Timeout: obj.GetTimeout(),
FailFast: r.FailFast,
}); err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.HealthCheckFailedReason, err.Error())
conditions.MarkFalse(obj, meta.HealthyCondition, meta.HealthCheckFailedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, meta.HealthCheckFailedReason, "%s", err)
conditions.MarkFalse(obj, meta.HealthyCondition, meta.HealthCheckFailedReason, "%s", err)
return fmt.Errorf("health check failed after %s: %w", time.Since(checkStart).String(), err)
}

Expand All @@ -913,7 +913,7 @@ func (r *KustomizationReconciler) checkHealth(ctx context.Context,
r.event(obj, revision, eventv1.EventSeverityInfo, msg, nil)
}

conditions.MarkTrue(obj, meta.HealthyCondition, meta.SucceededReason, msg)
conditions.MarkTrue(obj, meta.HealthyCondition, meta.SucceededReason, "%s", msg)
if err := r.patch(ctx, obj, patcher); err != nil {
return fmt.Errorf("unable to update the healthy status to progressing: %w", err)
}
Expand Down