Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add concurrency and adjust queue logic #3808

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+16 −9
Open

feat: add concurrency and adjust queue logic #3808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6b3b77
feat: add concurrency and adjust queue logic
xunholy Nov 13, 2024
75cb8f8
fix: test typo
xunholy Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions controllers/actions.github.com/ephemeralrunner_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)
Expand Down Expand Up @@ -133,7 +134,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
}

if ephemeralRunner.IsDone() {
log.Info("Cleaning up resources after after ephemeral runner termination", "phase", ephemeralRunner.Status.Phase)
log.Info("Cleaning up resources after ephemeral runner termination", "phase", ephemeralRunner.Status.Phase)
done, err := r.cleanupResources(ctx, ephemeralRunner, log)
if err != nil {
log.Error(err, "Failed to clean up ephemeral runner owned resources")
Expand Down Expand Up @@ -211,10 +212,10 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
default:
// Pod was not found. Create if the pod has never been created
log.Info("Creating new EphemeralRunner pod.")
result, err := r.createPod(ctx, ephemeralRunner, secret, log)
err := r.createPod(ctx, ephemeralRunner, secret, log)
switch {
case err == nil:
return result, nil
return ctrl.Result{}, err
case kerrors.IsInvalid(err) || kerrors.IsForbidden(err):
log.Error(err, "Failed to create a pod due to unrecoverable failure")
errMessage := fmt.Sprintf("Failed to create the pod: %v", err)
Expand Down Expand Up @@ -305,7 +306,6 @@ func (r *EphemeralRunnerReconciler) cleanupRunnerFromService(ctx context.Context
if actionsError.StatusCode == http.StatusBadRequest && actionsError.IsException("JobStillRunningException") {
log.Info("Runner is still running the job. Re-queue in 30 seconds")
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil

}

log.Error(err, "Failed clean up runner from the service")
Expand Down Expand Up @@ -592,7 +592,7 @@ func (r *EphemeralRunnerReconciler) updateStatusWithRunnerConfig(ctx context.Con
return ctrl.Result{}, nil
}

func (r *EphemeralRunnerReconciler) createPod(ctx context.Context, runner *v1alpha1.EphemeralRunner, secret *corev1.Secret, log logr.Logger) (ctrl.Result, error) {
func (r *EphemeralRunnerReconciler) createPod(ctx context.Context, runner *v1alpha1.EphemeralRunner, secret *corev1.Secret, log logr.Logger) error {
var envs []corev1.EnvVar
if runner.Spec.ProxySecretRef != "" {
http := corev1.EnvVar{
Expand Down Expand Up @@ -646,13 +646,13 @@ func (r *EphemeralRunnerReconciler) createPod(ctx context.Context, runner *v1alp

if err := ctrl.SetControllerReference(runner, newPod, r.Scheme); err != nil {
log.Error(err, "Failed to set controller reference to a new pod")
return ctrl.Result{}, err
return err
}

log.Info("Created new pod spec for ephemeral runner")
if err := r.Create(ctx, newPod); err != nil {
log.Error(err, "Failed to create pod resource for ephemeral runner.")
return ctrl.Result{}, err
return err
}

log.Info("Created ephemeral runner pod",
Expand All @@ -662,7 +662,7 @@ func (r *EphemeralRunnerReconciler) createPod(ctx context.Context, runner *v1alp
"configUrl", runner.Spec.GitHubConfigUrl,
"podName", newPod.Name)

return ctrl.Result{}, nil
return nil
}

func (r *EphemeralRunnerReconciler) createSecret(ctx context.Context, runner *v1alpha1.EphemeralRunner, log logr.Logger) (ctrl.Result, error) {
Expand Down Expand Up @@ -828,6 +828,9 @@ func (r *EphemeralRunnerReconciler) SetupWithManager(mgr ctrl.Manager) error {
For(&v1alpha1.EphemeralRunner{}).
Owns(&corev1.Pod{}).
WithEventFilter(predicate.ResourceVersionChangedPredicate{}).
WithOptions(controller.Options{
MaxConcurrentReconciles: 10,
}).
Complete(r)
}

Expand Down
4 changes: 4 additions & 0 deletions controllers/actions.github.com/ephemeralrunnerset_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)
Expand Down Expand Up @@ -575,6 +576,9 @@ func (r *EphemeralRunnerSetReconciler) SetupWithManager(mgr ctrl.Manager) error
For(&v1alpha1.EphemeralRunnerSet{}).
Owns(&v1alpha1.EphemeralRunner{}).
WithEventFilter(predicate.ResourceVersionChangedPredicate{}).
WithOptions(controller.Options{
MaxConcurrentReconciles: 10,
}).
Complete(r)
}

Expand Down
2 changes: 1 addition & 1 deletion controllers/actions.github.com/ephemeralrunnerset_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func(
startManagers(GinkgoT(), mgr)
})

It("should create a proxy secret and delete the proxy secreat after the runner-set is deleted", func() {
It("should create a proxy secret and delete the proxy secret after the runner-set is deleted", func() {
secretCredentials := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "proxy-credentials",
Expand Down