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

GitHub actions overhaul #228

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

ngehrsitz
Copy link

This PR reworks the Github Actions in this repo:

  • Fixes the broken windows build caused by actions/upload-artifact@v2
  • Updates all action versions
  • Combines all of the docker workflows into a single one that runs for all Dockerfiles and only pushes if not triggered from a PR
  • Add a secondary push to the Github container registry
  • Reduces the permissions of the Github token
  • Skips the push to Docker Hub if the credential secrets are missing which is the case for forks
  • Runs the test always before pushing, not just on PRs

Workflow output: https://github.com/ngehrsitz/NodeODM/actions

@ngehrsitz
Copy link
Author

Maybe #174 could be fixed by setting max-parallel: 1, but that´s not the most elegant solution

@pierotofy
Copy link
Member

pierotofy commented Oct 6, 2024

Thanks for the PR @ngehrsitz 🙏

The upload-artifact action definitely needs fixing. I'm just not sure about combining all actions into a single workflow. It makes the logic more complex to follow (albeit it does decrease some repetition).

I'm also unsure about adding ghcr.io support, for the OpenDroneMap organization that would become (in the near future) an additional cost if I'm reading this correctly. Might be free for now, but ... https://docs.github.com/en/billing/managing-billing-for-your-products/managing-billing-for-github-packages/about-billing-for-github-packages#about-billing-for-github-packages

What's the use case for adding ghcr.io support? Just curious.

@ngehrsitz
Copy link
Author

We could significantly reduce complexity with the https://github.com/docker/bake-action. That moves most of the configuration into separate config files. But I did not find an option to implement the conditional docker hub pushes that way.
The reason for adding ghcr for me was that it makes it easier to to contribute. I can simply fork the project and the CI still keeps working.
Also in terms of it being free I would argue that Github with their opensource commitment is the safer option. Docker Hub already tried to cut down on free hosting and only reverted after public outcry: https://www.docker.com/blog/no-longer-sunsetting-the-free-team-plan/

@pierotofy
Copy link
Member

pierotofy commented Oct 6, 2024

Yes docker has threatened to remove free plans and that's concerning. So far they've been good on their word and have renewed OpenDroneMap's accounts as a sponsored FOSS project twice already, we'll see I guess. Just don't want to worry about two companies (one is plenty already).

Note that for ease of contributing, as far as NodeODM goes, you can just pass the --test argument to index.js at startup, that way you don't need ODM or rebuild containers, you can just develop with NodeJS natively on your machine.

If ghcr.io auto builds are more convenient for you, by all means use them, I just don't think they are a fit for us at the moment.

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