Manage runner with label (#355)
* Update RunnerDeploymentSpec to have Selector field Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Update RunnerReplicaSetSpec to have Selector field Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Add CloneSelectorAndAddLabel to add Selector field Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Fix tests Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Use label to find RunnerReplicaSet/Runner Signed-off-by: binoue <banji-inoue@cybozu.co.jp> * Update controller-gen versions in CRD Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Update autoscaler to list Pods with labels Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Add debug log Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Modify RunnerDeployment tests Signed-off-by: binoue <banji-inoue@cybozu.co.jp> * Modify RunnerReplicaset test Signed-off-by: binoue <banji-inoue@cybozu.co.jp> * Modify integration test Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Use RunnerDeployment Template Labels as the default selector for backward compatibility * Fix labeling Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Update func in Eventually to return (int, error) Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Update RunnerDeployment controller not to use label selector Signed-off-by: Hiroshi Muraoka <h.muraoka714@gmail.com> * Fix potential replicaset controller breakage on replicaset created before v0.17.0 * Fix errors on existing runner replica sets * Ensure RunnerReplicaSet Spec Selector addition does not break controller * Ensure RunnerDeployment Template.Spec.Labels change does result in template hash change * Fix comment Co-authored-by: binoue <banji-inoue@cybozu.co.jp> Co-authored-by: Yusuke Kuoka <ykuoka@gmail.com>
This commit is contained in:
		
							parent
							
								
									f220fefe92
								
							
						
					
					
						commit
						11e58fcc41
					
				|  | @ -25,12 +25,15 @@ const ( | ||||||
| 	AutoscalingMetricTypePercentageRunnersBusy                        = "PercentageRunnersBusy" | 	AutoscalingMetricTypePercentageRunnersBusy                        = "PercentageRunnersBusy" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
| // RunnerReplicaSetSpec defines the desired state of RunnerDeployment
 | // RunnerDeploymentSpec defines the desired state of RunnerDeployment
 | ||||||
| type RunnerDeploymentSpec struct { | type RunnerDeploymentSpec struct { | ||||||
| 	// +optional
 | 	// +optional
 | ||||||
| 	// +nullable
 | 	// +nullable
 | ||||||
| 	Replicas *int `json:"replicas,omitempty"` | 	Replicas *int `json:"replicas,omitempty"` | ||||||
| 
 | 
 | ||||||
|  | 	// +optional
 | ||||||
|  | 	// +nullable
 | ||||||
|  | 	Selector *metav1.LabelSelector `json:"selector"` | ||||||
| 	Template RunnerTemplate        `json:"template"` | 	Template RunnerTemplate        `json:"template"` | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -26,6 +26,9 @@ type RunnerReplicaSetSpec struct { | ||||||
| 	// +nullable
 | 	// +nullable
 | ||||||
| 	Replicas *int `json:"replicas,omitempty"` | 	Replicas *int `json:"replicas,omitempty"` | ||||||
| 
 | 
 | ||||||
|  | 	// +optional
 | ||||||
|  | 	// +nullable
 | ||||||
|  | 	Selector *metav1.LabelSelector `json:"selector"` | ||||||
| 	Template RunnerTemplate        `json:"template"` | 	Template RunnerTemplate        `json:"template"` | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -22,6 +22,7 @@ package v1alpha1 | ||||||
| 
 | 
 | ||||||
| import ( | import ( | ||||||
| 	"k8s.io/api/core/v1" | 	"k8s.io/api/core/v1" | ||||||
|  | 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||
| 	"k8s.io/apimachinery/pkg/runtime" | 	"k8s.io/apimachinery/pkg/runtime" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
|  | @ -403,6 +404,11 @@ func (in *RunnerDeploymentSpec) DeepCopyInto(out *RunnerDeploymentSpec) { | ||||||
| 		*out = new(int) | 		*out = new(int) | ||||||
| 		**out = **in | 		**out = **in | ||||||
| 	} | 	} | ||||||
|  | 	if in.Selector != nil { | ||||||
|  | 		in, out := &in.Selector, &out.Selector | ||||||
|  | 		*out = new(metav1.LabelSelector) | ||||||
|  | 		(*in).DeepCopyInto(*out) | ||||||
|  | 	} | ||||||
| 	in.Template.DeepCopyInto(&out.Template) | 	in.Template.DeepCopyInto(&out.Template) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -535,6 +541,11 @@ func (in *RunnerReplicaSetSpec) DeepCopyInto(out *RunnerReplicaSetSpec) { | ||||||
| 		*out = new(int) | 		*out = new(int) | ||||||
| 		**out = **in | 		**out = **in | ||||||
| 	} | 	} | ||||||
|  | 	if in.Selector != nil { | ||||||
|  | 		in, out := &in.Selector, &out.Selector | ||||||
|  | 		*out = new(metav1.LabelSelector) | ||||||
|  | 		(*in).DeepCopyInto(*out) | ||||||
|  | 	} | ||||||
| 	in.Template.DeepCopyInto(&out.Template) | 	in.Template.DeepCopyInto(&out.Template) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -38,11 +38,42 @@ spec: | ||||||
|         metadata: |         metadata: | ||||||
|           type: object |           type: object | ||||||
|         spec: |         spec: | ||||||
|           description: RunnerReplicaSetSpec defines the desired state of RunnerDeployment |           description: RunnerDeploymentSpec defines the desired state of RunnerDeployment | ||||||
|           properties: |           properties: | ||||||
|             replicas: |             replicas: | ||||||
|               nullable: true |               nullable: true | ||||||
|               type: integer |               type: integer | ||||||
|  |             selector: | ||||||
|  |               description: A label selector is a label query over a set of resources. The result of matchLabels and matchExpressions are ANDed. An empty label selector matches all objects. A null label selector matches no objects. | ||||||
|  |               nullable: true | ||||||
|  |               properties: | ||||||
|  |                 matchExpressions: | ||||||
|  |                   description: matchExpressions is a list of label selector requirements. The requirements are ANDed. | ||||||
|  |                   items: | ||||||
|  |                     description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. | ||||||
|  |                     properties: | ||||||
|  |                       key: | ||||||
|  |                         description: key is the label key that the selector applies to. | ||||||
|  |                         type: string | ||||||
|  |                       operator: | ||||||
|  |                         description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. | ||||||
|  |                         type: string | ||||||
|  |                       values: | ||||||
|  |                         description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. | ||||||
|  |                         items: | ||||||
|  |                           type: string | ||||||
|  |                         type: array | ||||||
|  |                     required: | ||||||
|  |                       - key | ||||||
|  |                       - operator | ||||||
|  |                     type: object | ||||||
|  |                   type: array | ||||||
|  |                 matchLabels: | ||||||
|  |                   additionalProperties: | ||||||
|  |                     type: string | ||||||
|  |                   description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. | ||||||
|  |                   type: object | ||||||
|  |               type: object | ||||||
|             template: |             template: | ||||||
|               properties: |               properties: | ||||||
|                 metadata: |                 metadata: | ||||||
|  |  | ||||||
|  | @ -43,6 +43,37 @@ spec: | ||||||
|             replicas: |             replicas: | ||||||
|               nullable: true |               nullable: true | ||||||
|               type: integer |               type: integer | ||||||
|  |             selector: | ||||||
|  |               description: A label selector is a label query over a set of resources. The result of matchLabels and matchExpressions are ANDed. An empty label selector matches all objects. A null label selector matches no objects. | ||||||
|  |               nullable: true | ||||||
|  |               properties: | ||||||
|  |                 matchExpressions: | ||||||
|  |                   description: matchExpressions is a list of label selector requirements. The requirements are ANDed. | ||||||
|  |                   items: | ||||||
|  |                     description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. | ||||||
|  |                     properties: | ||||||
|  |                       key: | ||||||
|  |                         description: key is the label key that the selector applies to. | ||||||
|  |                         type: string | ||||||
|  |                       operator: | ||||||
|  |                         description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. | ||||||
|  |                         type: string | ||||||
|  |                       values: | ||||||
|  |                         description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. | ||||||
|  |                         items: | ||||||
|  |                           type: string | ||||||
|  |                         type: array | ||||||
|  |                     required: | ||||||
|  |                       - key | ||||||
|  |                       - operator | ||||||
|  |                     type: object | ||||||
|  |                   type: array | ||||||
|  |                 matchLabels: | ||||||
|  |                   additionalProperties: | ||||||
|  |                     type: string | ||||||
|  |                   description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. | ||||||
|  |                   type: object | ||||||
|  |               type: object | ||||||
|             template: |             template: | ||||||
|               properties: |               properties: | ||||||
|                 metadata: |                 metadata: | ||||||
|  |  | ||||||
|  | @ -38,11 +38,42 @@ spec: | ||||||
|         metadata: |         metadata: | ||||||
|           type: object |           type: object | ||||||
|         spec: |         spec: | ||||||
|           description: RunnerReplicaSetSpec defines the desired state of RunnerDeployment |           description: RunnerDeploymentSpec defines the desired state of RunnerDeployment | ||||||
|           properties: |           properties: | ||||||
|             replicas: |             replicas: | ||||||
|               nullable: true |               nullable: true | ||||||
|               type: integer |               type: integer | ||||||
|  |             selector: | ||||||
|  |               description: A label selector is a label query over a set of resources. The result of matchLabels and matchExpressions are ANDed. An empty label selector matches all objects. A null label selector matches no objects. | ||||||
|  |               nullable: true | ||||||
|  |               properties: | ||||||
|  |                 matchExpressions: | ||||||
|  |                   description: matchExpressions is a list of label selector requirements. The requirements are ANDed. | ||||||
|  |                   items: | ||||||
|  |                     description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. | ||||||
|  |                     properties: | ||||||
|  |                       key: | ||||||
|  |                         description: key is the label key that the selector applies to. | ||||||
|  |                         type: string | ||||||
|  |                       operator: | ||||||
|  |                         description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. | ||||||
|  |                         type: string | ||||||
|  |                       values: | ||||||
|  |                         description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. | ||||||
|  |                         items: | ||||||
|  |                           type: string | ||||||
|  |                         type: array | ||||||
|  |                     required: | ||||||
|  |                       - key | ||||||
|  |                       - operator | ||||||
|  |                     type: object | ||||||
|  |                   type: array | ||||||
|  |                 matchLabels: | ||||||
|  |                   additionalProperties: | ||||||
|  |                     type: string | ||||||
|  |                   description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. | ||||||
|  |                   type: object | ||||||
|  |               type: object | ||||||
|             template: |             template: | ||||||
|               properties: |               properties: | ||||||
|                 metadata: |                 metadata: | ||||||
|  |  | ||||||
|  | @ -43,6 +43,37 @@ spec: | ||||||
|             replicas: |             replicas: | ||||||
|               nullable: true |               nullable: true | ||||||
|               type: integer |               type: integer | ||||||
|  |             selector: | ||||||
|  |               description: A label selector is a label query over a set of resources. The result of matchLabels and matchExpressions are ANDed. An empty label selector matches all objects. A null label selector matches no objects. | ||||||
|  |               nullable: true | ||||||
|  |               properties: | ||||||
|  |                 matchExpressions: | ||||||
|  |                   description: matchExpressions is a list of label selector requirements. The requirements are ANDed. | ||||||
|  |                   items: | ||||||
|  |                     description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. | ||||||
|  |                     properties: | ||||||
|  |                       key: | ||||||
|  |                         description: key is the label key that the selector applies to. | ||||||
|  |                         type: string | ||||||
|  |                       operator: | ||||||
|  |                         description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. | ||||||
|  |                         type: string | ||||||
|  |                       values: | ||||||
|  |                         description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. | ||||||
|  |                         items: | ||||||
|  |                           type: string | ||||||
|  |                         type: array | ||||||
|  |                     required: | ||||||
|  |                       - key | ||||||
|  |                       - operator | ||||||
|  |                     type: object | ||||||
|  |                   type: array | ||||||
|  |                 matchLabels: | ||||||
|  |                   additionalProperties: | ||||||
|  |                     type: string | ||||||
|  |                   description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. | ||||||
|  |                   type: object | ||||||
|  |               type: object | ||||||
|             template: |             template: | ||||||
|               properties: |               properties: | ||||||
|                 metadata: |                 metadata: | ||||||
|  |  | ||||||
|  | @ -10,6 +10,8 @@ import ( | ||||||
| 	"time" | 	"time" | ||||||
| 
 | 
 | ||||||
| 	"github.com/summerwind/actions-runner-controller/api/v1alpha1" | 	"github.com/summerwind/actions-runner-controller/api/v1alpha1" | ||||||
|  | 	kerrors "k8s.io/apimachinery/pkg/api/errors" | ||||||
|  | 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||
| 	"sigs.k8s.io/controller-runtime/pkg/client" | 	"sigs.k8s.io/controller-runtime/pkg/client" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
|  | @ -257,11 +259,23 @@ func (r *HorizontalRunnerAutoscalerReconciler) calculateReplicasByPercentageRunn | ||||||
| 		scaleDownFactor = sdf | 		scaleDownFactor = sdf | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	// return the list of runners in namespace. Horizontal Runner Autoscaler should only be responsible for scaling resources in its own ns.
 | 	selector, err := metav1.LabelSelectorAsSelector(rd.Spec.Selector) | ||||||
| 	var runnerList v1alpha1.RunnerList | 	if err != nil { | ||||||
| 	if err := r.List(ctx, &runnerList, client.InNamespace(rd.Namespace)); err != nil { |  | ||||||
| 		return nil, err | 		return nil, err | ||||||
| 	} | 	} | ||||||
|  | 	// return the list of runners in namespace. Horizontal Runner Autoscaler should only be responsible for scaling resources in its own ns.
 | ||||||
|  | 	var runnerList v1alpha1.RunnerList | ||||||
|  | 	if err := r.List( | ||||||
|  | 		ctx, | ||||||
|  | 		&runnerList, | ||||||
|  | 		client.InNamespace(rd.Namespace), | ||||||
|  | 		client.MatchingLabelsSelector{Selector: selector}, | ||||||
|  | 	); err != nil { | ||||||
|  | 		if !kerrors.IsNotFound(err) { | ||||||
|  | 			return nil, err | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
| 	runnerMap := make(map[string]struct{}) | 	runnerMap := make(map[string]struct{}) | ||||||
| 	for _, items := range runnerList.Items { | 	for _, items := range runnerList.Items { | ||||||
| 		runnerMap[items.Name] = struct{}{} | 		runnerMap[items.Name] = struct{}{} | ||||||
|  |  | ||||||
|  | @ -443,7 +443,17 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) { | ||||||
| 					Name: "testrd", | 					Name: "testrd", | ||||||
| 				}, | 				}, | ||||||
| 				Spec: v1alpha1.RunnerDeploymentSpec{ | 				Spec: v1alpha1.RunnerDeploymentSpec{ | ||||||
|  | 					Selector: &metav1.LabelSelector{ | ||||||
|  | 						MatchLabels: map[string]string{ | ||||||
|  | 							"foo": "bar", | ||||||
|  | 						}, | ||||||
|  | 					}, | ||||||
| 					Template: v1alpha1.RunnerTemplate{ | 					Template: v1alpha1.RunnerTemplate{ | ||||||
|  | 						ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 							Labels: map[string]string{ | ||||||
|  | 								"foo": "bar", | ||||||
|  | 							}, | ||||||
|  | 						}, | ||||||
| 						Spec: v1alpha1.RunnerSpec{ | 						Spec: v1alpha1.RunnerSpec{ | ||||||
| 							Organization: tc.org, | 							Organization: tc.org, | ||||||
| 						}, | 						}, | ||||||
|  |  | ||||||
|  | @ -3,13 +3,14 @@ package controllers | ||||||
| import ( | import ( | ||||||
| 	"context" | 	"context" | ||||||
| 	"fmt" | 	"fmt" | ||||||
| 	"github.com/google/go-github/v33/github" |  | ||||||
| 	github2 "github.com/summerwind/actions-runner-controller/github" |  | ||||||
| 	"k8s.io/apimachinery/pkg/runtime" |  | ||||||
| 	"net/http" | 	"net/http" | ||||||
| 	"net/http/httptest" | 	"net/http/httptest" | ||||||
| 	"time" | 	"time" | ||||||
| 
 | 
 | ||||||
|  | 	"github.com/google/go-github/v33/github" | ||||||
|  | 	github2 "github.com/summerwind/actions-runner-controller/github" | ||||||
|  | 	"k8s.io/apimachinery/pkg/runtime" | ||||||
|  | 
 | ||||||
| 	"github.com/summerwind/actions-runner-controller/github/fake" | 	"github.com/summerwind/actions-runner-controller/github/fake" | ||||||
| 
 | 
 | ||||||
| 	corev1 "k8s.io/api/core/v1" | 	corev1 "k8s.io/api/core/v1" | ||||||
|  | @ -184,7 +185,17 @@ var _ = Context("INTEGRATION: Inside of a new namespace", func() { | ||||||
| 					}, | 					}, | ||||||
| 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | ||||||
| 						Replicas: intPtr(1), | 						Replicas: intPtr(1), | ||||||
|  | 						Selector: &metav1.LabelSelector{ | ||||||
|  | 							MatchLabels: map[string]string{ | ||||||
|  | 								"foo": "bar", | ||||||
|  | 							}, | ||||||
|  | 						}, | ||||||
| 						Template: actionsv1alpha1.RunnerTemplate{ | 						Template: actionsv1alpha1.RunnerTemplate{ | ||||||
|  | 							ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 								Labels: map[string]string{ | ||||||
|  | 									"foo": "bar", | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
| 							Spec: actionsv1alpha1.RunnerSpec{ | 							Spec: actionsv1alpha1.RunnerSpec{ | ||||||
| 								Repository: "test/valid", | 								Repository: "test/valid", | ||||||
| 								Image:      "bar", | 								Image:      "bar", | ||||||
|  | @ -301,7 +312,17 @@ var _ = Context("INTEGRATION: Inside of a new namespace", func() { | ||||||
| 					}, | 					}, | ||||||
| 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | ||||||
| 						Replicas: intPtr(1), | 						Replicas: intPtr(1), | ||||||
|  | 						Selector: &metav1.LabelSelector{ | ||||||
|  | 							MatchLabels: map[string]string{ | ||||||
|  | 								"foo": "bar", | ||||||
|  | 							}, | ||||||
|  | 						}, | ||||||
| 						Template: actionsv1alpha1.RunnerTemplate{ | 						Template: actionsv1alpha1.RunnerTemplate{ | ||||||
|  | 							ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 								Labels: map[string]string{ | ||||||
|  | 									"foo": "bar", | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
| 							Spec: actionsv1alpha1.RunnerSpec{ | 							Spec: actionsv1alpha1.RunnerSpec{ | ||||||
| 								Repository: "test/valid", | 								Repository: "test/valid", | ||||||
| 								Image:      "bar", | 								Image:      "bar", | ||||||
|  | @ -396,7 +417,17 @@ var _ = Context("INTEGRATION: Inside of a new namespace", func() { | ||||||
| 					}, | 					}, | ||||||
| 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | ||||||
| 						Replicas: intPtr(1), | 						Replicas: intPtr(1), | ||||||
|  | 						Selector: &metav1.LabelSelector{ | ||||||
|  | 							MatchLabels: map[string]string{ | ||||||
|  | 								"foo": "bar", | ||||||
|  | 							}, | ||||||
|  | 						}, | ||||||
| 						Template: actionsv1alpha1.RunnerTemplate{ | 						Template: actionsv1alpha1.RunnerTemplate{ | ||||||
|  | 							ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 								Labels: map[string]string{ | ||||||
|  | 									"foo": "bar", | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
| 							Spec: actionsv1alpha1.RunnerSpec{ | 							Spec: actionsv1alpha1.RunnerSpec{ | ||||||
| 								Repository: "test/valid", | 								Repository: "test/valid", | ||||||
| 								Image:      "bar", | 								Image:      "bar", | ||||||
|  | @ -515,7 +546,17 @@ var _ = Context("INTEGRATION: Inside of a new namespace", func() { | ||||||
| 					}, | 					}, | ||||||
| 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | ||||||
| 						Replicas: intPtr(1), | 						Replicas: intPtr(1), | ||||||
|  | 						Selector: &metav1.LabelSelector{ | ||||||
|  | 							MatchLabels: map[string]string{ | ||||||
|  | 								"foo": "bar", | ||||||
|  | 							}, | ||||||
|  | 						}, | ||||||
| 						Template: actionsv1alpha1.RunnerTemplate{ | 						Template: actionsv1alpha1.RunnerTemplate{ | ||||||
|  | 							ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 								Labels: map[string]string{ | ||||||
|  | 									"foo": "bar", | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
| 							Spec: actionsv1alpha1.RunnerSpec{ | 							Spec: actionsv1alpha1.RunnerSpec{ | ||||||
| 								Repository: "test/valid", | 								Repository: "test/valid", | ||||||
| 								Image:      "bar", | 								Image:      "bar", | ||||||
|  |  | ||||||
|  | @ -20,6 +20,7 @@ import ( | ||||||
| 	"context" | 	"context" | ||||||
| 	"fmt" | 	"fmt" | ||||||
| 	"hash/fnv" | 	"hash/fnv" | ||||||
|  | 	"reflect" | ||||||
| 	"sort" | 	"sort" | ||||||
| 	"time" | 	"time" | ||||||
| 
 | 
 | ||||||
|  | @ -143,6 +144,28 @@ func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e | ||||||
| 		return ctrl.Result{RequeueAfter: 5 * time.Second}, nil | 		return ctrl.Result{RequeueAfter: 5 * time.Second}, nil | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 	if !reflect.DeepEqual(newestSet.Spec.Selector, desiredRS.Spec.Selector) { | ||||||
|  | 		updateSet := newestSet.DeepCopy() | ||||||
|  | 		updateSet.Spec = *desiredRS.Spec.DeepCopy() | ||||||
|  | 
 | ||||||
|  | 		// A selector update change doesn't trigger replicaset replacement,
 | ||||||
|  | 		// but we still need to update the existing replicaset with it.
 | ||||||
|  | 		// Otherwise selector-based runner query will never work on replicasets created before the controller v0.17.0
 | ||||||
|  | 		// See https://github.com/summerwind/actions-runner-controller/pull/355#discussion_r585379259
 | ||||||
|  | 		if err := r.Client.Update(ctx, updateSet); err != nil { | ||||||
|  | 			log.Error(err, "Failed to update runnerreplicaset resource") | ||||||
|  | 
 | ||||||
|  | 			return ctrl.Result{}, err | ||||||
|  | 		} | ||||||
|  | 
 | ||||||
|  | 		// At this point, we are already sure that there's no need to create a new replicaset
 | ||||||
|  | 		// as the runner template hash is not changed.
 | ||||||
|  | 		//
 | ||||||
|  | 		// But we still need to requeue for the (possibly rare) cases that there are still old replicasets that needs
 | ||||||
|  | 		// to be cleaned up.
 | ||||||
|  | 		return ctrl.Result{RequeueAfter: 5 * time.Second}, nil | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
| 	const defaultReplicas = 1 | 	const defaultReplicas = 1 | ||||||
| 
 | 
 | ||||||
| 	currentDesiredReplicas := getIntOrDefault(newestSet.Spec.Replicas, defaultReplicas) | 	currentDesiredReplicas := getIntOrDefault(newestSet.Spec.Replicas, defaultReplicas) | ||||||
|  | @ -258,18 +281,70 @@ func CloneAndAddLabel(labels map[string]string, labelKey, labelValue string) map | ||||||
| 	return newLabels | 	return newLabels | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | // Clones the given selector and returns a new selector with the given key and value added.
 | ||||||
|  | // Returns the given selector, if labelKey is empty.
 | ||||||
|  | //
 | ||||||
|  | // Proudly copied from k8s.io/kubernetes/pkg/util/labels.CloneSelectorAndAddLabel
 | ||||||
|  | func CloneSelectorAndAddLabel(selector *metav1.LabelSelector, labelKey, labelValue string) *metav1.LabelSelector { | ||||||
|  | 	if labelKey == "" { | ||||||
|  | 		// Don't need to add a label.
 | ||||||
|  | 		return selector | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	// Clone.
 | ||||||
|  | 	newSelector := new(metav1.LabelSelector) | ||||||
|  | 
 | ||||||
|  | 	newSelector.MatchLabels = make(map[string]string) | ||||||
|  | 	if selector.MatchLabels != nil { | ||||||
|  | 		for key, val := range selector.MatchLabels { | ||||||
|  | 			newSelector.MatchLabels[key] = val | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | 	newSelector.MatchLabels[labelKey] = labelValue | ||||||
|  | 
 | ||||||
|  | 	if selector.MatchExpressions != nil { | ||||||
|  | 		newMExps := make([]metav1.LabelSelectorRequirement, len(selector.MatchExpressions)) | ||||||
|  | 		for i, me := range selector.MatchExpressions { | ||||||
|  | 			newMExps[i].Key = me.Key | ||||||
|  | 			newMExps[i].Operator = me.Operator | ||||||
|  | 			if me.Values != nil { | ||||||
|  | 				newMExps[i].Values = make([]string, len(me.Values)) | ||||||
|  | 				copy(newMExps[i].Values, me.Values) | ||||||
|  | 			} else { | ||||||
|  | 				newMExps[i].Values = nil | ||||||
|  | 			} | ||||||
|  | 		} | ||||||
|  | 		newSelector.MatchExpressions = newMExps | ||||||
|  | 	} else { | ||||||
|  | 		newSelector.MatchExpressions = nil | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	return newSelector | ||||||
|  | } | ||||||
|  | 
 | ||||||
| func (r *RunnerDeploymentReconciler) newRunnerReplicaSet(rd v1alpha1.RunnerDeployment) (*v1alpha1.RunnerReplicaSet, error) { | func (r *RunnerDeploymentReconciler) newRunnerReplicaSet(rd v1alpha1.RunnerDeployment) (*v1alpha1.RunnerReplicaSet, error) { | ||||||
|  | 	return newRunnerReplicaSet(&rd, r.CommonRunnerLabels, r.Scheme) | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | func newRunnerReplicaSet(rd *v1alpha1.RunnerDeployment, commonRunnerLabels []string, scheme *runtime.Scheme) (*v1alpha1.RunnerReplicaSet, error) { | ||||||
| 	newRSTemplate := *rd.Spec.Template.DeepCopy() | 	newRSTemplate := *rd.Spec.Template.DeepCopy() | ||||||
|  | 
 | ||||||
| 	templateHash := ComputeHash(&newRSTemplate) | 	templateHash := ComputeHash(&newRSTemplate) | ||||||
| 	// Add template hash label to selector.
 | 	// Add template hash label to selector.
 | ||||||
| 	labels := CloneAndAddLabel(rd.Spec.Template.Labels, LabelKeyRunnerTemplateHash, templateHash) | 	labels := CloneAndAddLabel(rd.Spec.Template.Labels, LabelKeyRunnerTemplateHash, templateHash) | ||||||
| 
 | 
 | ||||||
| 	for _, l := range r.CommonRunnerLabels { | 	for _, l := range commonRunnerLabels { | ||||||
| 		newRSTemplate.Spec.Labels = append(newRSTemplate.Spec.Labels, l) | 		newRSTemplate.Spec.Labels = append(newRSTemplate.Spec.Labels, l) | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	newRSTemplate.Labels = labels | 	newRSTemplate.Labels = labels | ||||||
| 
 | 
 | ||||||
|  | 	selector := rd.Spec.Selector | ||||||
|  | 	if selector == nil { | ||||||
|  | 		selector = &metav1.LabelSelector{MatchLabels: labels} | ||||||
|  | 	} | ||||||
|  | 	newRSSelector := CloneSelectorAndAddLabel(selector, LabelKeyRunnerTemplateHash, templateHash) | ||||||
|  | 
 | ||||||
| 	rs := v1alpha1.RunnerReplicaSet{ | 	rs := v1alpha1.RunnerReplicaSet{ | ||||||
| 		TypeMeta: metav1.TypeMeta{}, | 		TypeMeta: metav1.TypeMeta{}, | ||||||
| 		ObjectMeta: metav1.ObjectMeta{ | 		ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | @ -279,11 +354,12 @@ func (r *RunnerDeploymentReconciler) newRunnerReplicaSet(rd v1alpha1.RunnerDeplo | ||||||
| 		}, | 		}, | ||||||
| 		Spec: v1alpha1.RunnerReplicaSetSpec{ | 		Spec: v1alpha1.RunnerReplicaSetSpec{ | ||||||
| 			Replicas: rd.Spec.Replicas, | 			Replicas: rd.Spec.Replicas, | ||||||
|  | 			Selector: newRSSelector, | ||||||
| 			Template: newRSTemplate, | 			Template: newRSTemplate, | ||||||
| 		}, | 		}, | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	if err := ctrl.SetControllerReference(&rd, &rs, r.Scheme); err != nil { | 	if err := ctrl.SetControllerReference(rd, &rs, scheme); err != nil { | ||||||
| 		return &rs, err | 		return &rs, err | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -2,11 +2,13 @@ package controllers | ||||||
| 
 | 
 | ||||||
| import ( | import ( | ||||||
| 	"context" | 	"context" | ||||||
| 	"github.com/google/go-cmp/cmp" | 	"fmt" | ||||||
| 	"k8s.io/apimachinery/pkg/runtime" |  | ||||||
| 	"testing" | 	"testing" | ||||||
| 	"time" | 	"time" | ||||||
| 
 | 
 | ||||||
|  | 	"github.com/google/go-cmp/cmp" | ||||||
|  | 	"k8s.io/apimachinery/pkg/runtime" | ||||||
|  | 
 | ||||||
| 	corev1 "k8s.io/api/core/v1" | 	corev1 "k8s.io/api/core/v1" | ||||||
| 	"k8s.io/apimachinery/pkg/types" | 	"k8s.io/apimachinery/pkg/types" | ||||||
| 	"k8s.io/client-go/kubernetes/scheme" | 	"k8s.io/client-go/kubernetes/scheme" | ||||||
|  | @ -36,7 +38,17 @@ func TestNewRunnerReplicaSet(t *testing.T) { | ||||||
| 			Name: "example", | 			Name: "example", | ||||||
| 		}, | 		}, | ||||||
| 		Spec: actionsv1alpha1.RunnerDeploymentSpec{ | 		Spec: actionsv1alpha1.RunnerDeploymentSpec{ | ||||||
|  | 			Selector: &metav1.LabelSelector{ | ||||||
|  | 				MatchLabels: map[string]string{ | ||||||
|  | 					"foo": "bar", | ||||||
|  | 				}, | ||||||
|  | 			}, | ||||||
| 			Template: actionsv1alpha1.RunnerTemplate{ | 			Template: actionsv1alpha1.RunnerTemplate{ | ||||||
|  | 				ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 					Labels: map[string]string{ | ||||||
|  | 						"foo": "bar", | ||||||
|  | 					}, | ||||||
|  | 				}, | ||||||
| 				Spec: actionsv1alpha1.RunnerSpec{ | 				Spec: actionsv1alpha1.RunnerSpec{ | ||||||
| 					Labels: []string{"project1"}, | 					Labels: []string{"project1"}, | ||||||
| 				}, | 				}, | ||||||
|  | @ -49,10 +61,63 @@ func TestNewRunnerReplicaSet(t *testing.T) { | ||||||
| 		t.Fatalf("%v", err) | 		t.Fatalf("%v", err) | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	want := []string{"project1", "dev"} | 	if val, ok := rs.Labels["foo"]; ok { | ||||||
| 	if d := cmp.Diff(want, rs.Spec.Template.Spec.Labels); d != "" { | 		if val != "bar" { | ||||||
|  | 			t.Errorf("foo label does not have bar but %v", val) | ||||||
|  | 		} | ||||||
|  | 	} else { | ||||||
|  | 		t.Errorf("foo label does not exist") | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	hash1, ok := rs.Labels[LabelKeyRunnerTemplateHash] | ||||||
|  | 	if !ok { | ||||||
|  | 		t.Errorf("missing runner-template-hash label") | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	runnerLabel := []string{"project1", "dev"} | ||||||
|  | 	if d := cmp.Diff(runnerLabel, rs.Spec.Template.Spec.Labels); d != "" { | ||||||
| 		t.Errorf("%s", d) | 		t.Errorf("%s", d) | ||||||
| 	} | 	} | ||||||
|  | 
 | ||||||
|  | 	rd2 := rd.DeepCopy() | ||||||
|  | 	rd2.Spec.Template.Spec.Labels = []string{"project2"} | ||||||
|  | 
 | ||||||
|  | 	rs2, err := r.newRunnerReplicaSet(*rd2) | ||||||
|  | 	if err != nil { | ||||||
|  | 		t.Fatalf("%v", err) | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	hash2, ok := rs2.Labels[LabelKeyRunnerTemplateHash] | ||||||
|  | 	if !ok { | ||||||
|  | 		t.Errorf("missing runner-template-hash label") | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	if hash1 == hash2 { | ||||||
|  | 		t.Errorf( | ||||||
|  | 			"runner replica sets from runner deployments with varying labels must have different template hash, but got %s and %s", | ||||||
|  | 			hash1, hash2, | ||||||
|  | 		) | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	rd3 := rd.DeepCopy() | ||||||
|  | 	rd3.Spec.Template.Labels["foo"] = "baz" | ||||||
|  | 
 | ||||||
|  | 	rs3, err := r.newRunnerReplicaSet(*rd3) | ||||||
|  | 	if err != nil { | ||||||
|  | 		t.Fatalf("%v", err) | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	hash3, ok := rs3.Labels[LabelKeyRunnerTemplateHash] | ||||||
|  | 	if !ok { | ||||||
|  | 		t.Errorf("missing runner-template-hash label") | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	if hash1 == hash3 { | ||||||
|  | 		t.Errorf( | ||||||
|  | 			"runner replica sets from runner deployments with varying meta labels must have different template hash, but got %s and %s", | ||||||
|  | 			hash1, hash3, | ||||||
|  | 		) | ||||||
|  | 	} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| // SetupDeploymentTest will set up a testing environment.
 | // SetupDeploymentTest will set up a testing environment.
 | ||||||
|  | @ -112,7 +177,113 @@ var _ = Context("Inside of a new namespace", func() { | ||||||
| 	Describe("when no existing resources exist", func() { | 	Describe("when no existing resources exist", func() { | ||||||
| 
 | 
 | ||||||
| 		It("should create a new RunnerReplicaSet resource from the specified template, add a another RunnerReplicaSet on template modification, and eventually removes old runnerreplicasets", func() { | 		It("should create a new RunnerReplicaSet resource from the specified template, add a another RunnerReplicaSet on template modification, and eventually removes old runnerreplicasets", func() { | ||||||
| 			name := "example-runnerdeploy" | 			name := "example-runnerdeploy-1" | ||||||
|  | 
 | ||||||
|  | 			{ | ||||||
|  | 				rs := &actionsv1alpha1.RunnerDeployment{ | ||||||
|  | 					ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 						Name:      name, | ||||||
|  | 						Namespace: ns.Name, | ||||||
|  | 					}, | ||||||
|  | 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | ||||||
|  | 						Replicas: intPtr(1), | ||||||
|  | 						Selector: &metav1.LabelSelector{ | ||||||
|  | 							MatchLabels: map[string]string{ | ||||||
|  | 								"foo": "bar", | ||||||
|  | 							}, | ||||||
|  | 						}, | ||||||
|  | 						Template: actionsv1alpha1.RunnerTemplate{ | ||||||
|  | 							ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 								Labels: map[string]string{ | ||||||
|  | 									"foo": "bar", | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
|  | 							Spec: actionsv1alpha1.RunnerSpec{ | ||||||
|  | 								Repository: "foo/bar", | ||||||
|  | 								Image:      "bar", | ||||||
|  | 								Env: []corev1.EnvVar{ | ||||||
|  | 									{Name: "FOO", Value: "FOOVALUE"}, | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
|  | 						}, | ||||||
|  | 					}, | ||||||
|  | 				} | ||||||
|  | 
 | ||||||
|  | 				err := k8sClient.Create(ctx, rs) | ||||||
|  | 
 | ||||||
|  | 				Expect(err).NotTo(HaveOccurred(), "failed to create test RunnerReplicaSet resource") | ||||||
|  | 
 | ||||||
|  | 				runnerSets := actionsv1alpha1.RunnerReplicaSetList{Items: []actionsv1alpha1.RunnerReplicaSet{}} | ||||||
|  | 
 | ||||||
|  | 				Eventually( | ||||||
|  | 					func() (int, error) { | ||||||
|  | 						selector, err := metav1.LabelSelectorAsSelector(rs.Spec.Selector) | ||||||
|  | 						if err != nil { | ||||||
|  | 							return 0, err | ||||||
|  | 						} | ||||||
|  | 						err = k8sClient.List( | ||||||
|  | 							ctx, | ||||||
|  | 							&runnerSets, | ||||||
|  | 							client.InNamespace(ns.Name), | ||||||
|  | 							client.MatchingLabelsSelector{Selector: selector}, | ||||||
|  | 						) | ||||||
|  | 						if err != nil { | ||||||
|  | 							return 0, err | ||||||
|  | 						} | ||||||
|  | 						if len(runnerSets.Items) != 1 { | ||||||
|  | 							return 0, fmt.Errorf("runnerreplicasets is not 1 but %d", len(runnerSets.Items)) | ||||||
|  | 						} | ||||||
|  | 
 | ||||||
|  | 						return *runnerSets.Items[0].Spec.Replicas, nil | ||||||
|  | 					}, | ||||||
|  | 					time.Second*5, time.Millisecond*500).Should(BeEquivalentTo(1)) | ||||||
|  | 			} | ||||||
|  | 
 | ||||||
|  | 			{ | ||||||
|  | 				// We wrap the update in the Eventually block to avoid the below error that occurs due to concurrent modification
 | ||||||
|  | 				// made by the controller to update .Status.AvailableReplicas and .Status.ReadyReplicas
 | ||||||
|  | 				//   Operation cannot be fulfilled on runnersets.actions.summerwind.dev "example-runnerset": the object has been modified; please apply your changes to the latest version and try again
 | ||||||
|  | 				var rd actionsv1alpha1.RunnerDeployment | ||||||
|  | 				Eventually(func() error { | ||||||
|  | 					err := k8sClient.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: name}, &rd) | ||||||
|  | 					if err != nil { | ||||||
|  | 						return fmt.Errorf("failed to get test RunnerReplicaSet resource: %v\n", err) | ||||||
|  | 					} | ||||||
|  | 					rd.Spec.Replicas = intPtr(2) | ||||||
|  | 
 | ||||||
|  | 					return k8sClient.Update(ctx, &rd) | ||||||
|  | 				}, | ||||||
|  | 					time.Second*1, time.Millisecond*500).Should(BeNil()) | ||||||
|  | 
 | ||||||
|  | 				runnerSets := actionsv1alpha1.RunnerReplicaSetList{Items: []actionsv1alpha1.RunnerReplicaSet{}} | ||||||
|  | 
 | ||||||
|  | 				Eventually( | ||||||
|  | 					func() (int, error) { | ||||||
|  | 						selector, err := metav1.LabelSelectorAsSelector(rd.Spec.Selector) | ||||||
|  | 						if err != nil { | ||||||
|  | 							return 0, err | ||||||
|  | 						} | ||||||
|  | 						err = k8sClient.List( | ||||||
|  | 							ctx, | ||||||
|  | 							&runnerSets, | ||||||
|  | 							client.InNamespace(ns.Name), | ||||||
|  | 							client.MatchingLabelsSelector{Selector: selector}, | ||||||
|  | 						) | ||||||
|  | 						if err != nil { | ||||||
|  | 							return 0, err | ||||||
|  | 						} | ||||||
|  | 						if len(runnerSets.Items) != 1 { | ||||||
|  | 							return 0, fmt.Errorf("runnerreplicasets is not 1 but %d", len(runnerSets.Items)) | ||||||
|  | 						} | ||||||
|  | 
 | ||||||
|  | 						return *runnerSets.Items[0].Spec.Replicas, nil | ||||||
|  | 					}, | ||||||
|  | 					time.Second*5, time.Millisecond*500).Should(BeEquivalentTo(2)) | ||||||
|  | 			} | ||||||
|  | 		}) | ||||||
|  | 
 | ||||||
|  | 		It("should create a new RunnerReplicaSet resource from the specified template without labels and selector, add a another RunnerReplicaSet on template modification, and eventually removes old runnerreplicasets", func() { | ||||||
|  | 			name := "example-runnerdeploy-2" | ||||||
| 
 | 
 | ||||||
| 			{ | 			{ | ||||||
| 				rs := &actionsv1alpha1.RunnerDeployment{ | 				rs := &actionsv1alpha1.RunnerDeployment{ | ||||||
|  | @ -141,29 +312,25 @@ var _ = Context("Inside of a new namespace", func() { | ||||||
| 				runnerSets := actionsv1alpha1.RunnerReplicaSetList{Items: []actionsv1alpha1.RunnerReplicaSet{}} | 				runnerSets := actionsv1alpha1.RunnerReplicaSetList{Items: []actionsv1alpha1.RunnerReplicaSet{}} | ||||||
| 
 | 
 | ||||||
| 				Eventually( | 				Eventually( | ||||||
| 					func() int { | 					func() (int, error) { | ||||||
| 						err := k8sClient.List(ctx, &runnerSets, client.InNamespace(ns.Name)) | 						selector, err := metav1.LabelSelectorAsSelector(rs.Spec.Selector) | ||||||
| 						if err != nil { | 						if err != nil { | ||||||
| 							logf.Log.Error(err, "list runner sets") | 							return 0, err | ||||||
| 						} | 						} | ||||||
| 
 | 						err = k8sClient.List( | ||||||
| 						return len(runnerSets.Items) | 							ctx, | ||||||
| 					}, | 							&runnerSets, | ||||||
| 					time.Second*5, time.Millisecond*500).Should(BeEquivalentTo(1)) | 							client.InNamespace(ns.Name), | ||||||
| 
 | 							client.MatchingLabelsSelector{Selector: selector}, | ||||||
| 				Eventually( | 						) | ||||||
| 					func() int { |  | ||||||
| 						err := k8sClient.List(ctx, &runnerSets, client.InNamespace(ns.Name)) |  | ||||||
| 						if err != nil { | 						if err != nil { | ||||||
| 							logf.Log.Error(err, "list runner sets") | 							return 0, err | ||||||
|  | 						} | ||||||
|  | 						if len(runnerSets.Items) != 1 { | ||||||
|  | 							return 0, fmt.Errorf("runnerreplicasets is not 1 but %d", len(runnerSets.Items)) | ||||||
| 						} | 						} | ||||||
| 
 | 
 | ||||||
| 						if len(runnerSets.Items) == 0 { | 						return *runnerSets.Items[0].Spec.Replicas, nil | ||||||
| 							logf.Log.Info("No runnerreplicasets exist yet") |  | ||||||
| 							return -1 |  | ||||||
| 						} |  | ||||||
| 
 |  | ||||||
| 						return *runnerSets.Items[0].Spec.Replicas |  | ||||||
| 					}, | 					}, | ||||||
| 					time.Second*5, time.Millisecond*500).Should(BeEquivalentTo(1)) | 					time.Second*5, time.Millisecond*500).Should(BeEquivalentTo(1)) | ||||||
| 			} | 			} | ||||||
|  | @ -172,13 +339,12 @@ var _ = Context("Inside of a new namespace", func() { | ||||||
| 				// We wrap the update in the Eventually block to avoid the below error that occurs due to concurrent modification
 | 				// We wrap the update in the Eventually block to avoid the below error that occurs due to concurrent modification
 | ||||||
| 				// made by the controller to update .Status.AvailableReplicas and .Status.ReadyReplicas
 | 				// made by the controller to update .Status.AvailableReplicas and .Status.ReadyReplicas
 | ||||||
| 				//   Operation cannot be fulfilled on runnersets.actions.summerwind.dev "example-runnerset": the object has been modified; please apply your changes to the latest version and try again
 | 				//   Operation cannot be fulfilled on runnersets.actions.summerwind.dev "example-runnerset": the object has been modified; please apply your changes to the latest version and try again
 | ||||||
| 				Eventually(func() error { |  | ||||||
| 				var rd actionsv1alpha1.RunnerDeployment | 				var rd actionsv1alpha1.RunnerDeployment | ||||||
| 
 | 				Eventually(func() error { | ||||||
| 					err := k8sClient.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: name}, &rd) | 					err := k8sClient.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: name}, &rd) | ||||||
| 
 | 					if err != nil { | ||||||
| 					Expect(err).NotTo(HaveOccurred(), "failed to get test RunnerReplicaSet resource") | 						return fmt.Errorf("failed to get test RunnerReplicaSet resource: %v\n", err) | ||||||
| 
 | 					} | ||||||
| 					rd.Spec.Replicas = intPtr(2) | 					rd.Spec.Replicas = intPtr(2) | ||||||
| 
 | 
 | ||||||
| 					return k8sClient.Update(ctx, &rd) | 					return k8sClient.Update(ctx, &rd) | ||||||
|  | @ -188,27 +354,126 @@ var _ = Context("Inside of a new namespace", func() { | ||||||
| 				runnerSets := actionsv1alpha1.RunnerReplicaSetList{Items: []actionsv1alpha1.RunnerReplicaSet{}} | 				runnerSets := actionsv1alpha1.RunnerReplicaSetList{Items: []actionsv1alpha1.RunnerReplicaSet{}} | ||||||
| 
 | 
 | ||||||
| 				Eventually( | 				Eventually( | ||||||
| 					func() int { | 					func() (int, error) { | ||||||
| 						err := k8sClient.List(ctx, &runnerSets, client.InNamespace(ns.Name)) | 						selector, err := metav1.LabelSelectorAsSelector(rd.Spec.Selector) | ||||||
| 						if err != nil { | 						if err != nil { | ||||||
| 							logf.Log.Error(err, "list runner sets") | 							return 0, err | ||||||
|  | 						} | ||||||
|  | 						err = k8sClient.List( | ||||||
|  | 							ctx, | ||||||
|  | 							&runnerSets, | ||||||
|  | 							client.InNamespace(ns.Name), | ||||||
|  | 							client.MatchingLabelsSelector{Selector: selector}, | ||||||
|  | 						) | ||||||
|  | 						if err != nil { | ||||||
|  | 							return 0, err | ||||||
|  | 						} | ||||||
|  | 						if len(runnerSets.Items) != 1 { | ||||||
|  | 							return 0, fmt.Errorf("runnerreplicasets is not 1 but %d", len(runnerSets.Items)) | ||||||
| 						} | 						} | ||||||
| 
 | 
 | ||||||
| 						return len(runnerSets.Items) | 						return *runnerSets.Items[0].Spec.Replicas, nil | ||||||
| 					}, |  | ||||||
| 					time.Second*5, time.Millisecond*500).Should(BeEquivalentTo(1)) |  | ||||||
| 
 |  | ||||||
| 				Eventually( |  | ||||||
| 					func() int { |  | ||||||
| 						err := k8sClient.List(ctx, &runnerSets, client.InNamespace(ns.Name)) |  | ||||||
| 						if err != nil { |  | ||||||
| 							logf.Log.Error(err, "list runner sets") |  | ||||||
| 						} |  | ||||||
| 
 |  | ||||||
| 						return *runnerSets.Items[0].Spec.Replicas |  | ||||||
| 					}, | 					}, | ||||||
| 					time.Second*5, time.Millisecond*500).Should(BeEquivalentTo(2)) | 					time.Second*5, time.Millisecond*500).Should(BeEquivalentTo(2)) | ||||||
| 			} | 			} | ||||||
| 		}) | 		}) | ||||||
|  | 
 | ||||||
|  | 		It("should adopt RunnerReplicaSet created before 0.18.0 to have Spec.Selector", func() { | ||||||
|  | 			name := "example-runnerdeploy-2" | ||||||
|  | 
 | ||||||
|  | 			{ | ||||||
|  | 				rd := &actionsv1alpha1.RunnerDeployment{ | ||||||
|  | 					ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 						Name:      name, | ||||||
|  | 						Namespace: ns.Name, | ||||||
|  | 					}, | ||||||
|  | 					Spec: actionsv1alpha1.RunnerDeploymentSpec{ | ||||||
|  | 						Replicas: intPtr(1), | ||||||
|  | 						Template: actionsv1alpha1.RunnerTemplate{ | ||||||
|  | 							Spec: actionsv1alpha1.RunnerSpec{ | ||||||
|  | 								Repository: "foo/bar", | ||||||
|  | 								Image:      "bar", | ||||||
|  | 								Env: []corev1.EnvVar{ | ||||||
|  | 									{Name: "FOO", Value: "FOOVALUE"}, | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
|  | 						}, | ||||||
|  | 					}, | ||||||
|  | 				} | ||||||
|  | 
 | ||||||
|  | 				createRDErr := k8sClient.Create(ctx, rd) | ||||||
|  | 				Expect(createRDErr).NotTo(HaveOccurred(), "failed to create test RunnerReplicaSet resource") | ||||||
|  | 
 | ||||||
|  | 				Eventually( | ||||||
|  | 					func() (int, error) { | ||||||
|  | 						runnerSets := actionsv1alpha1.RunnerReplicaSetList{Items: []actionsv1alpha1.RunnerReplicaSet{}} | ||||||
|  | 
 | ||||||
|  | 						err := k8sClient.List( | ||||||
|  | 							ctx, | ||||||
|  | 							&runnerSets, | ||||||
|  | 							client.InNamespace(ns.Name), | ||||||
|  | 						) | ||||||
|  | 						if err != nil { | ||||||
|  | 							return 0, err | ||||||
|  | 						} | ||||||
|  | 
 | ||||||
|  | 						return len(runnerSets.Items), nil | ||||||
|  | 					}, | ||||||
|  | 					time.Second*1, time.Millisecond*500).Should(BeEquivalentTo(1)) | ||||||
|  | 
 | ||||||
|  | 				var rs17 *actionsv1alpha1.RunnerReplicaSet | ||||||
|  | 
 | ||||||
|  | 				Consistently( | ||||||
|  | 					func() (*metav1.LabelSelector, error) { | ||||||
|  | 						runnerSets := actionsv1alpha1.RunnerReplicaSetList{Items: []actionsv1alpha1.RunnerReplicaSet{}} | ||||||
|  | 
 | ||||||
|  | 						err := k8sClient.List( | ||||||
|  | 							ctx, | ||||||
|  | 							&runnerSets, | ||||||
|  | 							client.InNamespace(ns.Name), | ||||||
|  | 						) | ||||||
|  | 						if err != nil { | ||||||
|  | 							return nil, err | ||||||
|  | 						} | ||||||
|  | 						if len(runnerSets.Items) != 1 { | ||||||
|  | 							return nil, fmt.Errorf("runnerreplicasets is not 1 but %d", len(runnerSets.Items)) | ||||||
|  | 						} | ||||||
|  | 
 | ||||||
|  | 						rs17 = &runnerSets.Items[0] | ||||||
|  | 
 | ||||||
|  | 						return runnerSets.Items[0].Spec.Selector, nil | ||||||
|  | 					}, | ||||||
|  | 					time.Second*1, time.Millisecond*500).Should(Not(BeNil())) | ||||||
|  | 
 | ||||||
|  | 				// We simulate the old, pre 0.18.0 RunnerReplicaSet by updating it.
 | ||||||
|  | 				// I've tried to use controllerutil.Set{Owner,Controller}Reference and k8sClient.Create(rs17)
 | ||||||
|  | 				// but it didn't work due to missing RD UID, where UID is generated on K8s API server on k8sCLient.Create(rd)
 | ||||||
|  | 				rs17.Spec.Selector = nil | ||||||
|  | 
 | ||||||
|  | 				updateRSErr := k8sClient.Update(ctx, rs17) | ||||||
|  | 				Expect(updateRSErr).NotTo(HaveOccurred()) | ||||||
|  | 
 | ||||||
|  | 				Eventually( | ||||||
|  | 					func() (*metav1.LabelSelector, error) { | ||||||
|  | 						runnerSets := actionsv1alpha1.RunnerReplicaSetList{Items: []actionsv1alpha1.RunnerReplicaSet{}} | ||||||
|  | 
 | ||||||
|  | 						err := k8sClient.List( | ||||||
|  | 							ctx, | ||||||
|  | 							&runnerSets, | ||||||
|  | 							client.InNamespace(ns.Name), | ||||||
|  | 						) | ||||||
|  | 						if err != nil { | ||||||
|  | 							return nil, err | ||||||
|  | 						} | ||||||
|  | 						if len(runnerSets.Items) != 1 { | ||||||
|  | 							return nil, fmt.Errorf("runnerreplicasets is not 1 but %d", len(runnerSets.Items)) | ||||||
|  | 						} | ||||||
|  | 
 | ||||||
|  | 						return runnerSets.Items[0].Spec.Selector, nil | ||||||
|  | 					}, | ||||||
|  | 					time.Second*1, time.Millisecond*500).Should(Not(BeNil())) | ||||||
|  | 			} | ||||||
|  | 		}) | ||||||
|  | 
 | ||||||
| 	}) | 	}) | ||||||
| }) | }) | ||||||
|  |  | ||||||
|  | @ -68,8 +68,18 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e | ||||||
| 		return ctrl.Result{}, nil | 		return ctrl.Result{}, nil | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 	selector, err := metav1.LabelSelectorAsSelector(rs.Spec.Selector) | ||||||
|  | 	if err != nil { | ||||||
|  | 		return ctrl.Result{}, err | ||||||
|  | 	} | ||||||
|  | 	// Get the Runners managed by the target RunnerReplicaSet
 | ||||||
| 	var allRunners v1alpha1.RunnerList | 	var allRunners v1alpha1.RunnerList | ||||||
| 	if err := r.List(ctx, &allRunners, client.InNamespace(req.Namespace)); err != nil { | 	if err := r.List( | ||||||
|  | 		ctx, | ||||||
|  | 		&allRunners, | ||||||
|  | 		client.InNamespace(req.Namespace), | ||||||
|  | 		client.MatchingLabelsSelector{Selector: selector}, | ||||||
|  | 	); err != nil { | ||||||
| 		if !kerrors.IsNotFound(err) { | 		if !kerrors.IsNotFound(err) { | ||||||
| 			return ctrl.Result{}, err | 			return ctrl.Result{}, err | ||||||
| 		} | 		} | ||||||
|  | @ -77,9 +87,14 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e | ||||||
| 
 | 
 | ||||||
| 	var myRunners []v1alpha1.Runner | 	var myRunners []v1alpha1.Runner | ||||||
| 
 | 
 | ||||||
| 	var available, ready int | 	var ( | ||||||
|  | 		available int | ||||||
|  | 		ready     int | ||||||
|  | 	) | ||||||
| 
 | 
 | ||||||
| 	for _, r := range allRunners.Items { | 	for _, r := range allRunners.Items { | ||||||
|  | 		// This guard is required to avoid the RunnerReplicaSet created by the controller v0.17.0 or before
 | ||||||
|  | 		// to not treat all the runners in the namespace as its children.
 | ||||||
| 		if metav1.IsControlledBy(&r, &rs) { | 		if metav1.IsControlledBy(&r, &rs) { | ||||||
| 			myRunners = append(myRunners, r) | 			myRunners = append(myRunners, r) | ||||||
| 
 | 
 | ||||||
|  | @ -106,7 +121,7 @@ func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e | ||||||
| 
 | 
 | ||||||
| 		// get runners that are currently not busy
 | 		// get runners that are currently not busy
 | ||||||
| 		var notBusy []v1alpha1.Runner | 		var notBusy []v1alpha1.Runner | ||||||
| 		for _, runner := range myRunners { | 		for _, runner := range allRunners.Items { | ||||||
| 			busy, err := r.GitHubClient.IsRunnerBusy(ctx, runner.Spec.Enterprise, runner.Spec.Organization, runner.Spec.Repository, runner.Name) | 			busy, err := r.GitHubClient.IsRunnerBusy(ctx, runner.Spec.Enterprise, runner.Spec.Organization, runner.Spec.Repository, runner.Name) | ||||||
| 			if err != nil { | 			if err != nil { | ||||||
| 				notRegistered := false | 				notRegistered := false | ||||||
|  |  | ||||||
|  | @ -115,7 +115,17 @@ var _ = Context("Inside of a new namespace", func() { | ||||||
| 					}, | 					}, | ||||||
| 					Spec: actionsv1alpha1.RunnerReplicaSetSpec{ | 					Spec: actionsv1alpha1.RunnerReplicaSetSpec{ | ||||||
| 						Replicas: intPtr(1), | 						Replicas: intPtr(1), | ||||||
|  | 						Selector: &metav1.LabelSelector{ | ||||||
|  | 							MatchLabels: map[string]string{ | ||||||
|  | 								"foo": "bar", | ||||||
|  | 							}, | ||||||
|  | 						}, | ||||||
| 						Template: actionsv1alpha1.RunnerTemplate{ | 						Template: actionsv1alpha1.RunnerTemplate{ | ||||||
|  | 							ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 								Labels: map[string]string{ | ||||||
|  | 									"foo": "bar", | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
| 							Spec: actionsv1alpha1.RunnerSpec{ | 							Spec: actionsv1alpha1.RunnerSpec{ | ||||||
| 								Repository: "foo/bar", | 								Repository: "foo/bar", | ||||||
| 								Image:      "bar", | 								Image:      "bar", | ||||||
|  | @ -135,9 +145,26 @@ var _ = Context("Inside of a new namespace", func() { | ||||||
| 
 | 
 | ||||||
| 				Eventually( | 				Eventually( | ||||||
| 					func() int { | 					func() int { | ||||||
| 						err := k8sClient.List(ctx, &runners, client.InNamespace(ns.Name)) | 						selector, err := metav1.LabelSelectorAsSelector( | ||||||
|  | 							&metav1.LabelSelector{ | ||||||
|  | 								MatchLabels: map[string]string{ | ||||||
|  | 									"foo": "bar", | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
|  | 						) | ||||||
|  | 						if err != nil { | ||||||
|  | 							logf.Log.Error(err, "failed to create labelselector") | ||||||
|  | 							return -1 | ||||||
|  | 						} | ||||||
|  | 						err = k8sClient.List( | ||||||
|  | 							ctx, | ||||||
|  | 							&runners, | ||||||
|  | 							client.InNamespace(ns.Name), | ||||||
|  | 							client.MatchingLabelsSelector{Selector: selector}, | ||||||
|  | 						) | ||||||
| 						if err != nil { | 						if err != nil { | ||||||
| 							logf.Log.Error(err, "list runners") | 							logf.Log.Error(err, "list runners") | ||||||
|  | 							return -1 | ||||||
| 						} | 						} | ||||||
| 
 | 
 | ||||||
| 						for i, runner := range runners.Items { | 						for i, runner := range runners.Items { | ||||||
|  | @ -176,7 +203,23 @@ var _ = Context("Inside of a new namespace", func() { | ||||||
| 
 | 
 | ||||||
| 				Eventually( | 				Eventually( | ||||||
| 					func() int { | 					func() int { | ||||||
| 						err := k8sClient.List(ctx, &runners, client.InNamespace(ns.Name)) | 						selector, err := metav1.LabelSelectorAsSelector( | ||||||
|  | 							&metav1.LabelSelector{ | ||||||
|  | 								MatchLabels: map[string]string{ | ||||||
|  | 									"foo": "bar", | ||||||
|  | 								}, | ||||||
|  | 							}, | ||||||
|  | 						) | ||||||
|  | 						if err != nil { | ||||||
|  | 							logf.Log.Error(err, "failed to create labelselector") | ||||||
|  | 							return -1 | ||||||
|  | 						} | ||||||
|  | 						err = k8sClient.List( | ||||||
|  | 							ctx, | ||||||
|  | 							&runners, | ||||||
|  | 							client.InNamespace(ns.Name), | ||||||
|  | 							client.MatchingLabelsSelector{Selector: selector}, | ||||||
|  | 						) | ||||||
| 						if err != nil { | 						if err != nil { | ||||||
| 							logf.Log.Error(err, "list runners") | 							logf.Log.Error(err, "list runners") | ||||||
| 						} | 						} | ||||||
|  | @ -220,6 +263,7 @@ var _ = Context("Inside of a new namespace", func() { | ||||||
| 						err := k8sClient.List(ctx, &runners, client.InNamespace(ns.Name)) | 						err := k8sClient.List(ctx, &runners, client.InNamespace(ns.Name)) | ||||||
| 						if err != nil { | 						if err != nil { | ||||||
| 							logf.Log.Error(err, "list runners") | 							logf.Log.Error(err, "list runners") | ||||||
|  | 							return -1 | ||||||
| 						} | 						} | ||||||
| 
 | 
 | ||||||
| 						for i, runner := range runners.Items { | 						for i, runner := range runners.Items { | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue