Skip to content

Created a smoke test workflow that installs IIAB with Calibre-Web - #324

Merged
holta merged 18 commits into
iiab:masterfrom
Akatama:smoke-test
Jul 23, 2025
Merged

Created a smoke test workflow that installs IIAB with Calibre-Web#324
holta merged 18 commits into
iiab:masterfrom
Akatama:smoke-test

Conversation

@Akatama

@Akatama Akatama commented Jul 17, 2025

Copy link
Copy Markdown

Pretty much what the title says. Since this is a new workflow it is not changing the original one we have.

@holta

@holta

holta commented Jul 17, 2025

Copy link
Copy Markdown
Member

Looks great!

So it appears chmod 755 /opt/iiab/iiab permissions aren't set by the sudo git clone https://github.com/iiab/iiab step ?

(Does that mean root has an unusual umask within GitHub Actions — instead of the usual 022 ?)

@holta holta added the enhancement New feature or request label Jul 17, 2025
@Akatama

Akatama commented Jul 17, 2025

Copy link
Copy Markdown
Author

So it appears chmod 755 /opt/iiab/iiab permissions aren't set by the sudo git clone https://github.com/iiab/iiab step ?

That's correct, for whatever reason it is 777.

(Does that mean root has an unusual umask within GitHub Actions — instead of the usual 022 ?)

At this moment I cannot confirm, but the result seems to suggest that the umask for root within a GitHub actions runner (VM) is 000

@holta

holta commented Jul 17, 2025

Copy link
Copy Markdown
Member

result seems to suggest that the umask for root within a GitHub actions runner (VM) is 000

Strange, as umask 000 is normally considered very dangerous! (I'd love to understand how it ends up this way, if we can poke at it a bit more and/or find GitHub Actions docs explaining the situation?)

@holta

holta commented Jul 17, 2025

Copy link
Copy Markdown
Member

FWIW this is confusing, as a quick Google search claims:

"GitHub Actions runners use a default umask of 0022"

@holta

holta commented Jul 17, 2025

Copy link
Copy Markdown
Member

ASIDE:

@holta

holta commented Jul 17, 2025

Copy link
Copy Markdown
Member

@Akatama thanks for pointing out this critical issue when we talked earlier today: (that we'd overlooked earlier!)

@holta

holta commented Jul 17, 2025

Copy link
Copy Markdown
Member
  • PR's runner ends up testing the master branch (iiab/calibre-web), rather than the PR's test branch!

Hopefully smoke-test.yml (renamed as appropriate) can fix this by forcibly appending calibreweb_repo_url: ACTUAL-REPO-URL and calibreweb_version: ACTUAL-TEST-BRANCH-NAME onto the end of file: /etc/iiab/local_vars.yml ! 🙏

@Akatama

Akatama commented Jul 19, 2025

Copy link
Copy Markdown
Author

@holta Results from umask testing

Result from running sudo umask on ubuntu-24.04 GitHub Actions runner:

sudo: umask: command not found

Result of running ls -l /opt/iiab/

drwxrwxrwx+ 8 root root 4096 Jul 19 02:35 iiab

This is the same perms as 777

Result of running umask note that means I am just running as whatever the default user is (I think the username is runner)

0022

Based on this, I think the issue is sudo git clone compared to git clone. I can try out if I can clone to the repo without sudo, but it seems like the root user on the VM doesn't have access to the umask command, which suggest it does not have a umask.

@Akatama

Akatama commented Jul 19, 2025

Copy link
Copy Markdown
Author

Even with git clone https://github.com/iiab/iiab (no sudo) the perms for the /opt/iiab/iiab directory still are:

drwxrwxrwx+ 8 runner docker 4096 Jul 19 02:57 iiab

This is still 777. So we really need to set the perms to 755 or something lower.

@holta

holta commented Jul 19, 2025

Copy link
Copy Markdown
Member

root user on the VM doesn't have access to the umask command, which suggest it does not have a umask.

No worries, regular Ubuntu behaves the same way: (REASON: umask is a "shell builtin command")

$ sudo umask
sudo: umask: command not found

Running ls -l / in a generic GitHub Actions runner (Ubuntu 24.04) is revealing:

total 104
drwxr-xr-x   23 root   root  4096 Jul 19 04:11 .
drwxr-xr-x   23 root   root  4096 Jul 19 04:11 ..
lrwxrwxrwx    1 root   root     7 Apr 22  2024 bin -> usr/bin
drwxr-xr-x    2 root   root  4096 Feb 26  2024 bin.usr-is-merged
drwxr-xr-x    5 root   root  4096 Jul  8 04:18 boot
drwxr-xr-x   18 root   root  4020 Jul 19 04:11 dev
drwxr-xr-x  140 root   root 12288 Jul 19 04:11 etc
drwxr-xr-x+   5 root   root  4096 Jul 10 09:44 home
drwxr-xr-x    6 root   root  4096 Jul 10 08:43 imagegeneration
lrwxrwxrwx    1 root   root     7 Apr 22  2024 lib -> usr/lib
drwxr-xr-x    2 root   root  4096 Apr  8  2024 lib.usr-is-merged
lrwxrwxrwx    1 root   root     9 Jul 10 08:07 lib32 -> usr/lib32
lrwxrwxrwx    1 root   root     9 Apr 22  2024 lib64 -> usr/lib64
drwx------    2 root   root 16384 Jul  8 04:16 lost+found
drwxr-xr-x    2 root   root  4096 Jul  8 04:13 media
drwxr-xr-x    3 root   root  4096 Jul 19 03:31 mnt
drwxrwxrwx+  15 runner root  4096 Jul 19 04:23 opt
dr-xr-xr-x  211 root   root     0 Jul 19 04:11 proc
drwx------   20 root   root  4096 Jul 10 08:41 root
drwxr-xr-x   33 root   root   980 Jul 19 04:11 run
lrwxrwxrwx    1 root   root     8 Apr 22  2024 sbin -> usr/sbin
drwxr-xr-x    2 root   root  4096 Mar 31  2024 sbin.usr-is-merged
drwxr-xr-x    2 root   root  4096 Jul 10 07:56 snap
drwxr-xr-x    2 root   root  4096 Jul  8 04:13 srv
dr-xr-xr-x   12 root   root     0 Jul 19 04:11 sys
drwxrwxrwt   13 root   root 12288 Jul 19 04:23 tmp
drwxr-xr-x   13 root   root  4096 Jul 10 08:07 usr
drwxr-xr-x   14 root   root  4096 Jul 10 08:40 var

Running ls -ld $GITHUB_WORKSPACE likewise:

drwxr-xr-x   2 runner docker 4096 Jul 19 04:23 /home/runner/work/iiab/iiab

TAKEAWAYS:

  • chmod permissions are inherited from the parent directory by default — regardless how children are created, e.g. by any command, or e.g. by any user (runner or root if using sudo, etc).

  • Should we need it later (just FWIW now!) running getfacl $GITHUB_WORKSPACE /opt/iiab reveals ACL detail beyond the original chmod tradition — i.e. if a "+" sign appears on the right side of chmod permissions: (e.g. drwxrwxrwx+)

# file: home/runner/work/iiab/iiab
# owner: runner
# group: docker
user::rwx
group::r-x
other::r-x

# file: opt/iiab
# owner: runner
# group: docker
user::rwx
user:runner:rwx
group::rwx
mask::rwx
other::rwx
default:user::rwx
default:user:runner:rwx
default:group::rwx
default:mask::rwx
default:other::rwx

@Akatama

Akatama commented Jul 19, 2025

Copy link
Copy Markdown
Author

@holta I think I got calibreweb_repo_url and calibreweb_version set to the correct info. However, we do seem to have a problem. Let's look at what local_vars.yml looks like on the smoke-test branch on my fork:

# LAST UPDATED: October 10, 2024

# USAGE: https://github.com/iiab/calibre-web/wiki#wrench-installation

runcmd:
  - mkdir -p /etc/iiab
  - |
    cat >> /etc/iiab/local_vars.yml << EOF
    calibreweb_install: True
    calibreweb_enabled: True
    nodocs: True
    kolibri_install: False
    kolibri_enabled: False
    kiwix_install: False
    kiwix_enabled: False
    osm_vector_maps_install: False
    awstats_install: False
    awstats_enabled: False
    matomo_install: False
    matomo_enabled: False
    captiveportal_install: False
    network_install: False
    network_enabled: False
    admin_console_install: False
    admin_console_enabled: False
    EOF
  - curl iiab.io/risky.txt | bash &
calibreweb_repo_url: https://github.com/Akatama/calibre-web
calibreweb_version: smoke-test

So I was able to get the URL for my repo and the name of my branch. However, the run failed on main because the smoke-test branch does not exist on iiab/callibre-web:

# LAST UPDATED: October 10, 2024

# USAGE: https://github.com/iiab/calibre-web/wiki#wrench-installation

runcmd:
  - mkdir -p /etc/iiab
  - |
    cat >> /etc/iiab/local_vars.yml << EOF
    calibreweb_install: True
    calibreweb_enabled: True
    nodocs: True
    kolibri_install: False
    kolibri_enabled: False
    kiwix_install: False
    kiwix_enabled: False
    osm_vector_maps_install: False
    awstats_install: False
    awstats_enabled: False
    matomo_install: False
    matomo_enabled: False
    captiveportal_install: False
    network_install: False
    network_enabled: False
    admin_console_install: False
    admin_console_enabled: False
    EOF
  - curl iiab.io/risky.txt | bash &
calibreweb_repo_url: https://github.com/iiab/calibre-web
calibreweb_version: smoke-test

@holta

holta commented Jul 19, 2025

Copy link
Copy Markdown
Member

Right!

We don't need all of omg.yml — all we really need is this part...

  - |
    cat >> /etc/iiab/local_vars.yml << EOF
    calibreweb_install: True
    calibreweb_enabled: True
    nodocs: True
    kolibri_install: False
    kolibri_enabled: False
    kiwix_install: False
    kiwix_enabled: False
    osm_vector_maps_install: False
    awstats_install: False
    awstats_enabled: False
    matomo_install: False
    matomo_enabled: False
    captiveportal_install: False
    network_install: False
    network_enabled: False
    admin_console_install: False
    admin_console_enabled: False
    EOF

@holta

holta commented Jul 19, 2025

Copy link
Copy Markdown
Member

@Akatama maybe try pasting those 20 lines (essentially) directly into your .github/workflows/iiab-install-smoke-test.yml ?

(In the "Set up /etc/iiab/local_vars.yml" section!)

@Akatama

Akatama commented Jul 19, 2025

Copy link
Copy Markdown
Author

maybe try pasting those 20 lines (essentially) directly into your .github/workflows/iiab-install-smoke-test.yml ?

Sure, but that's not going to fix the problem that iiab/calibre-web doesn't have a smoke-test branch (or whatever branch exists on someone else's fork)

@Akatama

Akatama commented Jul 19, 2025

Copy link
Copy Markdown
Author

To be clear, this is the fatal error from the workflow run on iiab/calibre-web:

ASK [calibre-web : git clone Calibre-Web (smoke-test) from https://github.com/iiab/calibre-web to /usr/local/calibre-web-py3 (~132 MB initially, ~296+ MB later) -- if /usr/local/calibre-web-py3 doesns't exist] ***
fatal: [127.0.0.1]: FAILED! => {"changed": false, "cmd": "/usr/bin/git checkout --force smoke-test", "msg": "Failed to checkout smoke-test", "rc": 1, "stderr": "error: pathspec 'smoke-test' did not match any file(s) known to git\n", "stderr_lines": ["error: pathspec 'smoke-test' did not match any file(s) known to git"], "stdout": "", "stdout_lines": []}

@Akatama

Akatama commented Jul 19, 2025

Copy link
Copy Markdown
Author

@holta Most recent run on iiab/calibre-web failed for the same reason. I have to get to bed, and I am not sure I will have the energy to work on this tomorrow as I will be throwing my daughter Chloe her 4th birthday party.

Here is the error again, just so you can see it hasn't change since I updated local_vars.yml:

TASK [calibre-web : git clone Calibre-Web (smoke-test) from https://github.com/iiab/calibre-web to /usr/local/calibre-web-py3 (~132 MB initially, ~296+ MB later) -- if /usr/local/calibre-web-py3 doesns't exist] ***
fatal: [127.0.0.1]: FAILED! => {"changed": false, "cmd": "/usr/bin/git checkout --force smoke-test", "msg": "Failed to checkout smoke-test", "rc": 1, "stderr": "error: pathspec 'smoke-test' did not match any file(s) known to git\n", "stderr_lines": ["error: pathspec 'smoke-test' did not match any file(s) known to git"], "stdout": "", "stdout_lines": []}

@holta

holta commented Jul 19, 2025

Copy link
Copy Markdown
Member

@Akatama when you get to it!

Here are GitHub Actions variables that capture the true {OWNER, REPO and BRANCH} from any commit — even if the commit is not part of a PR:

$GITHUB_REPOSITORY   e.g. Akatama/iiab
$GITHUB_REF_NAME     e.g. smoke-test

Good enough❓❗

A few other GitHub Actions vars FWIW:

$GITHUB_REPOSITORY_OWNER   e.g. Akatama
$GITHUB_TRIGGERING_ACTOR   e.g. Akatama
$GITHUB_ACTOR              e.g. Akatama

@holta

holta commented Jul 19, 2025

Copy link
Copy Markdown
Member

we really need to set the perms to 755

So be it for now!

(The root cause would appear to be GitHub Actions' choice to set unusual permissions for /opt — i.e. 777 + ACL.)

@holta

holta commented Jul 19, 2025

Copy link
Copy Markdown
Member

@Akatama no rush — but later this weekend I'd be curious if you think we really need to use EthanSK/git-branch-name-action@v1 to kick off these GitHub Actions?

My guess is that GitHub Actions's own variables are sufficient for our needs — ~4 years after EthanSK created the above (but I could be wrong!)

@Akatama

Akatama commented Jul 19, 2025

Copy link
Copy Markdown
Author

@Akatama no rush — but later this weekend I'd be curious if you think we really need to use EthanSK/git-branch-name-action@v1 to kick off these GitHub Actions?

I was searching for how to grab the branch name, and this was the first answer I found that seemed like it could work.

In another comment you are suggesting $GITHUB_REF_NAME, so I can take a look at using that. If it still says smoke-test, we will still have the same problem.

@holta

holta commented Jul 19, 2025

Copy link
Copy Markdown
Member

In another comment you are suggesting $GITHUB_REF_NAME, so I can take a look at using that. If it still says smoke-test, we will still have the same problem.

  1. Can you explain? I'm not following:

    I thought the branch name (e.g. "smoke-test" in this case) was exactly what we wanted.

  2. FYI I suggested $GITHUB_REF_NAME as it appears to work far better than $GIT_BRANCH_NAME (providing the branch name in many more situations, e.g. with commits not yet part of a PR, etc).

@Akatama

Akatama commented Jul 19, 2025

Copy link
Copy Markdown
Author

@holta This is what the last two lines of local_vars.yml looks like on the run from the PR:

calibreweb_repo_url: https://github.com/iiab/calibre-web
calibreweb_version: 324/merge

Compared to the run from my fork, I think this means it may work!

calibreweb_repo_url: https://github.com/Akatama/calibre-web
calibreweb_version: smoke-test

@holta

holta commented Jul 19, 2025

Copy link
Copy Markdown
Member

Ah well! (It failed but I'll dig further later today...)

TASK [calibre-web : git clone Calibre-Web (324/merge) from https://github.com/iiab/calibre-web to /usr/local/calibre-web-py3 (~132 MB initially, ~296+ MB later) -- if /usr/local/calibre-web-py3 doesns't exist] ***
fatal: [127.0.0.1]: FAILED! => {"changed": false, "cmd": "/usr/bin/git checkout --force 324/merge", "msg": "Failed to checkout 324/merge", "rc": 1, "stderr": "error: pathspec '324/merge' did not match any file(s) known to git\n", "stderr_lines": ["error: pathspec '324/merge' did not match any file(s) known to git"], "stdout": "", "stdout_lines": []}

PS after we get this right, later on I'd personally suggest we tighten up a bit, by dumping all ~20 lines directly into /etc/iiab/local_vars.yml using 1 single "heredoc".

@Akatama

Akatama commented Jul 20, 2025

Copy link
Copy Markdown
Author

@holta It looks like git would use something like this to pull from a PR:

git pull origin pull/324/head

I am hoping if we convert 324/merge to pull/324/head then we can get this working. There may be an easier way to go about it, but to be clear about what I think we would need to do if we followed this idea.

Check $GITHUB_REF_NAME

  • If $GITHUB_REF_NAME does not include '/merge' then we can assume we are pulling from the repo directly and not a PR, so no change is needed to the workflow for this case
  • If $GITHUB_REF_NAME does include '/merge', we take the PR number from before the / and use it to constrcut pull/<PR_NUM>/head, then set that as calibreweb_version

@holta

holta commented Jul 20, 2025

Copy link
Copy Markdown
Member

@Akatama check this out which seems to work well — for both event "pull_request" and event "push":

  1. Workflows triggered by PRs work:
    https://github.com/iiab/calibre-web/actions/workflows/10min-iiab-smoke-test.yml

  2. Workflows triggered by "pushes" i.e. commits on (forked) test branches also work: (in future we might avoid duplicate jobs, i.e. when 1. and 2. trigger simultaneously ;)
    https://github.com/holta/calibre-web/actions/workflows/10min-iiab-smoke-test.yml

  3. For the third kind of event however, I have not yet tested a workflow triggered by a "workflow_dispatch" (i.e. manual trigger) — that requires the workflow be in the default branch (i.e. master branch), so we can test that later — after something is merged!

PS We should get to know (or at least learn something about) GitHub Action contexts at some point 💫

@Akatama

Akatama commented Jul 22, 2025

Copy link
Copy Markdown
Author

@holta Take a look, I took a lot from your branch. Thanks for doing that, if I didn't have two big events this last weekend I would've done that research.

I removed some comments, and I tested the workflow_dispatch for this workflow on the master branch of my fork - it works flawlessly. Let me know if you see any more adjustments that are needed.

@holta

holta commented Jul 22, 2025

Copy link
Copy Markdown
Member

I tested the workflow_dispatch for this workflow on the master branch of my fork - it works flawlessly

(Great idea, wow!)

We should merge this later today I think. I can do that when I have a bit more time PM.

Feel free to add any fine-tuning or explanation. e.g. if this can be a model for other future workflows validating more specific functionality.

@Akatama

Akatama commented Jul 22, 2025

Copy link
Copy Markdown
Author

Maybe its worth it to run the tests we have now after install. Just to be sure Calibre-Web is reachable.

In the future, if our tests take up significant time, we may want to have this workflow just run the most basic ones and have another workflow that does the install and runs them all.

@holta

holta commented Jul 22, 2025

Copy link
Copy Markdown
Member
  1. @thotmx @deldesir @ZachLiebl &/or @mpimble can one of you review this today, please?

  2. Thanks @Akatama for your hard work making this foundation come together — towards a very supportive CI (community infra ;) environment helping nudge contributors-of-all-kind in healthy/organic directions!

@thotmx thotmx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me.

I have a few questions:

  • Running the install script successful is the smoke test?
  • Wouldn't be required to verify something else? like something is running? or a port was open? or something like that?

Just questions, if I'm missing something please let me known.

@holta @Akatama

@holta

holta commented Jul 23, 2025

Copy link
Copy Markdown
Member

@thotmx that's a great question:

  • Validation checks need to be added to other PRs that build on this one, creating relevant "sanity tests" of particular functionalities?

  • Exactly as you laid out in November 2024, thanks to Selenium especially?

  • Is that reasonable, allowing this PR to be merged today, e.g. as a foundation (common install path) for others?

@Akatama might have other suggestions! e.g. he mentioned possible ways we might want to lay this out + evolve this going forward:

we may want to have this workflow just run the most basic ones and have another workflow that does the install and runs them all.

@thotmx thotmx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@holta
Agree. For me it's ok to merge it, and use this as a foundation to add extra validations.

And it would help to keep simple PRs, to make the reviewing process easier.

@holta
holta merged commit e89cdf9 into iiab:master Jul 23, 2025
1 check passed
@Akatama
Akatama deleted the smoke-test branch July 24, 2025 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants