Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: build 32 bit and 64 bit images #1690

Merged
merged 12 commits into from
Jan 28, 2025
Merged

Conversation

Dhanus3133
Copy link
Contributor

/claim #1688

@billz
Copy link
Member

billz commented Oct 31, 2024

@Dhanus3133 thanks for the PR! I'll give it a round of testing first thing tomorrow.

@Dhanus3133
Copy link
Contributor Author

@Dhanus3133 thanks for the PR! I'll give it a round of testing first thing tomorrow.

Thanks! However, I’m running into an issue with the custom user stage for package installation. You can check the logs here.

Error: pi-gen build failed with exit code 100
mkfs.fat: Warning: lowercase labels might not work properly on some systems
mke2fs 1.46.2 (28-Feb-2021)
E: Failed to fetch http://archive.raspberrypi.com/debian/pool/main/r/raspberrypi-sys-mods/raspberrypi-sys-mods_20241030_arm64.deb  Temporary failure resolving 'archive.raspberrypi.com'
E: Failed to fetch http://archive.raspberrypi.com/debian/pool/main/u/userconf-pi/userconf-pi_0.10_all.deb  Temporary failure resolving 'archive.raspberrypi.com'
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?

When I skip the custom stage, the CI build from my fork runs successfully, as build images in this release. I also tried reaching out to you via Discord.

@Dhanus3133
Copy link
Contributor Author

@billz resolved the issue. It should now be good for testing. You can use the build image from the fork.

@billz
Copy link
Member

billz commented Nov 1, 2024

@Dhanus3133 I'll give each image a thorough test. thanks!

@billz
Copy link
Member

billz commented Nov 2, 2024

Testing the 64-bit image. A couple of observations:

  1. The optional flags are set to '0' (--openvpn 0 --restapi 0 --adblock 0 --wireguard 0 --tcp-bbr 0). Per the issue, these values should be '1'.
  2. The dhcpcd package isn't present in the final image, which is required to assign IP addresses to clients. This isn't necessarily a fault of the action, but possbily related to ubuntu-latest used by pi-gen and the OS detection done by the installer.

The first point should be easily fixed. Will need to give the latter a closer look. Otherwise looking good.

@billz
Copy link
Member

billz commented Nov 2, 2024

Interestingly, the build-raspap-image (64-bit, arm64) log indicates that the package is installed:

RaspAP Install: Installing required packages
  php8.2-cgi will be installed from the main deb sources list
  dhcpcd5, iw and rsync will be installed from the main deb sources list

@billz
Copy link
Member

billz commented Nov 2, 2024

Also noticed that routing and IP masquerading fails in the build:

Checking iptables rules
/usr/bin/grep: /etc/iptables/rules.v4: No such file or directory
Adding rule: -t nat -A POSTROUTING -j MASQUERADE
iptables: Failed to initialize nft: Protocol not supported
[ error ]  Unable to execute iptables 
/usr/bin/grep: /etc/iptables/rules.v4: No such file or directory
Adding rule: -t nat -A POSTROUTING -s 192.168.50.0/24 ! -d 192.168.50.0/24 -j MASQUERADE
iptables: Failed to initialize nft: Protocol not supported
[ error ]  Unable to execute iptables 
Persisting IP tables rules
iptables-save/1.8.9 (nf_tables) Failed to initialize nft: Protocol not supported
[ error ]  Unable to execute iptables-save

I haven't looked more closely to determine the cause. Hopefully these are minor fixes that can be resolved.

@Dhanus3133
Copy link
Contributor Author

  1. The optional flags are set to '0' (--openvpn 0 --restapi 0 --adblock 0 --wireguard 0 --tcp-bbr 0). Per the issue, these values should be '1'.

Regarding this, I believe enabling OpenVPN may be causing a DNS issue in the export-image stage, specifically during the package installation for the user rename step (linked here for reference: https://github.com/RPi-Distro/pi-gen/blob/master/export-image/01-user-rename/00-packages). This error seems related to network configuration changes when OpenVPN is enabled, as described in this related comment. Do you know if there’s a workaround to bypass this DNS issue?

@billz
Copy link
Member

billz commented Nov 2, 2024

Do you know if there’s a workaround to bypass this DNS issue?

See #1689 (comment)

@Dhanus3133
Copy link
Contributor Author

Dhanus3133 commented Nov 2, 2024

See #1689 (comment)

Thanks, this didn't work out, but there was a simple way to fix it.

Also noticed that routing and IP masquerading fails in the build:

Checking iptables rules
/usr/bin/grep: /etc/iptables/rules.v4: No such file or directory
Adding rule: -t nat -A POSTROUTING -j MASQUERADE
iptables: Failed to initialize nft: Protocol not supported
[ error ]  Unable to execute iptables 
/usr/bin/grep: /etc/iptables/rules.v4: No such file or directory
Adding rule: -t nat -A POSTROUTING -s 192.168.50.0/24 ! -d 192.168.50.0/24 -j MASQUERADE
iptables: Failed to initialize nft: Protocol not supported
[ error ]  Unable to execute iptables 
Persisting IP tables rules
iptables-save/1.8.9 (nf_tables) Failed to initialize nft: Protocol not supported
[ error ]  Unable to execute iptables-save

I haven't looked more closely to determine the cause. Hopefully these are minor fixes that can be resolved.

All the required packages are installed, but since the actual error is hidden, I'm not sure how to solve this issue. Let me know what do you think about this.

You can find the latest build images for all the enabled flags here.

PS: Can you please verify if that installed properly, cause I noticed the new build images are much more smaller than the previous builds.

@billz
Copy link
Member

billz commented Nov 3, 2024

Pulling the latest 64-bit image now.

@Dhanus3133
Copy link
Contributor Author

@billz Can you test this release build for the commit 38ce0ec? You can find the release here: test release. It should work, but please note that this build was created through a custom fork of my pi-gen repository. You can check the changes here on the arm64 branch: pi-gen/arm64.

I believe these changes are required to ensure the builds succeed without any issues related to networking.

If you think this is a viable solution, we can consider forking pi-gen into the RaspAP org, make the necessary changes, and use that fork repository within the pi-gen-action.

Let me know your thoughts!

@billz
Copy link
Member

billz commented Jan 7, 2025

@Dhanus3133 I'll make time and re-test with these changes soon. thanks!

@billz
Copy link
Member

billz commented Jan 28, 2025

@Dhanus3133 I tested RaspAP's core functionality in the raspap-image-test-64-bit-custom image and it worked flawlessly! Well done!

From what I saw in your pi-gen/arm64 fork, changing the execution order in export-image was the trick. Is this correct?

So to integrate this we'd fork your changes to pi-gen into the RaspAP org and specify it in the release workflow:

     - name: Build RaspAP Image
        id: build
        uses: usimd/pi-gen-action@v1

Let me know if I've got this right, thanks.

@Dhanus3133
Copy link
Contributor Author

Dhanus3133 commented Jan 28, 2025

From what I saw in your pi-gen/arm64 fork, changing the execution order in export-image was the trick. Is this correct?

@billz Yes, I have pushed all the required changes in the recent commit. Here is the build release.

So to integrate this we'd fork your changes to pi-gen into the RaspAP org and specify it in the release workflow:

      - name: Build RaspAP Image
        id: build
        uses: usimd/pi-gen-action@v1
        with:
          image-name: "raspap-${{ github.ref_name }}-${{ matrix.arch }}"
          enable-ssh: 1
          stage-list: stage0 stage1 stage2 ./stage-raspap
          verbose-output: true
          pi-gen-version: ${{ matrix.pi_gen_version }}
+         pi-gen-repository: RaspAP/pi-gen

I have added this. You can fork pi-gen into RaspAP and apply this patch to the arm64 branch and everything should work as expected! :)

@billz
Copy link
Member

billz commented Jan 28, 2025

Makes sense. I'll fork the repo and apply your patch. This is looking really good :)

@billz billz merged commit d689024 into RaspAP:master Jan 28, 2025
3 checks passed
@Dhanus3133
Copy link
Contributor Author

@billz I checked the fork you created, and it seems you might have forgotten to uncheck this option.

image
And applying patch to the arm64 branch is alone enough since only the arm64 build fails, and the master is for 32 bit build which works without any changes.

@billz
Copy link
Member

billz commented Jan 28, 2025

@Dhanus3133 you're absolutely right. I'll create a new fork. thanks for catching this.

@billz
Copy link
Member

billz commented Jan 28, 2025

Alright, patch applied to the arm64 branch.

Your latest 32- and 64-bit builds both passed, so I should expect the same on the next published release.

Adding workflow_dispatch would allow manual (on demand) builds, no?

on:
  release:
    types: [published]
  workflow_dispatch:

I assume that without a corresponding release or tag there won't be a valid destination for upload-release-action. This isn't critical, mostly just curious.

I'll award you the bounty and give you a shout on our various channels (x, discord, reddit, etc).
Thanks again for your exceptional work on this!

@Dhanus3133
Copy link
Contributor Author

Dhanus3133 commented Jan 28, 2025

Thanks! Yes, adding workflow_dispatch will indeed allow manual triggers. When triggered manually, the branch name will be used for the artifact name.

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/accessing-contextual-information-about-workflow-runs
github.ref_name - The short ref name of the branch or tag that triggered the workflow run. This value matches the branch or tag name shown on GitHub. For example, feature-branch-1.

@billz
Copy link
Member

billz commented Jan 28, 2025

I see that now, thanks! Probably could have answered my own question by searching 😉

@billz
Copy link
Member

billz commented Jan 29, 2025

@Dhanus3133 what do you think about opening a PR with your patch on the arm64 branch of RPi-Distro/pi-gen with a concise description of the issue you found?

@Dhanus3133
Copy link
Contributor Author

@Dhanus3133 what do you think about opening a PR with your patch on the arm64 branch of RPi-Distro/pi-gen with a concise description of the issue you found?

Yes, I had tried here. I did DM'ed regarding this one on Discord seems you had missed it.

@billz
Copy link
Member

billz commented Jan 29, 2025

Yes, I had tried RPi-Distro/pi-gen#805. I did DM'ed regarding this one on Discord seems you had missed it.

I see. Well, it's not a big issue. Just thought it could potentially help other users. We'll maintain the pi-gen fork going forward. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants