forgejo-runner (FIX): support generated/unattended mode and configurable runner labels#1645
Open
WaffleThief123 wants to merge 8 commits intocommunity-scripts:mainfrom
Open
forgejo-runner (FIX): support generated/unattended mode and configurable runner labels#1645WaffleThief123 wants to merge 8 commits intocommunity-scripts:mainfrom
WaffleThief123 wants to merge 8 commits intocommunity-scripts:mainfrom
Conversation
Open
Author
|
I recorded each of my tests using https://github.com/asciinema/asciinema and https://github.com/asciinema/agg to render them as gifs after modifying out internal information (storage and template devices, network info, forgejo url and key) as well as removing long waits (OS/container updates, installs, etc). Each test case is shown below. Test 1Test 1: Interactive install: verify prompts appear for instance URL, token, and additional labels
Test 2Test 2: Interactive install with no extra labels: verify only linux-amd64:docker://node:22-bookworm is registered
Test 3Test 3: Interactive install with extra labels: verify default + custom labels are both registered
Test 4 has been withheld at this time, pending question asked in issue #1576 (comment) Test 5Test 5: Unattended install with missing token: verify hard error and clean exit
|
…le runner labels - Export app-specific variables (var_forgejo_instance, var_forgejo_runner_token, var_runner_labels) in ct script so they survive lxc-attach into the container - Replace nonexistent prompt_input_required/show_missing_values_warning calls with standard read prompts that skip when variables are pre-set - Add hard error when no runner token is provided - Make runner labels configurable via var_runner_labels instead of hardcoded - Update default runner image from node:20-bookworm to node:22-bookworm (Node 22 LTS)
CT script hardcoded the upstream URL for build.func, so the install script was always fetched from upstream — ignoring fork/branch fixes. Now respects COMMUNITY_SCRIPTS_URL override, consistent with how build.func already handles this for all other resource fetching.
core.func overwrites $HOSTNAME with a formatted emoji string for display purposes, causing runner registration to send garbled ANSI codes as the runner name. Use $(hostname) to get the actual system hostname.
forgejo-runner register writes .runner config to the current working directory. The install script runs in a temp directory (via build.func), so the config was lost on cleanup. The systemd service expects it in /root (WorkingDirectory=/root), so cd there before registering.
Abort before build_container if mode is set (unattended) but no runner registration token was provided. Avoids a 20+ minute container build only to fail at the registration step.
…active Check for missing token/URL only when mode is set AND there's no TTY, so mode=default with a terminal still allows interactive prompts inside the container.
mode is only set when the user is automating (e.g. mode=default with var_* overrides). The default bash -c curl invocation leaves mode empty and shows the whiptail menu, so this check won't affect interactive use.
WaffleThief123
force-pushed
the
forgejo-runner-fix
branch
from
1b19d62 to
17b2229
Compare
WaffleThief123
changed the title
WaffleThief123
changed the title
Member
|
@lengschder97 could you take a look and review this? @WaffleThief123 can you revert the change at the very top with the URL for sourcing the funcs? |
CrazyWolf13
requested changes
Apr 23, 2026
Comment on lines
+35
to
+39
| if [[ -n "${var_runner_labels:-}" ]]; then | ||
| RUNNER_LABELS="${DEFAULT_RUNNER_LABELS},${var_runner_labels}" | ||
| else | ||
| RUNNER_LABELS="${DEFAULT_RUNNER_LABELS}" | ||
| fi |
Member
There was a problem hiding this comment.
can't you do something like this?
var_keyctl="${var_keyctl:-1}"
|
|
||
| msg_info "Registering Forgejo Runner" | ||
| export DOCKER_HOST="unix:///run/podman/podman.sock" | ||
| cd /root |
| --token "$FORGEJO_RUNNER_TOKEN" \ | ||
| --name "$HOSTNAME" \ | ||
| --labels "linux-amd64:docker://node:20-bookworm" \ | ||
| --name "$(hostname)" \ |
Member
There was a problem hiding this comment.
I think hostname var is set by our core funcs, why does this need changing?
| msg_ok "Created Services" | ||
|
|
||
| # Show warning if any required values used fallbacks | ||
| show_missing_values_warning |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✍️ Description
NOTE: I have weighed using
node:ltsas the image tag, however that would make the CI runner not deploy in the same manner every time, undermining a core principle in the DevOps world: Reproducible Builds.🔗 Related PR / Issue
Still requires testing, will do so following the test plan outlined below in Additional Information✅ Prerequisites (X in brackets)
🛠️ Type of Change (X in brackets)
README,AppName.md,CONTRIBUTING.md, or other docs.🔍 Code & Security Review (X in brackets)
Code_Audit.md&CONTRIBUTING.mdguidelinesAppName.sh,AppName-install.sh,AppName.json)📋 Additional Information
Test plan
- Unattended install with mode="generated" and all var_* set: verify no prompts, container created with correct configTest 4 has been withheld at this time, pending question asked in issue Forgejo-Runner #1576 (comment)Once steps have been tested (and gif of output provided here) I will update and remove DRAFT, indicating ready for merge.
📦 Application Requirements (for new scripts)
brevity removal
🌐 Source
brevity removal