Wrap errors in controller helper methods and swap logic in cleanups (#3960)

This commit is contained in:
Nikola Jokic
2025-03-07 11:58:53 +01:00
committed by GitHub
parent 7a5996f467
commit 2dab45c373
4 changed files with 72 additions and 73 deletions

View File

@@ -344,7 +344,7 @@ func (r *EphemeralRunnerReconciler) cleanupResources(ctx context.Context, epheme
if pod.ObjectMeta.DeletionTimestamp.IsZero() {
log.Info("Deleting the runner pod")
if err := r.Delete(ctx, pod); err != nil && !kerrors.IsNotFound(err) {
return false, fmt.Errorf("failed to delete pod: %v", err)
return false, fmt.Errorf("failed to delete pod: %w", err)
}
}
return false, nil
@@ -361,7 +361,7 @@ func (r *EphemeralRunnerReconciler) cleanupResources(ctx context.Context, epheme
if secret.ObjectMeta.DeletionTimestamp.IsZero() {
log.Info("Deleting the jitconfig secret")
if err := r.Delete(ctx, secret); err != nil && !kerrors.IsNotFound(err) {
return false, fmt.Errorf("failed to delete secret: %v", err)
return false, fmt.Errorf("failed to delete secret: %w", err)
}
}
return false, nil
@@ -377,7 +377,7 @@ func (r *EphemeralRunnerReconciler) cleanupContainerHooksResources(ctx context.C
log.Info("Cleaning up runner linked pods")
done, err = r.cleanupRunnerLinkedPods(ctx, ephemeralRunner, log)
if err != nil {
return false, fmt.Errorf("failed to clean up runner linked pods: %v", err)
return false, fmt.Errorf("failed to clean up runner linked pods: %w", err)
}
if !done {
@@ -402,7 +402,7 @@ func (r *EphemeralRunnerReconciler) cleanupRunnerLinkedPods(ctx context.Context,
var runnerLinkedPodList corev1.PodList
err = r.List(ctx, &runnerLinkedPodList, client.InNamespace(ephemeralRunner.Namespace), runnerLinedLabels)
if err != nil {
return false, fmt.Errorf("failed to list runner-linked pods: %v", err)
return false, fmt.Errorf("failed to list runner-linked pods: %w", err)
}
if len(runnerLinkedPodList.Items) == 0 {
@@ -421,7 +421,7 @@ func (r *EphemeralRunnerReconciler) cleanupRunnerLinkedPods(ctx context.Context,
log.Info("Deleting container hooks runner-linked pod", "name", linkedPod.Name)
if err := r.Delete(ctx, linkedPod); err != nil && !kerrors.IsNotFound(err) {
errs = append(errs, fmt.Errorf("failed to delete runner linked pod %q: %v", linkedPod.Name, err))
errs = append(errs, fmt.Errorf("failed to delete runner linked pod %q: %w", linkedPod.Name, err))
}
}
@@ -456,7 +456,7 @@ func (r *EphemeralRunnerReconciler) cleanupRunnerLinkedSecrets(ctx context.Conte
log.Info("Deleting container hooks runner-linked secret", "name", s.Name)
if err := r.Delete(ctx, s); err != nil && !kerrors.IsNotFound(err) {
errs = append(errs, fmt.Errorf("failed to delete runner linked secret %q: %v", s.Name, err))
errs = append(errs, fmt.Errorf("failed to delete runner linked secret %q: %w", s.Name, err))
}
}
@@ -470,12 +470,12 @@ func (r *EphemeralRunnerReconciler) markAsFailed(ctx context.Context, ephemeralR
obj.Status.Reason = reason
obj.Status.Message = errMessage
}); err != nil {
return fmt.Errorf("failed to update ephemeral runner status Phase/Message: %v", err)
return fmt.Errorf("failed to update ephemeral runner status Phase/Message: %w", err)
}
log.Info("Removing the runner from the service")
if err := r.deleteRunnerFromService(ctx, ephemeralRunner, log); err != nil {
return fmt.Errorf("failed to remove the runner from service: %v", err)
return fmt.Errorf("failed to remove the runner from service: %w", err)
}
log.Info("EphemeralRunner is marked as Failed and deleted from the service")
@@ -487,7 +487,7 @@ func (r *EphemeralRunnerReconciler) markAsFinished(ctx context.Context, ephemera
if err := patchSubResource(ctx, r.Status(), ephemeralRunner, func(obj *v1alpha1.EphemeralRunner) {
obj.Status.Phase = corev1.PodSucceeded
}); err != nil {
return fmt.Errorf("failed to update ephemeral runner with status finished: %v", err)
return fmt.Errorf("failed to update ephemeral runner with status finished: %w", err)
}
log.Info("EphemeralRunner status is marked as Finished")
@@ -500,7 +500,7 @@ func (r *EphemeralRunnerReconciler) deletePodAsFailed(ctx context.Context, ephem
if pod.ObjectMeta.DeletionTimestamp.IsZero() {
log.Info("Deleting the ephemeral runner pod", "podId", pod.UID)
if err := r.Delete(ctx, pod); err != nil && !kerrors.IsNotFound(err) {
return fmt.Errorf("failed to delete pod with status failed: %v", err)
return fmt.Errorf("failed to delete pod with status failed: %w", err)
}
}
@@ -514,7 +514,7 @@ func (r *EphemeralRunnerReconciler) deletePodAsFailed(ctx context.Context, ephem
obj.Status.Reason = pod.Status.Reason
obj.Status.Message = pod.Status.Message
}); err != nil {
return fmt.Errorf("failed to update ephemeral runner status: failed attempts: %v", err)
return fmt.Errorf("failed to update ephemeral runner status: failed attempts: %w", err)
}
log.Info("EphemeralRunner pod is deleted and status is updated with failure count")
@@ -528,7 +528,7 @@ func (r *EphemeralRunnerReconciler) updateStatusWithRunnerConfig(ctx context.Con
log.Info("Creating ephemeral runner JIT config")
actionsClient, err := r.actionsClientFor(ctx, ephemeralRunner)
if err != nil {
return &ctrl.Result{}, fmt.Errorf("failed to get actions client for generating JIT config: %v", err)
return &ctrl.Result{}, fmt.Errorf("failed to get actions client for generating JIT config: %w", err)
}
jitSettings := &actions.RunnerScaleSetJitRunnerSetting{
@@ -546,12 +546,12 @@ func (r *EphemeralRunnerReconciler) updateStatusWithRunnerConfig(ctx context.Con
if err != nil {
actionsError := &actions.ActionsError{}
if !errors.As(err, &actionsError) {
return &ctrl.Result{}, fmt.Errorf("failed to generate JIT config with generic error: %v", err)
return &ctrl.Result{}, fmt.Errorf("failed to generate JIT config with generic error: %w", err)
}
if actionsError.StatusCode != http.StatusConflict ||
!actionsError.IsException("AgentExistsException") {
return &ctrl.Result{}, fmt.Errorf("failed to generate JIT config with Actions service error: %v", err)
return &ctrl.Result{}, fmt.Errorf("failed to generate JIT config with Actions service error: %w", err)
}
// If the runner with the name we want already exists it means:
@@ -564,7 +564,7 @@ func (r *EphemeralRunnerReconciler) updateStatusWithRunnerConfig(ctx context.Con
log.Info("Getting runner jit config failed with conflict error, trying to get the runner by name", "runnerName", ephemeralRunner.Name)
existingRunner, err := actionsClient.GetRunnerByName(ctx, ephemeralRunner.Name)
if err != nil {
return &ctrl.Result{}, fmt.Errorf("failed to get runner by name: %v", err)
return &ctrl.Result{}, fmt.Errorf("failed to get runner by name: %w", err)
}
if existingRunner == nil {
@@ -577,7 +577,7 @@ func (r *EphemeralRunnerReconciler) updateStatusWithRunnerConfig(ctx context.Con
log.Info("Removing the runner with the same name")
err := actionsClient.RemoveRunner(ctx, int64(existingRunner.Id))
if err != nil {
return &ctrl.Result{}, fmt.Errorf("failed to remove runner from the service: %v", err)
return &ctrl.Result{}, fmt.Errorf("failed to remove runner from the service: %w", err)
}
log.Info("Removed the runner with the same name, re-queuing the reconciliation")
@@ -586,7 +586,7 @@ func (r *EphemeralRunnerReconciler) updateStatusWithRunnerConfig(ctx context.Con
// TODO: Do we want to mark the ephemeral runner as failed, and let EphemeralRunnerSet to clean it up, so we can recover from this situation?
// The situation is that the EphemeralRunner's name is already used by something else to register a runner, and we can't take the control back.
return &ctrl.Result{}, fmt.Errorf("runner with the same name but doesn't belong to this RunnerScaleSet: %v", err)
return &ctrl.Result{}, fmt.Errorf("runner with the same name but doesn't belong to this RunnerScaleSet: %w", err)
}
log.Info("Created ephemeral runner JIT config", "runnerId", jitConfig.Runner.Id)
@@ -597,7 +597,7 @@ func (r *EphemeralRunnerReconciler) updateStatusWithRunnerConfig(ctx context.Con
obj.Status.RunnerJITConfig = jitConfig.EncodedJITConfig
})
if err != nil {
return &ctrl.Result{}, fmt.Errorf("failed to update runner status for RunnerId/RunnerName/RunnerJITConfig: %v", err)
return &ctrl.Result{}, fmt.Errorf("failed to update runner status for RunnerId/RunnerName/RunnerJITConfig: %w", err)
}
// We want to continue without a requeue for faster pod creation.
@@ -691,12 +691,12 @@ func (r *EphemeralRunnerReconciler) createSecret(ctx context.Context, runner *v1
jitSecret := r.ResourceBuilder.newEphemeralRunnerJitSecret(runner)
if err := ctrl.SetControllerReference(runner, jitSecret, r.Scheme); err != nil {
return &ctrl.Result{}, fmt.Errorf("failed to set controller reference: %v", err)
return &ctrl.Result{}, fmt.Errorf("failed to set controller reference: %w", err)
}
log.Info("Created new secret spec for ephemeral runner")
if err := r.Create(ctx, jitSecret); err != nil {
return &ctrl.Result{}, fmt.Errorf("failed to create jit secret: %v", err)
return &ctrl.Result{}, fmt.Errorf("failed to create jit secret: %w", err)
}
log.Info("Created ephemeral runner secret", "secretName", jitSecret.Name)
@@ -743,7 +743,7 @@ func (r *EphemeralRunnerReconciler) updateRunStatusFromPod(ctx context.Context,
obj.Status.Message = pod.Status.Message
})
if err != nil {
return fmt.Errorf("failed to update runner status for Phase/Reason/Message/Ready: %v", err)
return fmt.Errorf("failed to update runner status for Phase/Reason/Message/Ready: %w", err)
}
log.Info("Updated ephemeral runner status")
@@ -835,7 +835,7 @@ func (r EphemeralRunnerReconciler) runnerRegisteredWithService(ctx context.Conte
if actionsError.StatusCode != http.StatusNotFound ||
!actionsError.IsException("AgentNotFoundException") {
return false, fmt.Errorf("failed to check if runner exists in GitHub service: %v", err)
return false, fmt.Errorf("failed to check if runner exists in GitHub service: %w", err)
}
log.Info("Runner does not exist in GitHub service", "runnerId", runner.Status.RunnerId)
@@ -849,7 +849,7 @@ func (r EphemeralRunnerReconciler) runnerRegisteredWithService(ctx context.Conte
func (r *EphemeralRunnerReconciler) deleteRunnerFromService(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, log logr.Logger) error {
client, err := r.actionsClientFor(ctx, ephemeralRunner)
if err != nil {
return fmt.Errorf("failed to get actions client for runner: %v", err)
return fmt.Errorf("failed to get actions client for runner: %w", err)
}
log.Info("Removing runner from the service", "runnerId", ephemeralRunner.Status.RunnerId)