-
Notifications
You must be signed in to change notification settings - Fork 467
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
[wip] github-actions: use cache-image in devshell workflow #3798
base: master
Are you sure you want to change the base?
[wip] github-actions: use cache-image in devshell workflow #3798
Conversation
While with the current setup it has very limited functionality, it allows to change registries without code change by utilising the "rules.conf" file or an environment variable. Signed-off-by: Laszlo Szemere <[email protected]>
GitHub Actions can filter Workflow runs based on the source tree. The "paths" keyword has the same effect as the dbld/rules cache-image-% Changed the default value of the CONTAINER_REGISTRY variable. Signed-off-by: Laszlo Szemere <[email protected]>
This change is a result of a long discussion, trying to summarize the reasons behind it. There were two main concerns regarding builder images: Are there anybody who wants to maintain a custom cache for dbld images? Can we support it without a code change on the forked repository? The answer for the first question is probably none or very few. Keep in mind: we still build the docker images in case of dbld code changes, so one can be sure that nothing breaks. We just skip the upload step. The answer to the second question is YES. We already do it without this commit, so this is a step backwards. However using those images in the GitHub Actions of the forked repository already requires a code change. example: 1) Remove the if clause from the dbld-images.yml action. Or simply revert this commit. 2) Add the env: CONTAINER_REGISTRY: ghcr.io/${{ github.actor }} to the selected action. (i.e.: packages.yml) Signed-off-by: Laszlo Szemere <[email protected]>
Signed-off-by: Laszlo Szemere <[email protected]>
The main purpose of the scheduled job is to detect any problem with the images as early as possible. Changes in upstream packages can break our images without a code change on our side. By not uploading the result, we can preserve the last working version in the cache. This will give us some time to fix the issue, and will not block other - non related - PR's from merging. To avoid code duplication, I extracted the if condition of image upload. Signed-off-by: Laszlo Szemere <[email protected]>
This is an aesthetic change. We use GitHub's container registry to host our dbld builder images. This service is referred as "Packages" and will show up on the project's main page. We wanted to avoid any confusion with the binary packages of Syslog-ng. Signed-off-by: Laszlo Szemere <[email protected]>
It is usefull for testing changes on forked repositories. Cron jobs and push events do not set the input field. (Not even the default value.) Signed-off-by: Laszlo Szemere <[email protected]>
Depends on syslog-ng#3782 Signed-off-by: Laszlo Szemere <[email protected]>
Signed-off-by: Attila Szakacs <[email protected]>
Signed-off-by: Attila Szakacs <[email protected]>
5b9d1f0
to
add015f
Compare
Signed-off-by: Attila Szakacs <[email protected]>
add015f
to
e21a1be
Compare
Build FAILURE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please rebase the PR.
|
||
- name: Determine cached image ID | ||
run: | | ||
./dbld/rules pull-image-devshell || true # remove the || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a left-in TODO?
if: env.USE_CACHED_IMAGE == 'false' | ||
run: | | ||
docker push ${{ steps.determine-actual-image.outputs.docker-image }} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we push this logic into dbld/rules itself?
I mean:
- we already have a pull-image invoked from cache-image, and we have a separate step here. I'd love to see a single cache-image here and do anything to produce an image (potentially form cache) within dbld/rules
- we seem to do a lot of back and forth deciding whether we are using the pulled or the rebuilt image, which is then used to decide if we need to push the temporary image.
- What if dbld/rules cache-image would be able to return information that we need here, without having to do a prefetch
- also what if cache-image would be able to push to the registry, if a specific make variable is set (and if we did a rebuild)
I hope I make sense here, my point is if we push this entire logic to dbld/rules, and add a means to communicate information in a structured form, then we might be able to use the same cache-image-* target in all similar cases, avoiding the need to copy steps between github workflows files.
What do you think?
Depends on #3782 and #3793
Signed-off-by: Attila Szakacs [email protected]