Fix additionalPrinterColumns (#556)

This fixes human-readable output of `kubectl get` on `runnerdeployment`, `runnerreplicaset`, and `runner`.

Most notably, CURRENT and READY of runner replicasets are now computed and printed correctly. Runner deployments now have UP-TO-DATE and AVAILABLE instead of READY so that it is consistent with columns of K8s deployments.

A few fixes has been also made to runner deployment and runner replicaset controllers so that those numbers stored in Status objects are reliably updated and in-sync with actual values.

Finally, `AGE` columns are added to runnerdeployment, runnerreplicaset, runnner to make that more visible to users.

`kubectl get` outputs should now look like the below examples:

```
# Immediately after runnerdeployment updated/created
$ k get runnerdeployment
NAME                   DESIRED   CURRENT   UP-TO-DATE   AVAILABLE   AGE
example-runnerdeploy   0         0         0            0           8d
org-runnerdeploy       5         5         5            0           8d

# A few dozens of seconds after update/create all the runners are registered that "available" numbers increase
$ k get runnerdeployment
NAME                   DESIRED   CURRENT   UP-TO-DATE   AVAILABLE   AGE
example-runnerdeploy   0         0         0            0           8d
org-runnerdeploy       5         5         5            5           8d
```

```
$ k get runnerreplicaset
NAME                         DESIRED   CURRENT   READY   AGE
example-runnerdeploy-wnpf6   0         0         0       61m
org-runnerdeploy-fsnmr       2         2         0       8m41s
```

```
$ k get runner
NAME                                           ENTERPRISE   ORGANIZATION                REPOSITORY                                       LABELS                      STATUS    AGE
example-runnerdeploy-wnpf6-registration-only                                            actions-runner-controller/mumoshu-actions-test                               Running   61m
org-runnerdeploy-fsnmr-n8kkx                                actions-runner-controller                                                    ["mylabel 1","mylabel 2"]             21s
org-runnerdeploy-fsnmr-sq6m8                                actions-runner-controller                                                    ["mylabel 1","mylabel 2"]             21s
```

Fixes #490
This commit is contained in:
Yusuke Kuoka 2021-05-21 09:10:47 +09:00 committed by GitHub
parent a4631f345b
commit 0b88b246d3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 209 additions and 49 deletions

View File

@ -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 {

View File

@ -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 {

View File

@ -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 {

View File

@ -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.

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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
}
}

View File

@ -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 = &current
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