Add ephemeral runner finalizer during creation and check finalizer without requeue

This commit is contained in:
Nikola Jokic 2025-11-20 17:26:58 +01:00
parent 138b39bfcb
commit 6b345a919c
No known key found for this signature in database
GPG Key ID: 554517D3D15A5D2F
4 changed files with 86 additions and 70 deletions

View File

@ -154,31 +154,17 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, nil
}
if !controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerFinalizerName) {
log.Info("Adding finalizer")
addFinalizers := !controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerFinalizerName) || !controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerActionsFinalizerName)
if addFinalizers {
log.Info("Adding finalizers")
if err := patch(ctx, r.Client, ephemeralRunner, func(obj *v1alpha1.EphemeralRunner) {
controllerutil.AddFinalizer(obj, ephemeralRunnerFinalizerName)
controllerutil.AddFinalizer(obj, ephemeralRunnerActionsFinalizerName)
}); err != nil {
log.Error(err, "Failed to update with finalizer set")
return ctrl.Result{}, err
}
log.Info("Successfully added finalizer")
return ctrl.Result{}, nil
}
if !controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerActionsFinalizerName) {
log.Info("Adding runner registration finalizer")
err := patch(ctx, r.Client, ephemeralRunner, func(obj *v1alpha1.EphemeralRunner) {
controllerutil.AddFinalizer(obj, ephemeralRunnerActionsFinalizerName)
})
if err != nil {
log.Error(err, "Failed to update with runner registration finalizer set")
return ctrl.Result{}, err
}
log.Info("Successfully added runner registration finalizer")
return ctrl.Result{}, nil
log.Info("Successfully added finalizers")
}
secret := new(corev1.Secret)

View File

@ -32,9 +32,8 @@ import (
)
const (
ephemeralRunnerSetTestTimeout = time.Second * 20
ephemeralRunnerSetTestInterval = time.Millisecond * 250
ephemeralRunnerSetTestGitHubToken = "gh_token"
ephemeralRunnerSetTestTimeout = time.Second * 20
ephemeralRunnerSetTestInterval = time.Millisecond * 250
)
func TestPrecomputedConstants(t *testing.T) {
@ -119,8 +118,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
Consistently(
func() (int, error) {
runnerList := new(v1alpha1.EphemeralRunnerList)
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
if err != nil {
if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil {
return -1, err
}
@ -153,8 +151,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
Eventually(
func() (int, error) {
runnerList := new(v1alpha1.EphemeralRunnerList)
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
if err != nil {
if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil {
return -1, err
}
@ -172,8 +169,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
}
if refetch {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
if err != nil {
if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil {
return -1, err
}
}
@ -215,8 +211,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
Eventually(
func() (int, error) {
runnerList := new(v1alpha1.EphemeralRunnerList)
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
if err != nil {
if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil {
return -1, err
}
@ -234,8 +229,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
}
if refetch {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
if err != nil {
if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil {
return -1, err
}
}
@ -253,8 +247,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
Eventually(
func() (int, error) {
runnerList := new(v1alpha1.EphemeralRunnerList)
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
if err != nil {
if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil {
return -1, err
}
@ -299,8 +292,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
if err != nil {
if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil {
return -1, err
}
@ -326,8 +318,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
if err != nil {
if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil {
return -1, err
}
@ -351,7 +342,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -378,7 +369,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -403,7 +394,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -429,7 +420,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -456,7 +447,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -481,7 +472,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Consistently(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -508,7 +499,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -545,7 +536,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
// We should have 3 runners, and have no Succeeded ones
Eventually(
func() error {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return err
}
@ -582,7 +573,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -607,7 +598,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Eventually(
func() error {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return err
}
@ -648,7 +639,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
// We should have 1 runner up and pending
Eventually(
func() error {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return err
}
@ -675,7 +666,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -702,7 +693,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -728,7 +719,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Eventually(
func() error {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return err
}
@ -772,8 +763,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Consistently(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
if err != nil {
if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil {
return -1, err
}
@ -799,7 +789,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -826,7 +816,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -853,7 +843,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Eventually(
func() error {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return err
}
@ -897,7 +887,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList = new(v1alpha1.EphemeralRunnerList)
Eventually(
func() error {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return err
}
@ -938,7 +928,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(
func() (bool, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return false, err
}
@ -1046,7 +1036,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
Eventually(
func() (int, error) {
runnerList = new(v1alpha1.EphemeralRunnerList)
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -1125,7 +1115,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() {
FIt("should create a proxy secret and delete the proxy secreat after the runner-set is deleted", func() {
secretCredentials := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "proxy-credentials",
@ -1208,7 +1198,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func(
Eventually(func(g Gomega) {
runnerList := new(v1alpha1.EphemeralRunnerList)
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
g.Expect(err).NotTo(HaveOccurred(), "failed to list EphemeralRunners")
for _, runner := range runnerList.Items {
@ -1226,7 +1216,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func(
Eventually(
func(g Gomega) (int, error) {
runnerList := new(v1alpha1.EphemeralRunnerList)
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -1245,7 +1235,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func(
}
if refetch {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -1260,6 +1250,18 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func(
err = k8sClient.Delete(ctx, ephemeralRunnerSet)
Expect(err).NotTo(HaveOccurred(), "failed to delete EphemeralRunnerSet")
Eventually(func(g Gomega) (int, error) {
runnerList := new(v1alpha1.EphemeralRunnerList)
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
return len(runnerList.Items), nil
},
ephemeralRunnerSetTestTimeout,
ephemeralRunnerSetTestInterval,
).Should(BeEquivalentTo(0), "EphemeralRunners should be deleted")
// Assert that the proxy secret is deleted
Eventually(func(g Gomega) {
proxySecret := &corev1.Secret{}
@ -1343,7 +1345,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func(
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -1490,7 +1492,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func(
runnerList := new(v1alpha1.EphemeralRunnerList)
Eventually(func() (int, error) {
err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace)
if err != nil {
return -1, err
}
@ -1529,3 +1531,27 @@ var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func(
).Should(BeTrue(), "server was not called")
})
})
// helper function to remove ephemeral runners since in the test, ephemeral runner reconciler is not started
func listEphemeralRunnersAndRemoveFinalizers(ctx context.Context, k8sClient client.Client, list *v1alpha1.EphemeralRunnerList, namespace string) error {
err := k8sClient.List(ctx, list, client.InNamespace(namespace))
if err != nil {
return err
}
// Since we are not starting ephemeral runner reconciler, ignore
liveItems := make([]v1alpha1.EphemeralRunner, 0)
for _, item := range list.Items {
if !item.DeletionTimestamp.IsZero() {
if err := patch(ctx, k8sClient, &item, func(runner *v1alpha1.EphemeralRunner) {
runner.Finalizers = []string{}
}); err != nil {
return err
}
continue
}
liveItems = append(liveItems, item)
}
list.Items = liveItems
return nil
}

View File

@ -572,12 +572,15 @@ func (b *ResourceBuilder) newEphemeralRunner(ephemeralRunnerSet *v1alpha1.Epheme
annotations[AnnotationKeyPatchID] = strconv.Itoa(ephemeralRunnerSet.Spec.PatchID)
return &v1alpha1.EphemeralRunner{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
GenerateName: ephemeralRunnerSet.Name + "-runner-",
Namespace: ephemeralRunnerSet.Namespace,
Labels: labels,
Annotations: annotations,
Finalizers: []string{
ephemeralRunnerFinalizerName,
ephemeralRunnerActionsFinalizerName,
},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: ephemeralRunnerSet.GetObjectKind().GroupVersionKind().GroupVersion().String(),

View File

@ -11,11 +11,12 @@ export PLATFORMS="linux/amd64"
TARGETS=()
function set_targets() {
local cases="$(find "${TEST_DIR}" -name '*.test.sh' | sed "s#^${TEST_DIR}/##g" )"
local cases
cases="$(find "${TEST_DIR}" -name '*.test.sh' | sed "s#^${TEST_DIR}/##g")"
mapfile -t TARGETS < <(echo "${cases}")
echo $TARGETS
echo "${TARGETS[@]}"
}
function env_test() {
@ -89,4 +90,4 @@ function main() {
fi
}
main $@
main "$@"