diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 0541d8bed..ff06427da 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -100,6 +100,9 @@ jobs: kubectl -n source-system apply -f ./config/testdata/helmchart-from-bucket/source.yaml kubectl -n source-system wait bucket/charts --for=condition=ready --timeout=1m kubectl -n source-system wait helmchart/helmchart-bucket --for=condition=ready --timeout=1m + - name: Logs + run: | + kubectl -n source-system logs deploy/source-controller - name: Debug failure if: failure() run: | diff --git a/controllers/bucket_controller.go b/controllers/bucket_controller.go index 61586f698..de9b31be7 100644 --- a/controllers/bucket_controller.go +++ b/controllers/bucket_controller.go @@ -100,11 +100,11 @@ func (r *BucketReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { // set initial status if resetBucket, ok := r.resetStatus(bucket); ok { bucket = resetBucket - if err := r.Status().Update(ctx, &bucket); err != nil { + if err := r.updateStatus(ctx, req, bucket.Status); err != nil { log.Error(err, "unable to update status") return ctrl.Result{Requeue: true}, err } - r.recordReadiness(bucket, false) + r.recordReadiness(bucket) } // purge old artifacts from storage @@ -116,7 +116,7 @@ func (r *BucketReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { reconciledBucket, reconcileErr := r.reconcile(ctx, *bucket.DeepCopy()) // update status with the reconciliation result - if err := r.Status().Update(ctx, &reconciledBucket); err != nil { + if err := r.updateStatus(ctx, req, reconciledBucket.Status); err != nil { log.Error(err, "unable to update status") return ctrl.Result{Requeue: true}, err } @@ -124,7 +124,7 @@ func (r *BucketReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { // if reconciliation failed, record the failure and requeue immediately if reconcileErr != nil { r.event(reconciledBucket, events.EventSeverityError, reconcileErr.Error()) - r.recordReadiness(reconciledBucket, false) + r.recordReadiness(reconciledBucket) return ctrl.Result{Requeue: true}, reconcileErr } @@ -132,7 +132,7 @@ func (r *BucketReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { if bucket.Status.Artifact == nil || reconciledBucket.Status.Artifact.Revision != bucket.Status.Artifact.Revision { r.event(reconciledBucket, events.EventSeverityInfo, sourcev1.BucketReadyMessage(reconciledBucket)) } - r.recordReadiness(reconciledBucket, false) + r.recordReadiness(reconciledBucket) log.Info(fmt.Sprintf("Reconciliation finished in %s, next run in %s", time.Now().Sub(start).String(), @@ -259,7 +259,7 @@ func (r *BucketReconciler) reconcileDelete(ctx context.Context, bucket sourcev1. } // Record deleted status - r.recordReadiness(bucket, true) + r.recordReadiness(bucket) // Remove our finalizer from the list and update it controllerutil.RemoveFinalizer(&bucket, sourcev1.SourceFinalizer) @@ -385,7 +385,7 @@ func (r *BucketReconciler) event(bucket sourcev1.Bucket, severity, msg string) { } } -func (r *BucketReconciler) recordReadiness(bucket sourcev1.Bucket, deleted bool) { +func (r *BucketReconciler) recordReadiness(bucket sourcev1.Bucket) { if r.MetricsRecorder == nil { return } @@ -399,11 +399,23 @@ func (r *BucketReconciler) recordReadiness(bucket sourcev1.Bucket, deleted bool) return } if rc := meta.GetCondition(bucket.Status.Conditions, meta.ReadyCondition); rc != nil { - r.MetricsRecorder.RecordCondition(*objRef, *rc, deleted) + r.MetricsRecorder.RecordCondition(*objRef, *rc, !bucket.DeletionTimestamp.IsZero()) } else { r.MetricsRecorder.RecordCondition(*objRef, meta.Condition{ Type: meta.ReadyCondition, Status: corev1.ConditionUnknown, - }, deleted) + }, !bucket.DeletionTimestamp.IsZero()) } } + +func (r *BucketReconciler) updateStatus(ctx context.Context, req ctrl.Request, newStatus sourcev1.BucketStatus) error { + var bucket sourcev1.Bucket + if err := r.Get(ctx, req.NamespacedName, &bucket); err != nil { + return err + } + + patch := client.MergeFrom(bucket.DeepCopy()) + bucket.Status = newStatus + + return r.Status().Patch(ctx, &bucket, patch) +} diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index b88c0d86b..8be85e67f 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -99,11 +99,11 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro // set initial status if resetRepository, ok := r.resetStatus(repository); ok { repository = resetRepository - if err := r.Status().Update(ctx, &repository); err != nil { + if err := r.updateStatus(ctx, req, repository.Status); err != nil { log.Error(err, "unable to update status") return ctrl.Result{Requeue: true}, err } - r.recordReadiness(repository, false) + r.recordReadiness(repository) } // purge old artifacts from storage @@ -115,7 +115,7 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro reconciledRepository, reconcileErr := r.reconcile(ctx, *repository.DeepCopy()) // update status with the reconciliation result - if err := r.Status().Update(ctx, &reconciledRepository); err != nil { + if err := r.updateStatus(ctx, req, reconciledRepository.Status); err != nil { log.Error(err, "unable to update status") return ctrl.Result{Requeue: true}, err } @@ -123,7 +123,7 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro // if reconciliation failed, record the failure and requeue immediately if reconcileErr != nil { r.event(reconciledRepository, events.EventSeverityError, reconcileErr.Error()) - r.recordReadiness(reconciledRepository, false) + r.recordReadiness(reconciledRepository) return ctrl.Result{Requeue: true}, reconcileErr } @@ -131,7 +131,7 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro if repository.Status.Artifact == nil || reconciledRepository.Status.Artifact.Revision != repository.Status.Artifact.Revision { r.event(reconciledRepository, events.EventSeverityInfo, sourcev1.GitRepositoryReadyMessage(reconciledRepository)) } - r.recordReadiness(reconciledRepository, false) + r.recordReadiness(reconciledRepository) log.Info(fmt.Sprintf("Reconciliation finished in %s, next run in %s", time.Now().Sub(start).String(), @@ -257,7 +257,7 @@ func (r *GitRepositoryReconciler) reconcileDelete(ctx context.Context, repositor } // Record deleted status - r.recordReadiness(repository, true) + r.recordReadiness(repository) // Remove our finalizer from the list and update it controllerutil.RemoveFinalizer(&repository, sourcev1.SourceFinalizer) @@ -345,7 +345,7 @@ func (r *GitRepositoryReconciler) event(repository sourcev1.GitRepository, sever } } -func (r *GitRepositoryReconciler) recordReadiness(repository sourcev1.GitRepository, deleted bool) { +func (r *GitRepositoryReconciler) recordReadiness(repository sourcev1.GitRepository) { if r.MetricsRecorder == nil { return } @@ -359,11 +359,23 @@ func (r *GitRepositoryReconciler) recordReadiness(repository sourcev1.GitReposit return } if rc := meta.GetCondition(repository.Status.Conditions, meta.ReadyCondition); rc != nil { - r.MetricsRecorder.RecordCondition(*objRef, *rc, deleted) + r.MetricsRecorder.RecordCondition(*objRef, *rc, !repository.DeletionTimestamp.IsZero()) } else { r.MetricsRecorder.RecordCondition(*objRef, meta.Condition{ Type: meta.ReadyCondition, Status: corev1.ConditionUnknown, - }, deleted) + }, !repository.DeletionTimestamp.IsZero()) } } + +func (r *GitRepositoryReconciler) updateStatus(ctx context.Context, req ctrl.Request, newStatus sourcev1.GitRepositoryStatus) error { + var repository sourcev1.GitRepository + if err := r.Get(ctx, req.NamespacedName, &repository); err != nil { + return err + } + + patch := client.MergeFrom(repository.DeepCopy()) + repository.Status = newStatus + + return r.Status().Patch(ctx, &repository, patch) +} diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 45b56132d..1a0245b7b 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -106,11 +106,11 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { resetChart, changed := r.resetStatus(chart) if changed { chart = resetChart - if err := r.Status().Update(ctx, &chart); err != nil { + if err := r.updateStatus(ctx, req, chart.Status); err != nil { log.Error(err, "unable to update status") return ctrl.Result{Requeue: true}, err } - r.recordReadiness(chart, false) + r.recordReadiness(chart) } // Purge all but current artifact from storage @@ -122,7 +122,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { source, err := r.getSource(ctx, chart) if err != nil { chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error()) - if err := r.Status().Update(ctx, &chart); err != nil { + if err := r.updateStatus(ctx, req, chart.Status); err != nil { log.Error(err, "unable to update status") } return ctrl.Result{Requeue: true}, err @@ -133,10 +133,10 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { err = fmt.Errorf("no artifact found for source `%s` kind '%s'", chart.Spec.SourceRef.Name, chart.Spec.SourceRef.Kind) chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error()) - if err := r.Status().Update(ctx, &chart); err != nil { + if err := r.updateStatus(ctx, req, chart.Status); err != nil { log.Error(err, "unable to update status") } - r.recordReadiness(chart, false) + r.recordReadiness(chart) return ctrl.Result{Requeue: true}, err } @@ -155,7 +155,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } // Update status with the reconciliation result - if err := r.Status().Update(ctx, &reconciledChart); err != nil { + if err := r.updateStatus(ctx, req, reconciledChart.Status); err != nil { log.Error(err, "unable to update status") return ctrl.Result{Requeue: true}, err } @@ -163,7 +163,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { // If reconciliation failed, record the failure and requeue immediately if reconcileErr != nil { r.event(reconciledChart, events.EventSeverityError, reconcileErr.Error()) - r.recordReadiness(reconciledChart, false) + r.recordReadiness(reconciledChart) return ctrl.Result{Requeue: true}, reconcileErr } @@ -171,7 +171,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { if chart.Status.Artifact == nil || reconciledChart.Status.Artifact.Revision != chart.Status.Artifact.Revision { r.event(reconciledChart, events.EventSeverityInfo, sourcev1.HelmChartReadyMessage(reconciledChart)) } - r.recordReadiness(reconciledChart, false) + r.recordReadiness(reconciledChart) log.Info(fmt.Sprintf("Reconciliation finished in %s, next run in %s", time.Now().Sub(start).String(), @@ -596,7 +596,7 @@ func (r *HelmChartReconciler) reconcileDelete(ctx context.Context, chart sourcev } // Record deleted status - r.recordReadiness(chart, true) + r.recordReadiness(chart) // Remove our finalizer from the list and update it controllerutil.RemoveFinalizer(&chart, sourcev1.SourceFinalizer) @@ -662,7 +662,7 @@ func (r *HelmChartReconciler) event(chart sourcev1.HelmChart, severity, msg stri } } -func (r *HelmChartReconciler) recordReadiness(chart sourcev1.HelmChart, deleted bool) { +func (r *HelmChartReconciler) recordReadiness(chart sourcev1.HelmChart) { if r.MetricsRecorder == nil { return } @@ -676,15 +676,27 @@ func (r *HelmChartReconciler) recordReadiness(chart sourcev1.HelmChart, deleted return } if rc := meta.GetCondition(chart.Status.Conditions, meta.ReadyCondition); rc != nil { - r.MetricsRecorder.RecordCondition(*objRef, *rc, deleted) + r.MetricsRecorder.RecordCondition(*objRef, *rc, !chart.DeletionTimestamp.IsZero()) } else { r.MetricsRecorder.RecordCondition(*objRef, meta.Condition{ Type: meta.ReadyCondition, Status: corev1.ConditionUnknown, - }, deleted) + }, !chart.DeletionTimestamp.IsZero()) } } +func (r *HelmChartReconciler) updateStatus(ctx context.Context, req ctrl.Request, newStatus sourcev1.HelmChartStatus) error { + var chart sourcev1.HelmChart + if err := r.Get(ctx, req.NamespacedName, &chart); err != nil { + return err + } + + patch := client.MergeFrom(chart.DeepCopy()) + chart.Status = newStatus + + return r.Status().Patch(ctx, &chart, patch) +} + func (r *HelmChartReconciler) indexHelmRepositoryByURL(o runtime.Object) []string { repo, ok := o.(*sourcev1.HelmRepository) if !ok { diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index d91c9110f..52d0a835b 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -100,11 +100,11 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err // set initial status if resetRepository, ok := r.resetStatus(repository); ok { repository = resetRepository - if err := r.Status().Update(ctx, &repository); err != nil { + if err := r.updateStatus(ctx, req, repository.Status); err != nil { log.Error(err, "unable to update status") return ctrl.Result{Requeue: true}, err } - r.recordReadiness(repository, false) + r.recordReadiness(repository) } // purge old artifacts from storage @@ -116,7 +116,7 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err reconciledRepository, reconcileErr := r.reconcile(ctx, *repository.DeepCopy()) // update status with the reconciliation result - if err := r.Status().Update(ctx, &reconciledRepository); err != nil { + if err := r.updateStatus(ctx, req, reconciledRepository.Status); err != nil { log.Error(err, "unable to update status") return ctrl.Result{Requeue: true}, err } @@ -124,7 +124,7 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err // if reconciliation failed, record the failure and requeue immediately if reconcileErr != nil { r.event(reconciledRepository, events.EventSeverityError, reconcileErr.Error()) - r.recordReadiness(reconciledRepository, false) + r.recordReadiness(reconciledRepository) return ctrl.Result{Requeue: true}, reconcileErr } @@ -132,7 +132,7 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err if repository.Status.Artifact == nil || reconciledRepository.Status.Artifact.Revision != repository.Status.Artifact.Revision { r.event(reconciledRepository, events.EventSeverityInfo, sourcev1.HelmRepositoryReadyMessage(reconciledRepository)) } - r.recordReadiness(reconciledRepository, false) + r.recordReadiness(reconciledRepository) log.Info(fmt.Sprintf("Reconciliation finished in %s, next run in %s", time.Now().Sub(start).String(), @@ -255,7 +255,7 @@ func (r *HelmRepositoryReconciler) reconcileDelete(ctx context.Context, reposito } // Record deleted status - r.recordReadiness(repository, true) + r.recordReadiness(repository) // Remove our finalizer from the list and update it controllerutil.RemoveFinalizer(&repository, sourcev1.SourceFinalizer) @@ -319,7 +319,7 @@ func (r *HelmRepositoryReconciler) event(repository sourcev1.HelmRepository, sev } } -func (r *HelmRepositoryReconciler) recordReadiness(repository sourcev1.HelmRepository, deleted bool) { +func (r *HelmRepositoryReconciler) recordReadiness(repository sourcev1.HelmRepository) { if r.MetricsRecorder == nil { return } @@ -333,11 +333,23 @@ func (r *HelmRepositoryReconciler) recordReadiness(repository sourcev1.HelmRepos return } if rc := meta.GetCondition(repository.Status.Conditions, meta.ReadyCondition); rc != nil { - r.MetricsRecorder.RecordCondition(*objRef, *rc, deleted) + r.MetricsRecorder.RecordCondition(*objRef, *rc, !repository.DeletionTimestamp.IsZero()) } else { r.MetricsRecorder.RecordCondition(*objRef, meta.Condition{ Type: meta.ReadyCondition, Status: corev1.ConditionUnknown, - }, deleted) + }, !repository.DeletionTimestamp.IsZero()) } } + +func (r *HelmRepositoryReconciler) updateStatus(ctx context.Context, req ctrl.Request, newStatus sourcev1.HelmRepositoryStatus) error { + var repository sourcev1.HelmRepository + if err := r.Get(ctx, req.NamespacedName, &repository); err != nil { + return err + } + + patch := client.MergeFrom(repository.DeepCopy()) + repository.Status = newStatus + + return r.Status().Patch(ctx, &repository, patch) +}