From 565b14a148c88469422afe416a5f3e756e4618c4 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 18 Mar 2021 10:20:49 +0900 Subject: [PATCH 1/3] Fix `status.lastRegistrationCheckTime in body must be of type string: \"null\"` error Follow-up for #392 --- api/v1alpha1/horizontalrunnerautoscaler_types.go | 1 + charts/actions-runner-controller/Chart.yaml | 2 +- .../actions.summerwind.dev_horizontalrunnerautoscalers.yaml | 1 + .../actions.summerwind.dev_horizontalrunnerautoscalers.yaml | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/api/v1alpha1/horizontalrunnerautoscaler_types.go b/api/v1alpha1/horizontalrunnerautoscaler_types.go index f1e1d78e..2bf5cb66 100644 --- a/api/v1alpha1/horizontalrunnerautoscaler_types.go +++ b/api/v1alpha1/horizontalrunnerautoscaler_types.go @@ -156,6 +156,7 @@ type HorizontalRunnerAutoscalerStatus struct { DesiredReplicas *int `json:"desiredReplicas,omitempty"` // +optional + // +nullable LastSuccessfulScaleOutTime *metav1.Time `json:"lastSuccessfulScaleOutTime,omitempty"` // +optional diff --git a/charts/actions-runner-controller/Chart.yaml b/charts/actions-runner-controller/Chart.yaml index 75100f06..b68e248b 100644 --- a/charts/actions-runner-controller/Chart.yaml +++ b/charts/actions-runner-controller/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.10.1 +version: 0.10.2 home: https://github.com/summerwind/actions-runner-controller diff --git a/charts/actions-runner-controller/crds/actions.summerwind.dev_horizontalrunnerautoscalers.yaml b/charts/actions-runner-controller/crds/actions.summerwind.dev_horizontalrunnerautoscalers.yaml index 7c261b3f..b7cd99ae 100644 --- a/charts/actions-runner-controller/crds/actions.summerwind.dev_horizontalrunnerautoscalers.yaml +++ b/charts/actions-runner-controller/crds/actions.summerwind.dev_horizontalrunnerautoscalers.yaml @@ -207,6 +207,7 @@ spec: type: integer lastSuccessfulScaleOutTime: format: date-time + nullable: true type: string observedGeneration: description: ObservedGeneration is the most recent generation observed diff --git a/config/crd/bases/actions.summerwind.dev_horizontalrunnerautoscalers.yaml b/config/crd/bases/actions.summerwind.dev_horizontalrunnerautoscalers.yaml index 7c261b3f..b7cd99ae 100644 --- a/config/crd/bases/actions.summerwind.dev_horizontalrunnerautoscalers.yaml +++ b/config/crd/bases/actions.summerwind.dev_horizontalrunnerautoscalers.yaml @@ -207,6 +207,7 @@ spec: type: integer lastSuccessfulScaleOutTime: format: date-time + nullable: true type: string observedGeneration: description: ObservedGeneration is the most recent generation observed From 7a7086e7aa14848ec19fe3a9ed6b1f97277397f2 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 18 Mar 2021 10:26:21 +0900 Subject: [PATCH 2/3] Make error logs more helpful --- controllers/runner_controller.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/controllers/runner_controller.go b/controllers/runner_controller.go index 3b7f109a..ff2d31ff 100644 --- a/controllers/runner_controller.go +++ b/controllers/runner_controller.go @@ -369,7 +369,7 @@ func (r *RunnerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { updated.Status.LastRegistrationCheckTime = &metav1.Time{Time: time.Now()} if err := r.Status().Patch(ctx, updated, client.MergeFrom(&runner)); err != nil { - log.Error(err, "Failed to update runner status") + log.Error(err, "Failed to update runner status for LastRegistrationCheckTime") return ctrl.Result{}, err } @@ -391,7 +391,7 @@ func (r *RunnerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { updated.Status.Message = pod.Status.Message if err := r.Status().Patch(ctx, updated, client.MergeFrom(&runner)); err != nil { - log.Error(err, "Failed to update runner status") + log.Error(err, "Failed to update runner status for Phase/Reason/Message") return ctrl.Result{}, err } } @@ -464,7 +464,7 @@ func (r *RunnerReconciler) updateRegistrationToken(ctx context.Context, runner v } if err := r.Status().Update(ctx, updated); err != nil { - log.Error(err, "Failed to update runner status") + log.Error(err, "Failed to update runner status for Registration") return false, err } From 3cccca8d097222a1022a16a9cd2e42bcdb1459b8 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 18 Mar 2021 10:31:17 +0900 Subject: [PATCH 3/3] Do patch runner status instead of update to reduce conflicts and avoid future bugs Ref https://github.com/summerwind/actions-runner-controller/pull/398#issuecomment-801548375 --- controllers/runner_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/runner_controller.go b/controllers/runner_controller.go index ff2d31ff..1dc9bb50 100644 --- a/controllers/runner_controller.go +++ b/controllers/runner_controller.go @@ -463,7 +463,7 @@ func (r *RunnerReconciler) updateRegistrationToken(ctx context.Context, runner v ExpiresAt: metav1.NewTime(rt.GetExpiresAt().Time), } - if err := r.Status().Update(ctx, updated); err != nil { + if err := r.Status().Patch(ctx, updated, client.MergeFrom(&runner)); err != nil { log.Error(err, "Failed to update runner status for Registration") return false, err }