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

Re-factor fluentd CI workflows #1472

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

Conversation

joshuabaird
Copy link
Collaborator

@joshuabaird joshuabaird commented Feb 17, 2025

This PR re-factors the CI workflows for building and publishing fluent-operator's fluentd image.

Why the re-factor?

  • The fluentd image had not been touched for quite some time and there was a lot of old cruft (old Dockerfiles, etc)
  • The old build process was using QEMU for building arm64 images on amd64 and it was prone to issues (seg faults in modern gcc stacks) and was very slow (20+ minutes to build an image)
  • We were publishing a "base" image which was not actually being used
  • We were building the fluentd image from "scratch" instead of basing our image off of the upstream fluent/fluentd image and there was no benefit to doing so
  • The build process was actually broken for new (v1.17.1+) images of fluentd

The re-factor

  • Removes unused/legacy Dockerfiles and uses a single Dockerfile for both amd64 and arm64 images
  • The fluentd image is now based from the upstream fluent/fluentd image which makes it easier to maintain
  • Builds both amd64 and arm64 images
  • Builds arm64 images on native arm64 runners instead of using QEMU which makes it much faster (~3min vs 20+ min) and simplifies our Dockerfile
  • Uses Debian-based upstream fluentd images so that we get standard jmellaoc support
  • Replaces the clone-docker-image-action CI workflow with native functionality that publishes multi-arch images/manifests to both GHCR and Docker Hub
  • Temporarily removes the "image scan" workflow since we were publishing images regardless of the result
  • Adopts the VERSION file approach that was implemented for fluent-bit in feat: add VERSION file for fluentbit image #1447

Future enhancements

We can re-factor the fluent-bit CI to follow this same pattern and perhaps we can use a single workflow to build both fluent-bit and fluentd images.

Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
platforms: ${{ matrix.platform }}
labels: ${{ steps.image-metadata.outputs.labels }}
provenance: false
outputs: type=image,"name=${{ env.GHCR_REPO }},${{ env.DOCKERHUB_REPO }}",push-by-digest=true,name-canonical=true,push=true
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This step produces arm64 and amd64 digests and pushes them to both GHCR and Docker Hub.

username: ${{ secrets.REGISTRY_USER }}
password: ${{ secrets.REGISTRY_PASSWORD }}

- name: Create image manifest
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This step uses the arm64 and amd64 image digests that we pushed previously and creates a single multi-arch manifest tagged with the fluentd version.

Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
@joshuabaird
Copy link
Collaborator Author

cc: @benjaminhuo, @cw-Guo and @wenchajun for review.

Once merged, we'll publish a v1.17.1 (and maybe v1.18.0) image of fluentd.

@joshuabaird joshuabaird marked this pull request as ready for review February 18, 2025 16:25
@joshuabaird joshuabaird requested a review from cw-Guo February 18, 2025 16:27

LABEL org.opencontainers.image.description "A Fluentd image for use with fluent-operator"

USER root
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see some of the previous Dockerfiles run as non-root user, for example: https://github.com/fluent/fluent-operator/pull/1472/files#diff-57700540a4a9a3e075df0d7035170b3d3e4559e3a9e9fcef66ac0996f85447ebL61-L66.

Not sure whether we want to implement this too in the new dockerfile. From my understanding, it will enhance the security.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cw-Guo i would welcome the images to run as non-root, this is a Best Practice

Copy link
Collaborator Author

@joshuabaird joshuabaird Feb 19, 2025

Choose a reason for hiding this comment

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

Good callout -- I'll get this fixed up to run as a non-root user. This was actually an oversight.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is now fixed -- confirmed that the process is running as the fluent user:

$ USER         PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
fluent         1  0.0  0.0 1226248 2840 ?        Ssl  20:43   0:00 /fluentd/bin/fluentd-watcher
fluent        13  5.1  0.6 172192 50512 ?        Sl   20:43   0:00 /usr/local/bin/ruby /usr/local/bundle/bin/fluentd -c /fluentd/etc/fluent.conf -p /fluentd/plugins

Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
Signed-off-by: Josh Baird <[email protected]>
@@ -20,7 +20,7 @@ import (
)

const (
defaultBinPath = "/usr/bin/fluentd"
defaultBinPath = "/usr/local/bundle/bin/fluentd"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The default path for the fluentd binary (both amd64 and arm64) is /usr/local/bundle/bin in the upstream images (which we are now using).

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.

3 participants