diff --git a/api/v1alpha1/runner_types.go b/api/v1alpha1/runner_types.go index 5cc46cc9..930ed52e 100644 --- a/api/v1alpha1/runner_types.go +++ b/api/v1alpha1/runner_types.go @@ -164,6 +164,7 @@ type RunnerStatusRegistration struct { // +kubebuilder:printcolumn:JSONPath=".spec.repository",name=Repository,type=string // +kubebuilder:printcolumn:JSONPath=".spec.labels",name=Labels,type=string // +kubebuilder:printcolumn:JSONPath=".status.phase",name=Status,type=string +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" // Runner is the Schema for the runners API type Runner struct { diff --git a/api/v1alpha1/runnerdeployment_types.go b/api/v1alpha1/runnerdeployment_types.go index 128de533..04fc3ed1 100644 --- a/api/v1alpha1/runnerdeployment_types.go +++ b/api/v1alpha1/runnerdeployment_types.go @@ -38,20 +38,41 @@ type RunnerDeploymentSpec struct { } type RunnerDeploymentStatus struct { - AvailableReplicas int `json:"availableReplicas"` - ReadyReplicas int `json:"readyReplicas"` + // See K8s deployment controller code for reference + // https://github.com/kubernetes/kubernetes/blob/ea0764452222146c47ec826977f49d7001b0ea8c/pkg/controller/deployment/sync.go#L487-L505 - // Replicas is the total number of desired, non-terminated and latest pods to be set for the primary RunnerSet + // AvailableReplicas is the total number of available runners which have been sucessfully registered to GitHub and still running. + // This corresponds to the sum of status.availableReplicas of all the runner replica sets. + // +optional + AvailableReplicas *int `json:"availableReplicas"` + + // ReadyReplicas is the total number of available runners which have been sucessfully registered to GitHub and still running. + // This corresponds to the sum of status.readyReplicas of all the runner replica sets. + // +optional + ReadyReplicas *int `json:"readyReplicas"` + + // ReadyReplicas is the total number of available runners which have been sucessfully registered to GitHub and still running. + // This corresponds to status.replicas of the runner replica set that has the desired template hash. + // +optional + UpdatedReplicas *int `json:"updatedReplicas"` + + // DesiredReplicas is the total number of desired, non-terminated and latest pods to be set for the primary RunnerSet // This doesn't include outdated pods while upgrading the deployment and replacing the runnerset. // +optional - Replicas *int `json:"desiredReplicas,omitempty"` + DesiredReplicas *int `json:"desiredReplicas"` + + // Replicas is the total number of replicas + // +optional + Replicas *int `json:"replicas"` } // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:printcolumn:JSONPath=".spec.replicas",name=Desired,type=number -// +kubebuilder:printcolumn:JSONPath=".status.availableReplicas",name=Current,type=number -// +kubebuilder:printcolumn:JSONPath=".status.readyReplicas",name=Ready,type=number +// +kubebuilder:printcolumn:JSONPath=".status.replicas",name=Current,type=number +// +kubebuilder:printcolumn:JSONPath=".status.updatedReplicas",name=Up-To-Date,type=number +// +kubebuilder:printcolumn:JSONPath=".status.availableReplicas",name=Available,type=number +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" // RunnerDeployment is the Schema for the runnerdeployments API type RunnerDeployment struct { diff --git a/api/v1alpha1/runnerreplicaset_types.go b/api/v1alpha1/runnerreplicaset_types.go index 4c755a0d..cbad9c96 100644 --- a/api/v1alpha1/runnerreplicaset_types.go +++ b/api/v1alpha1/runnerreplicaset_types.go @@ -33,8 +33,19 @@ type RunnerReplicaSetSpec struct { } type RunnerReplicaSetStatus struct { - AvailableReplicas int `json:"availableReplicas"` - ReadyReplicas int `json:"readyReplicas"` + // See K8s replicaset controller code for reference + // https://github.com/kubernetes/kubernetes/blob/ea0764452222146c47ec826977f49d7001b0ea8c/pkg/controller/replicaset/replica_set_utils.go#L101-L106 + + // Replicas is the number of runners that are created and still being managed by this runner replica set. + // +optional + Replicas *int `json:"replicas"` + + // ReadyReplicas is the number of runners that are created and Runnning. + ReadyReplicas *int `json:"readyReplicas"` + + // AvailableReplicas is the number of runners that are created and Runnning. + // This is currently same as ReadyReplicas but perserved for future use. + AvailableReplicas *int `json:"availableReplicas"` } type RunnerTemplate struct { @@ -46,8 +57,9 @@ type RunnerTemplate struct { // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:printcolumn:JSONPath=".spec.replicas",name=Desired,type=number -// +kubebuilder:printcolumn:JSONPath=".status.availableReplicas",name=Current,type=number +// +kubebuilder:printcolumn:JSONPath=".status.replicas",name=Current,type=number // +kubebuilder:printcolumn:JSONPath=".status.readyReplicas",name=Ready,type=number +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" // RunnerReplicaSet is the Schema for the runnerreplicasets API type RunnerReplicaSet struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 336ce61f..9d2255b6 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -453,6 +453,26 @@ func (in *RunnerDeploymentSpec) DeepCopy() *RunnerDeploymentSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RunnerDeploymentStatus) DeepCopyInto(out *RunnerDeploymentStatus) { *out = *in + if in.AvailableReplicas != nil { + in, out := &in.AvailableReplicas, &out.AvailableReplicas + *out = new(int) + **out = **in + } + if in.ReadyReplicas != nil { + in, out := &in.ReadyReplicas, &out.ReadyReplicas + *out = new(int) + **out = **in + } + if in.UpdatedReplicas != nil { + in, out := &in.UpdatedReplicas, &out.UpdatedReplicas + *out = new(int) + **out = **in + } + if in.DesiredReplicas != nil { + in, out := &in.DesiredReplicas, &out.DesiredReplicas + *out = new(int) + **out = **in + } if in.Replicas != nil { in, out := &in.Replicas, &out.Replicas *out = new(int) @@ -508,7 +528,7 @@ func (in *RunnerReplicaSet) DeepCopyInto(out *RunnerReplicaSet) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RunnerReplicaSet. @@ -590,6 +610,21 @@ func (in *RunnerReplicaSetSpec) DeepCopy() *RunnerReplicaSetSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RunnerReplicaSetStatus) DeepCopyInto(out *RunnerReplicaSetStatus) { *out = *in + if in.Replicas != nil { + in, out := &in.Replicas, &out.Replicas + *out = new(int) + **out = **in + } + if in.ReadyReplicas != nil { + in, out := &in.ReadyReplicas, &out.ReadyReplicas + *out = new(int) + **out = **in + } + if in.AvailableReplicas != nil { + in, out := &in.AvailableReplicas, &out.AvailableReplicas + *out = new(int) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RunnerReplicaSetStatus. diff --git a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml index f33b9170..7c2f0ae5 100644 --- a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml +++ b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml @@ -10,12 +10,18 @@ spec: - JSONPath: .spec.replicas name: Desired type: number - - JSONPath: .status.availableReplicas + - JSONPath: .status.replicas name: Current type: number - - JSONPath: .status.readyReplicas - name: Ready + - JSONPath: .status.updatedReplicas + name: Up-To-Date type: number + - JSONPath: .status.availableReplicas + name: Available + type: number + - JSONPath: .metadata.creationTimestamp + name: Age + type: date group: actions.summerwind.dev names: kind: RunnerDeployment @@ -1631,15 +1637,20 @@ spec: status: properties: availableReplicas: + description: AvailableReplicas is the total number of available runners which have been sucessfully registered to GitHub and still running. This corresponds to the sum of status.availableReplicas of all the runner replica sets. type: integer desiredReplicas: - description: Replicas is the total number of desired, non-terminated and latest pods to be set for the primary RunnerSet This doesn't include outdated pods while upgrading the deployment and replacing the runnerset. + description: DesiredReplicas is the total number of desired, non-terminated and latest pods to be set for the primary RunnerSet This doesn't include outdated pods while upgrading the deployment and replacing the runnerset. type: integer readyReplicas: + description: ReadyReplicas is the total number of available runners which have been sucessfully registered to GitHub and still running. This corresponds to the sum of status.readyReplicas of all the runner replica sets. + type: integer + replicas: + description: Replicas is the total number of replicas + type: integer + updatedReplicas: + description: ReadyReplicas is the total number of available runners which have been sucessfully registered to GitHub and still running. This corresponds to status.replicas of the runner replica set that has the desired template hash. type: integer - required: - - availableReplicas - - readyReplicas type: object type: object version: v1alpha1 diff --git a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerreplicasets.yaml b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerreplicasets.yaml index 39d279f2..f859dec9 100644 --- a/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerreplicasets.yaml +++ b/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerreplicasets.yaml @@ -10,12 +10,15 @@ spec: - JSONPath: .spec.replicas name: Desired type: number - - JSONPath: .status.availableReplicas + - JSONPath: .status.replicas name: Current type: number - JSONPath: .status.readyReplicas name: Ready type: number + - JSONPath: .metadata.creationTimestamp + name: Age + type: date group: actions.summerwind.dev names: kind: RunnerReplicaSet @@ -1631,8 +1634,13 @@ spec: status: properties: availableReplicas: + description: AvailableReplicas is the number of runners that are created and Runnning. This is currently same as ReadyReplicas but perserved for future use. type: integer readyReplicas: + description: ReadyReplicas is the number of runners that are created and Runnning. + type: integer + replicas: + description: Replicas is the number of runners that are created and still being managed by this runner replica set. type: integer required: - availableReplicas diff --git a/charts/actions-runner-controller/crds/actions.summerwind.dev_runners.yaml b/charts/actions-runner-controller/crds/actions.summerwind.dev_runners.yaml index 5b8f0355..c8ff9711 100644 --- a/charts/actions-runner-controller/crds/actions.summerwind.dev_runners.yaml +++ b/charts/actions-runner-controller/crds/actions.summerwind.dev_runners.yaml @@ -22,6 +22,9 @@ spec: - JSONPath: .status.phase name: Status type: string + - JSONPath: .metadata.creationTimestamp + name: Age + type: date group: actions.summerwind.dev names: kind: Runner diff --git a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml b/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml index f33b9170..7c2f0ae5 100644 --- a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml +++ b/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml @@ -10,12 +10,18 @@ spec: - JSONPath: .spec.replicas name: Desired type: number - - JSONPath: .status.availableReplicas + - JSONPath: .status.replicas name: Current type: number - - JSONPath: .status.readyReplicas - name: Ready + - JSONPath: .status.updatedReplicas + name: Up-To-Date type: number + - JSONPath: .status.availableReplicas + name: Available + type: number + - JSONPath: .metadata.creationTimestamp + name: Age + type: date group: actions.summerwind.dev names: kind: RunnerDeployment @@ -1631,15 +1637,20 @@ spec: status: properties: availableReplicas: + description: AvailableReplicas is the total number of available runners which have been sucessfully registered to GitHub and still running. This corresponds to the sum of status.availableReplicas of all the runner replica sets. type: integer desiredReplicas: - description: Replicas is the total number of desired, non-terminated and latest pods to be set for the primary RunnerSet This doesn't include outdated pods while upgrading the deployment and replacing the runnerset. + description: DesiredReplicas is the total number of desired, non-terminated and latest pods to be set for the primary RunnerSet This doesn't include outdated pods while upgrading the deployment and replacing the runnerset. type: integer readyReplicas: + description: ReadyReplicas is the total number of available runners which have been sucessfully registered to GitHub and still running. This corresponds to the sum of status.readyReplicas of all the runner replica sets. + type: integer + replicas: + description: Replicas is the total number of replicas + type: integer + updatedReplicas: + description: ReadyReplicas is the total number of available runners which have been sucessfully registered to GitHub and still running. This corresponds to status.replicas of the runner replica set that has the desired template hash. type: integer - required: - - availableReplicas - - readyReplicas type: object type: object version: v1alpha1 diff --git a/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml b/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml index 39d279f2..f859dec9 100644 --- a/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml +++ b/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml @@ -10,12 +10,15 @@ spec: - JSONPath: .spec.replicas name: Desired type: number - - JSONPath: .status.availableReplicas + - JSONPath: .status.replicas name: Current type: number - JSONPath: .status.readyReplicas name: Ready type: number + - JSONPath: .metadata.creationTimestamp + name: Age + type: date group: actions.summerwind.dev names: kind: RunnerReplicaSet @@ -1631,8 +1634,13 @@ spec: status: properties: availableReplicas: + description: AvailableReplicas is the number of runners that are created and Runnning. This is currently same as ReadyReplicas but perserved for future use. type: integer readyReplicas: + description: ReadyReplicas is the number of runners that are created and Runnning. + type: integer + replicas: + description: Replicas is the number of runners that are created and still being managed by this runner replica set. type: integer required: - availableReplicas diff --git a/config/crd/bases/actions.summerwind.dev_runners.yaml b/config/crd/bases/actions.summerwind.dev_runners.yaml index 5b8f0355..c8ff9711 100644 --- a/config/crd/bases/actions.summerwind.dev_runners.yaml +++ b/config/crd/bases/actions.summerwind.dev_runners.yaml @@ -22,6 +22,9 @@ spec: - JSONPath: .status.phase name: Status type: string + - JSONPath: .metadata.creationTimestamp + name: Age + type: date group: actions.summerwind.dev names: kind: Runner diff --git a/controllers/runnerdeployment_controller.go b/controllers/runnerdeployment_controller.go index 37f36b07..038f9862 100644 --- a/controllers/runnerdeployment_controller.go +++ b/controllers/runnerdeployment_controller.go @@ -190,7 +190,10 @@ func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e // Do we have old runner replica sets that should eventually deleted? if len(oldSets) > 0 { - readyReplicas := newestSet.Status.ReadyReplicas + var readyReplicas int + if newestSet.Status.ReadyReplicas != nil { + readyReplicas = *newestSet.Status.ReadyReplicas + } oldSetsCount := len(oldSets) @@ -231,14 +234,49 @@ func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e } } - if rd.Spec.Replicas == nil && desiredRS.Spec.Replicas != nil { + var replicaSets []v1alpha1.RunnerReplicaSet + + replicaSets = append(replicaSets, *newestSet) + replicaSets = append(replicaSets, oldSets...) + + var totalCurrentReplicas, totalStatusAvailableReplicas, updatedReplicas int + + for _, rs := range replicaSets { + var current, available int + + if rs.Status.Replicas != nil { + current = *rs.Status.Replicas + } + + if rs.Status.AvailableReplicas != nil { + available = *rs.Status.AvailableReplicas + } + + totalCurrentReplicas += current + totalStatusAvailableReplicas += available + } + + if newestSet.Status.Replicas != nil { + updatedReplicas = *newestSet.Status.Replicas + } + + var status v1alpha1.RunnerDeploymentStatus + + status.AvailableReplicas = &totalStatusAvailableReplicas + status.ReadyReplicas = &totalStatusAvailableReplicas + status.DesiredReplicas = &newDesiredReplicas + status.Replicas = &totalCurrentReplicas + status.UpdatedReplicas = &updatedReplicas + + if !reflect.DeepEqual(rd.Status, status) { updated := rd.DeepCopy() - updated.Status.Replicas = desiredRS.Spec.Replicas + updated.Status = status - if err := r.Status().Update(ctx, updated); err != nil { - log.Error(err, "Failed to update runnerdeployment status") - - return ctrl.Result{}, err + if err := r.Status().Patch(ctx, updated, client.MergeFrom(&rd)); err != nil { + log.Info("Failed to patch runnerdeployment status. Retrying immediately", "error", err.Error()) + return ctrl.Result{ + Requeue: true, + }, nil } } diff --git a/controllers/runnerreplicaset_controller.go b/controllers/runnerreplicaset_controller.go index a9095ead..dce0c440 100644 --- a/controllers/runnerreplicaset_controller.go +++ b/controllers/runnerreplicaset_controller.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "reflect" "time" gogithub "github.com/google/go-github/v33/github" @@ -88,8 +89,9 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e var myRunners []v1alpha1.Runner var ( - available int + current int ready int + available int ) for _, r := range allRunners.Items { @@ -98,10 +100,12 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e if metav1.IsControlledBy(&r, &rs) && !metav1.HasAnnotation(r.ObjectMeta, annotationKeyRegistrationOnly) { myRunners = append(myRunners, r) - available += 1 + current += 1 if r.Status.Phase == string(corev1.PodRunning) { ready += 1 + // available is currently the same as ready, as we don't yet have minReadySeconds for runners + available += 1 } } } @@ -136,7 +140,7 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e // In this case We don't need to bother creating a registration-only runner which gets deleted soon after we have 1 or more available repolicas, // hence it's not `available == 0`, but `registrationOnlyRunnerExists && available == 0`. // See https://github.com/actions-runner-controller/actions-runner-controller/issues/516 - registrationOnlyRunnerNeeded := desired == 0 || (registrationOnlyRunnerExists && available == 0) + registrationOnlyRunnerNeeded := desired == 0 || (registrationOnlyRunnerExists && current == 0) if registrationOnlyRunnerNeeded { if registrationOnlyRunnerExists { @@ -179,10 +183,10 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e } } - if available > desired { - n := available - desired + if current > desired { + n := current - desired - log.V(0).Info(fmt.Sprintf("Deleting %d runners", n), "desired", desired, "available", available, "ready", ready) + log.V(0).Info(fmt.Sprintf("Deleting %d runners", n), "desired", desired, "current", current, "ready", ready) // get runners that are currently offline/not busy/timed-out to register var deletionCandidates []v1alpha1.Runner @@ -250,7 +254,7 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e n = len(deletionCandidates) } - log.V(0).Info(fmt.Sprintf("Deleting %d runner(s)", n), "desired", desired, "available", available, "ready", ready) + log.V(0).Info(fmt.Sprintf("Deleting %d runner(s)", n), "desired", desired, "current", current, "ready", ready) for i := 0; i < n; i++ { if err := r.Client.Delete(ctx, &deletionCandidates[i]); client.IgnoreNotFound(err) != nil { @@ -262,10 +266,10 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e r.Recorder.Event(&rs, corev1.EventTypeNormal, "RunnerDeleted", fmt.Sprintf("Deleted runner '%s'", deletionCandidates[i].Name)) log.Info("Deleted runner") } - } else if desired > available { - n := desired - available + } else if desired > current { + n := desired - current - log.V(0).Info(fmt.Sprintf("Creating %d runner(s)", n), "desired", desired, "available", available, "ready", ready) + log.V(0).Info(fmt.Sprintf("Creating %d runner(s)", n), "desired", desired, "available", current, "ready", ready) for i := 0; i < n; i++ { newRunner, err := r.newRunner(rs) @@ -283,13 +287,18 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e } } - if rs.Status.AvailableReplicas != available || rs.Status.ReadyReplicas != ready { - updated := rs.DeepCopy() - updated.Status.AvailableReplicas = available - updated.Status.ReadyReplicas = ready + var status v1alpha1.RunnerReplicaSetStatus - if err := r.Status().Update(ctx, updated); err != nil { - log.Info("Failed to update status. Retrying immediately", "error", err.Error()) + status.Replicas = ¤t + status.AvailableReplicas = &available + status.ReadyReplicas = &ready + + if !reflect.DeepEqual(rs.Status, status) { + updated := rs.DeepCopy() + updated.Status = status + + if err := r.Status().Patch(ctx, updated, client.MergeFrom(&rs)); err != nil { + log.Info("Failed to update runnerreplicaset status. Retrying immediately", "error", err.Error()) return ctrl.Result{ Requeue: true, }, nil