Fix old HRA capacity reservations not cleaned up (#354)
Similar to #348 for #346, but for HRA.Spec.CapacityReservations usually modified by the webhook-based autoscaler controller. This patch tries to fix that by improving the webhook-based autoscaler controller to omit expired reservations on updating HRA spec.
This commit is contained in:
		
							parent
							
								
									598dd1d9fe
								
							
						
					
					
						commit
						e9eef04993
					
				|  | @ -381,7 +381,9 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) tryScaleUp(ctx contex | |||
| 		amount = target.ScaleUpTrigger.Amount | ||||
| 	} | ||||
| 
 | ||||
| 	copy.Spec.CapacityReservations = append(copy.Spec.CapacityReservations, v1alpha1.CapacityReservation{ | ||||
| 	capacityReservations := getValidCapacityReservations(copy) | ||||
| 
 | ||||
| 	copy.Spec.CapacityReservations = append(capacityReservations, v1alpha1.CapacityReservation{ | ||||
| 		ExpirationTime: metav1.Time{Time: time.Now().Add(target.ScaleUpTrigger.Duration.Duration)}, | ||||
| 		Replicas:       amount, | ||||
| 	}) | ||||
|  | @ -395,6 +397,20 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) tryScaleUp(ctx contex | |||
| 	return nil | ||||
| } | ||||
| 
 | ||||
| func getValidCapacityReservations(autoscaler *v1alpha1.HorizontalRunnerAutoscaler) []v1alpha1.CapacityReservation { | ||||
| 	var capacityReservations []v1alpha1.CapacityReservation | ||||
| 
 | ||||
| 	now := time.Now() | ||||
| 
 | ||||
| 	for _, reservation := range autoscaler.Spec.CapacityReservations { | ||||
| 		if reservation.ExpirationTime.Time.After(now) { | ||||
| 			capacityReservations = append(capacityReservations, reservation) | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	return capacityReservations | ||||
| } | ||||
| 
 | ||||
| func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) SetupWithManager(mgr ctrl.Manager) error { | ||||
| 	name := "webhookbasedautoscaler" | ||||
| 	if autoscaler.Name != "" { | ||||
|  |  | |||
|  | @ -9,6 +9,7 @@ import ( | |||
| 	actionsv1alpha1 "github.com/summerwind/actions-runner-controller/api/v1alpha1" | ||||
| 	"io" | ||||
| 	"io/ioutil" | ||||
| 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||
| 	"k8s.io/apimachinery/pkg/runtime" | ||||
| 	clientgoscheme "k8s.io/client-go/kubernetes/scheme" | ||||
| 	"net/http" | ||||
|  | @ -17,6 +18,7 @@ import ( | |||
| 	"os" | ||||
| 	"sigs.k8s.io/controller-runtime/pkg/client/fake" | ||||
| 	"testing" | ||||
| 	"time" | ||||
| ) | ||||
| 
 | ||||
| var ( | ||||
|  | @ -111,6 +113,43 @@ func TestWebhookPing(t *testing.T) { | |||
| 	) | ||||
| } | ||||
| 
 | ||||
| func TestGetValidCapacityReservations(t *testing.T) { | ||||
| 	now := time.Now() | ||||
| 
 | ||||
| 	hra := &actionsv1alpha1.HorizontalRunnerAutoscaler{ | ||||
| 		Spec: actionsv1alpha1.HorizontalRunnerAutoscalerSpec{ | ||||
| 			CapacityReservations: []actionsv1alpha1.CapacityReservation{ | ||||
| 				{ | ||||
| 					ExpirationTime: metav1.Time{Time: now.Add(-time.Second)}, | ||||
| 					Replicas:       1, | ||||
| 				}, | ||||
| 				{ | ||||
| 					ExpirationTime: metav1.Time{Time: now}, | ||||
| 					Replicas:       2, | ||||
| 				}, | ||||
| 				{ | ||||
| 					ExpirationTime: metav1.Time{Time: now.Add(time.Second)}, | ||||
| 					Replicas:       3, | ||||
| 				}, | ||||
| 			}, | ||||
| 		}, | ||||
| 	} | ||||
| 
 | ||||
| 	revs := getValidCapacityReservations(hra) | ||||
| 
 | ||||
| 	var count int | ||||
| 
 | ||||
| 	for _, r := range revs { | ||||
| 		count += r.Replicas | ||||
| 	} | ||||
| 
 | ||||
| 	want := 3 | ||||
| 
 | ||||
| 	if count != want { | ||||
| 		t.Errorf("want %d, got %d", want, count) | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func installTestLogger(webhook *HorizontalRunnerAutoscalerGitHubWebhook) *bytes.Buffer { | ||||
| 	logs := &bytes.Buffer{} | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue