Skip to content

Commit

Permalink
Fix PercentageRunnersBusy scaling not working
Browse files Browse the repository at this point in the history
PercentageRunnerBusy seems to have regressed since #355 due to that RunnerDeployment.Spec.Selector is empty by default and the HRA controller was using that empty selector to query runners, which somehow returned 0 runners. This fixes that by using the newly added automatic `runner-deployment-name` label for the default runner label and the selector, which avoids querying with empty selector.

Ref #377 (comment)
  • Loading branch information
mumoshu committed Mar 11, 2021
1 parent f5c639a commit e863513
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 16 deletions.
19 changes: 14 additions & 5 deletions controllers/autoscaling.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,17 +266,26 @@ func (r *HorizontalRunnerAutoscalerReconciler) calculateReplicasByPercentageRunn
scaleDownFactor = sdf
}

selector, err := metav1.LabelSelectorAsSelector(rd.Spec.Selector)
// 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

var opts []client.ListOption

opts = append(opts, client.InNamespace(rd.Namespace))

selector, err := metav1.LabelSelectorAsSelector(getSelector(&rd))
if err != nil {
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

opts = append(opts, client.MatchingLabelsSelector{Selector: selector})

r.Log.V(2).Info("Finding runners with selector", "ns", rd.Namespace)

if err := r.List(
ctx,
&runnerList,
client.InNamespace(rd.Namespace),
client.MatchingLabelsSelector{Selector: selector},
opts...,
); err != nil {
if !kerrors.IsNotFound(err) {
return nil, err
Expand Down
32 changes: 21 additions & 11 deletions controllers/runnerdeployment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ import (
)

const (
LabelKeyRunnerTemplateHash = "runner-template-hash"
LabelKeyRunnerTemplateHash = "runner-template-hash"
LabelKeyRunnerDeploymentName = "runner-deployment-name"

runnerSetOwnerKey = ".metadata.controller"
)
Expand Down Expand Up @@ -326,31 +327,40 @@ func (r *RunnerDeploymentReconciler) newRunnerReplicaSet(rd v1alpha1.RunnerDeplo
return newRunnerReplicaSet(&rd, r.CommonRunnerLabels, r.Scheme)
}

func getSelector(rd *v1alpha1.RunnerDeployment) *metav1.LabelSelector {
selector := rd.Spec.Selector
if selector == nil {
selector = &metav1.LabelSelector{MatchLabels: map[string]string{LabelKeyRunnerDeploymentName: rd.Name}}
}

return selector
}

func newRunnerReplicaSet(rd *v1alpha1.RunnerDeployment, commonRunnerLabels []string, scheme *runtime.Scheme) (*v1alpha1.RunnerReplicaSet, error) {
newRSTemplate := *rd.Spec.Template.DeepCopy()

templateHash := ComputeHash(&newRSTemplate)
// Add template hash label to selector.
labels := CloneAndAddLabel(rd.Spec.Template.Labels, LabelKeyRunnerTemplateHash, templateHash)

for _, l := range commonRunnerLabels {
newRSTemplate.Spec.Labels = append(newRSTemplate.Spec.Labels, l)
}

newRSTemplate.Labels = labels
templateHash := ComputeHash(&newRSTemplate)

// Add template hash label to selector.
newRSTemplate.ObjectMeta.Labels = CloneAndAddLabel(newRSTemplate.ObjectMeta.Labels, LabelKeyRunnerTemplateHash, templateHash)

// This label selector is used by default when rd.Spec.Selector is empty.
newRSTemplate.ObjectMeta.Labels = CloneAndAddLabel(newRSTemplate.ObjectMeta.Labels, LabelKeyRunnerDeploymentName, rd.Name)

selector := getSelector(rd)

selector := rd.Spec.Selector
if selector == nil {
selector = &metav1.LabelSelector{MatchLabels: labels}
}
newRSSelector := CloneSelectorAndAddLabel(selector, LabelKeyRunnerTemplateHash, templateHash)

rs := v1alpha1.RunnerReplicaSet{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
GenerateName: rd.ObjectMeta.Name + "-",
Namespace: rd.ObjectMeta.Namespace,
Labels: labels,
Labels: newRSTemplate.ObjectMeta.Labels,
},
Spec: v1alpha1.RunnerReplicaSetSpec{
Replicas: rd.Spec.Replicas,
Expand Down

0 comments on commit e863513

Please sign in to comment.