runrole: Accept CLI arguments (and `--reinstall` flag) in any position by holta · Pull Request #4146 · iiab/iiab

holta · 2025-11-13T04:25:03Z

Fixes bug:

familiar zoom and scroll control on maps #4145 (comment)

Description of changes proposed in this pull request:

Refactor command-line argument parsing and update logging path handling.

Smoke-tested on which OS or OS's:

Ubuntu 26.04 (latest pre-release) but needs more testing! @muthuri-dev can you help?

Please see https://FAQ.IIAB.IO for an explanation (& examples) on how to use the ./runrole <IIAB ROLE> command (as root!) after you run: cd /opt/iiab/iiab

Mention a team member @username e.g. to help with code review:

@orblivion @muthuri-dev

Refactor command-line argument parsing and update logging path handling.

Refactor command-line argument parsing to use a case statement for future extensibility. Shift command is now consistently applied.

Comment out ROLE_NAME and ROLE_VAR initializations.

orblivion · 2025-11-13T21:00:40Z

Just for consideration - I wonder how much this actually bothers people, vs the risk of bug and added complication to the code. I don't think I ever got this wrong myself in the shell, I just typed it wrong in a github comment. Also, at a certain point it may be worth just writing in python? Just for the argument parsing.

orblivion · 2025-11-13T21:01:05Z

I don't speak fluent bash, I'm more at "conversational level", but I'll give it a look

orblivion · 2025-11-13T21:05:26Z

+                ROLE_NAME="$1"
+                export ANSIBLE_LOG_PATH="$CWD/iiab-debug.log"
+            else
+                export ANSIBLE_LOG_PATH="$1"


So this goes ./runrole ROLE_NAME ANSIBLE_LOG_PATH where ANSIBLE_LOG_PATH is optional and --reinstall can be stuck in anywhere after ./runrole?

If that's right, maybe you could assert (with a "too many arguments" failure) that ANSIBLE_LOG_PATH is not already set. The way it is now, you could have extra arguments that don't do anything and it only takes the last arg as ANSIBLE_LOG_PATH. It wouldn't break anything but maybe you could "fail fast" for the benefit of someone who accidentally put in an extra argument.

Done: only hard-core implementers use ./runrole but I agree with the spirit...

So now it will at least warn people who blindly try ./runrole <ROLE1> <ROLE2> <ROLE3>

Though I'm not going to worry about users who specify --reinstall multiple time (as you say, no harm done!)

orblivion · 2025-11-13T21:06:25Z

+    shift
+done
+
+if [ ! -v ROLE_NAME ]; then    # Test whether var is STILL not set!


And what if it's not? The rest of this script should fail, right?

Nevermind, this is the fail! (exit 0). Though, should it be exit 1?

> rmdir rmdir: missing operand Try 'rmdir --help' for more information. > echo $? 1

Refactor role name and log path handling in runrole script.

holta · 2025-11-13T22:23:11Z

@muthuri-dev can you test this newly refined ./runrole <ROLE> command after it's merged — even if you cannot today?!

holta · 2025-11-14T00:42:10Z

@deldesir would you be able to look over this PR quickly?

deldesir · 2025-11-14T00:43:33Z

Testing it right now...

deldesir

Smoke tested on U25.10. Does the job.

deldesir · 2025-11-14T01:06:25Z

TIL I can run 1+ roles in a single run!

UPDATE: Only the first one (Calibre-Web) ran...

holta · 2025-11-14T01:32:14Z

TIL I can run 1+ roles in a single run!

Sorry "runrole" doesn't do that yet !!

Rearranged the logging path export to ensure it is set correctly.

jvonau · 2025-11-18T13:46:44Z

Where did the log file go to?

holta · 2025-11-18T14:02:47Z

Where did the log file go to?

Log file should be in the same place as before.

Is there a bug or specific failure?

jvonau · 2025-11-18T15:22:14Z

jvonau@pi500:/opt/iiab/iiab $ sudo git pull origin master
remote: Enumerating objects: 75, done.
remote: Counting objects: 100% (65/65), done.
remote: Compressing objects: 100% (42/42), done.
remote: Total 75 (delta 37), reused 46 (delta 23), pack-reused 10 (from 2)
Unpacking objects: 100% (75/75), 29.17 KiB | 807.00 KiB/s, done.
From https://github.com/iiab/iiab

branch master -> FETCH_HEAD
985d024..272ae8c master -> origin/master
Updating 985d024..272ae8c
Fast-forward
roles/kiwix/templates/iiab-make-kiwix-lib | 2 +-
roles/kiwix/templates/iiab-make-kiwix-lib3.py | 13 +++++++++----
roles/maps/README.md | 38 +++++++++++++++++++++++++++++++++++++-
runrole | 59 ++++++++++++++++++++++++++++++++++++++---------------------
vars/local_vars_android.yml | 35 +++++++++++++++++++++++++++++++++++
5 files changed, 120 insertions(+), 27 deletions(-)
create mode 100644 vars/local_vars_android.yml

jvonau@pi500:/opt/iiab/iiab $ ls
ansible.cfg iiab-debug.log iiab-network install-support.yml.unused run-one-role.yml tests.unused
ansible_hosts iiab-from-cmdline.yml iiab-network.log LICENSE runrole test.yml
collections.yml iiab-from-console.yml iiab-network.yml LICENSING.md runroles unmaintained-roles.txt
CONTRIBUTING.md iiab-install iiab-setup README.md runroles-base.yml vars
iiab-configure iiab-install.log iiab-stages.yml roles scripts
jvonau@pi500:/opt/iiab/iiab $ sudo mv iiab-debug.log iiab-debug.log.old
jvonau@pi500:/opt/iiab/iiab $ sudo ./runrole bluetooth
jvonau@pi500:/opt/iiab/iiab $ ls
ansible.cfg iiab-debug.log.old iiab-network install-support.yml.unused run-one-role.yml tests.unused
ansible_hosts iiab-from-cmdline.yml iiab-network.log LICENSE runrole test.yml
collections.yml iiab-from-console.yml iiab-network.yml LICENSING.md runroles unmaintained-roles.txt
CONTRIBUTING.md iiab-install iiab-setup README.md runroles-base.yml vars
iiab-configure iiab-install.log iiab-stages.yml roles scripts

holta · 2025-11-18T15:53:00Z

Thanks @jvonau. Likely a bug that slipped through. I'll take a look.

jvonau · 2025-11-18T20:33:55Z

TIL I can run 1+ roles in a single run!
UPDATE: Only the first one (Calibre-Web) ran...

If you want to run multiple roles I suggest using iiab-configure but a reinstall would require removing the role from /etc/iiab/iiab_state.yml by hand something that was addressed 5 years ago within #2500, the companion iiab/iiab-factory#134 and related #2771

tim-moody · 2025-11-18T22:51:19Z

there's also runroles, a mini version of running configure in Adm Cons. But the caveat about uninstalling applies I think.

runrole: Accept CLI arguments in any position

5fa3301

Refactor command-line argument parsing and update logging path handling.

holta added this to the 8.3 milestone Nov 13, 2025

holta requested review from orblivion and sir-muthuri November 13, 2025 04:25

holta added the enhancement label Nov 13, 2025

runrole: Clean argument parsing

6e2aea8

Refactor command-line argument parsing to use a case statement for future extensibility. Shift command is now consistently applied.

holta changed the title ~~runrole: Accept CLI arguments in any position~~ runrole: Accept CLI arguments (and --reinstall flag) in any position Nov 13, 2025

Don't forget runrole default: ROLE_VAR="$ROLE_NAME"

6e19398

Comment out ROLE_NAME and ROLE_VAR initializations.

holta marked this pull request as ready for review November 13, 2025 07:12

orblivion reviewed Nov 13, 2025

View reviewed changes

holta added 2 commits November 13, 2025 17:01

Tighten up runrole, and detect "Too many arguments"

e9a9fa3

Refactor role name and log path handling in runrole script.

runrole: Highlight all vars (e.g. ANSIBLE_LOG_PATH) on top

df70dd9

holta mentioned this pull request Nov 13, 2025

"Unable to restart service dnsmasq" during "20 min" IIAB "MEDIUM" on Ubuntu 24.04 on x86-64 / test-install [race condition?] #4099

Open

holta requested a review from deldesir November 14, 2025 00:42

deldesir approved these changes Nov 14, 2025

View reviewed changes

holta merged commit 00d6aec into iiab:master Nov 14, 2025
4 of 5 checks passed

holta added a commit that referenced this pull request Nov 15, 2025

runrole: Cleaner code path (minor refinement of PR #4146)

ce29eff

Rearranged the logging path export to ensure it is set correctly.

jvonau mentioned this pull request Nov 18, 2025

fix logging [ANSIBLE_LOG_PATH for runrole] #4153

Merged

Conversation

holta commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes bug:

Description of changes proposed in this pull request:

Smoke-tested on which OS or OS's:

Mention a team member @username e.g. to help with code review:

Uh oh!

orblivion commented Nov 13, 2025

Uh oh!

orblivion commented Nov 13, 2025

Uh oh!

orblivion Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

holta Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

orblivion Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

orblivion Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

holta commented Nov 13, 2025

Uh oh!

holta commented Nov 14, 2025

Uh oh!

deldesir commented Nov 14, 2025

Uh oh!

deldesir left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deldesir commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

holta commented Nov 14, 2025

Uh oh!

Uh oh!

jvonau commented Nov 18, 2025

Uh oh!

holta commented Nov 18, 2025

Uh oh!

jvonau commented Nov 18, 2025

Uh oh!

holta commented Nov 18, 2025

Uh oh!

jvonau commented Nov 18, 2025

Uh oh!

tim-moody commented Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

holta commented Nov 13, 2025 •

edited

Loading

orblivion Nov 13, 2025 •

edited

Loading

deldesir left a comment •

edited

Loading

deldesir commented Nov 14, 2025 •

edited

Loading