0.21.0 with organization seems requiring group

[KENNYSOFT]

Describe the bug
I think #1012 broke our workload. We're using the organization with GitHub Team plan, so we're not available to make an additional runner group; only 'Default' exists. We have only organization-level runners like:

apiVersion: actions.summerwind.dev/v1alpha1
kind: RunnerDeployment
metadata:
  namespace: actions-runner-system
  name: github-actions-runnerdeploy
spec:
  template:
    spec:
      organization: myorg
---
apiVersion: actions.summerwind.dev/v1alpha1
kind: HorizontalRunnerAutoscaler
metadata:
  namespace: actions-runner-system
  name: github-actions-runner
spec:
  scaleTargetRef:
    name: github-actions-runnerdeploy
  scaleUpTriggers:
    - githubEvent: {}
      duration: "30m"

To 0.20.4, there were two logs that tried to find HRAs by key:

Found 0 HRAs by key {"key": "myorg/repo-name"}
Found 1 HRAs by key {"key": "myorg"}

But in 0.21.0, only the former one appears.

If I modify RunnerDeployment as:

apiVersion: actions.summerwind.dev/v1alpha1
kind: RunnerDeployment
metadata:
  namespace: actions-runner-system
  name: github-actions-runnerdeploy
spec:
  template:
    spec:
      organization: myorg
      group: Default

now I get two logs but it's somehow different and not understood well.

Found 0 HRAs by key {"key": "myorg/repo-name"}
Found some runner groups are managed by ARC {..., "groups": {}}
groups {..., "groups": {}}
Found 1 HRAs by key {"key": "myorg/group/Default"}
job scale up target found {..., "organization": "myorg", "repository": "repo-name", "key": "myorg/group/Default"}

Checks

• My actions-runner-controller version (v0.x.y) does support the feature
• I'm using an unreleased version of the controller I built from HEAD of the default branch

To Reproduce
Steps to reproduce the behavior:

1. Install v0.20.4, make a HorizontalRunnerAutoscaler with provided manifests, and confirm that it works well.
2. Upgrade to v0.21.0 and see that auto-scale does not work.
3. Add group property to the RunnerDeployment and see it re-works.

Expected behavior
If it's an intended change, it should be documented.

Screenshots
Since the object in logs can contain sensitive information, I can share only some parts of screenshots. Check for the quote block.

▲ With 0.20.4

▲ With 0.21.0, without group: Default added ( RED = myorg, YELLOW = repo-name)

▲ With 0.21.0 and group: Default added

Environment (please complete the following information):

• Controller Version: 0.21.0
• Deployment Method: Argo CD, with Helm values given in apps.argoproj.io/v1alpha1/applications
• Helm Chart Version: 0.16.0

Additional context
Add any other context about the problem here.

[mumoshu]

ARC assumes an empty group as a default runner group so you shouldn't be required to modify your config like

    spec:
      organization: myorg
      group: Default

so this does sound like a regression.

[mumoshu]

@KENNYSOFT Could you share logs from the v0.21.0 gh webhook server without the group: Default modification?

[KENNYSOFT]

@KENNYSOFT Could you share logs from the v0.21.0 gh webhook server without the group: Default modification?

Actually, it's hard because we use this controller in our daily CIs. But it's now the weekend so I'll try to break our system once again. Please wait!

[KENNYSOFT]

Updated the main issue. You can see it does not select even if the event is 'workflow_job'.

[mumoshu]

@KENNYSOFT I was able to reproduce your issue locally and here's the fix- #1134

I'll cut 0.21.1 ASAP so I'd appreciate it if you could give it a try in your next workday 😃

[KENNYSOFT]

Cool, thanks for the quick fix. Also, it's great you've fixed {..., "groups": {}}. I think it's also good if we make some regression tests.

[mumoshu]

@KENNYSOFT I'm trying my best but it turned out to require too much effort 😢

To be clear, it's not that I'm not testing it at all. I'm testing it a lot. I run e2e tests implemented in test/e2e these days, but it's test matrix is very limited. Your issue could have been found only if the test scenario had organization runners only and that's the reason I missed it this time.

If you have some expertise in it, I'd appreciate your contribution to our test suite so that it can be more stable!
In the meantime, I'd appreciate it if you could keep trying upgrading ARC and report issues like this so that I can quickly see it's broken.

Thanks for your help!

[KENNYSOFT]

Oh, for sure I'm not to blame someone at all. I'll happily make some test contributions like #1030 if I have some time. Thanks again for the quick fix, and I understand that making tests is hard work, also the priority is mostly lower than the functionality or fixes. It's totally good and my last comment is just a small wish.

If my comment misleads someone to understand as blame, that's total because of my poor English, please excuse. 🙇