Skip to content

Comments

dockerfiles: backport systemd lib fixes from #3177 to 1.8#4567

Merged
niedbalski merged 3 commits intofluent:1.8from
equinix-ms:backport-3177
Jan 12, 2022
Merged

dockerfiles: backport systemd lib fixes from #3177 to 1.8#4567
niedbalski merged 3 commits intofluent:1.8from
equinix-ms:backport-3177

Conversation

@jonkerj
Copy link

@jonkerj jonkerj commented Jan 5, 2022

Testing
Before we can approve your change; please submit the following in a comment:

  • [N/A] Example configuration file for the change
  • [N/A] Debug log output from testing the change
  • [N/A] Attached Valgrind output that shows no leaks or memory corruption was found

Documentation

  • [N/A] Documentation required for this feature

The 1.8 branch is not working on recent Flatcar Linux systems, because of incompatibility in systemd libs. #3177 took care of that, but targets master (ie, future 1.9). This PR is a cherry pick of @yasn77's commit, but targets 1.8.

What I did to test:

  • git checkout v1.8.11
  • git cherry-pick 3f991077
  • docker build -f dockerfiles/Dockerfile.x86_64-master
  • use that image on modern Flatcar Linux machines

@patrick-stephens
Copy link
Contributor

I presume you have but for completeness, what testing did you do? Best to follow the template if you can.
Also you'll need to update the PR title with dockerfiles: ..

@jonkerj jonkerj changed the title Backport systemd lib fixes from #3177 to 1.8 dockerfiles: backport systemd lib fixes from #3177 to 1.8 Jan 5, 2022
@patrick-stephens patrick-stephens mentioned this pull request Jan 7, 2022
5 tasks
@patrick-stephens
Copy link
Contributor

patrick-stephens commented Jan 7, 2022

So this is almost good to go from a CI perspective, I'll let someone else review the actual content. Ignore the mergebot failure.

@niedbalski
Copy link
Contributor

DCO needs fixing

@jonkerj
Copy link
Author

jonkerj commented Jan 7, 2022

I amended @yasn77 's commit, so it my sign-off as well.

@patrick-stephens patrick-stephens added backport to v1.8.x Used to tag items that must be backported to such version. and removed docs-required labels Jan 7, 2022
This commit is originally from @yasn77, but I had to sign it myself in
order to pass DCO checks. Commit message below is originally theirs.

The libraries created in the docker image can not read the journal db in newer
versions of systemd. This specifically impacts users that run systemd `246` or
above on their host server:

> * systemd-journald gained support for zstd compression of large fields in
> journal files. The hash tables in journal files have been hardened against
> hash collisions. This is an incompatible change and means that journal files
> created with new systemd versions are not readable with old versions. If the
> $SYSTEMD_JOURNAL_KEYED_HASH boolean environment variable for
> systemd-journald.service is set to 0 this new hardening functionality may be
> turned off, so that generated journal files remain compatible with older
> journalctl implementations.

(Taken from https://github.com/systemd/systemd/blob/v246/NEWS)

This commit uses the backported version of the systemd libs, which are `247` for
Debian buster. Fixing it for most distributions.

It might be worth considering having a separate build stage for systemd, that
would give better flexibility of the version shipped in the image.

Signed-off-by: Yasser Saleemi <yassersaleemi@gmail.com>
Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>
@patrick-stephens
Copy link
Contributor

@jonkerj is there any impact on ARM builds? I notice this is just for AMD64 targets and we've been burnt with missing changes on ARM before so wanted to check.

@jonkerj
Copy link
Author

jonkerj commented Jan 10, 2022

I'm not the author of this change (@yasn77 is), but looking at the changed files, it affects amd64, armv7 and arm64v8, so we're good here.

@patrick-stephens
Copy link
Contributor

patrick-stephens commented Jan 10, 2022

Oh yeah, sorry not sure how or why I missed that!

Did you test the arm ones at least run up? I resolved an issue previously with a change which just broke them hence my concern: #4404

@jonkerj
Copy link
Author

jonkerj commented Jan 10, 2022

Nope, sorry, I cannot test this easily. But I think if it builds (which is does) and runs on AMD64 (ditto), there is no reason why it would not work on Arm, as the changes incorporate installing the same backported packages.

@patrick-stephens
Copy link
Contributor

Ok I'll see if I can verify here just to confirm. Then we're good for merge.

@patrick-stephens
Copy link
Contributor

patrick-stephens commented Jan 12, 2022

Changes required

One slight issue is that a separate change has bumped the 1.8 branch to use 1.8.12 which doesn't exist (yet) so fails to build this PR. I therefore specified the 1.8.11 tarball during build:

docker build --build-arg FLB_TARBALL=https://github.com/fluent/fluent-bit/archive/v1.8.11.tar.gz -t backport-3177:arm32 -f ./dockerfiles/Dockerfile.arm32v7 ./dockerfiles/

This then triggers what I was worried about, namely #4404.

docker run --rm -it backport-3177:arm32
WARNING: The requested image's platform (linux/arm/v7) does not match the detected host platform (linux/amd64) and no specific platform was requested
/fluent-bit/bin/fluent-bit: error while loading shared libraries: libatomic.so.1: cannot open shared object file: No such file or directory

ARM 64 does not even build:

docker build --build-arg FLB_TARBALL=https://github.com/fluent/fluent-bit/archive/v1.8.11.tar.gz -t backport-3177:arm64 -f ./dockerfiles/Dockerfile.arm64v8 ./dockerfiles/
...
Step 9/55 : RUN apt update && apt install -y --no-remove --no-install-recommends     build-essential     g++-aarch64-linux-gnu     gcc-aarch64-linux-gnu     ca-certificates     pkg-config     cmake     make     flex     bison     dpkg-dev     libssl-dev:arm64     libsasl2-dev:arm64     libsystemd-dev:arm64/buster-backports     libzstd-dev:arm64     zlib1g-dev:arm64     libpq-dev:arm64
....
The following packages have unmet dependencies:
 libsystemd-dev:arm64 : Depends: libsystemd0:arm64 (= 247.3-6~bpo10+1) but it is not going to be installed
E: Unable to correct problems, you have held broken packages.

Therefore we need to include the changes from #4405 and #4408 .

Signed-off-by: Patrick Stephens <pat@calyptia.com>

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>
Signed-off-by: Patrick Stephens <pat@calyptia.com>

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>
@jonkerj
Copy link
Author

jonkerj commented Jan 12, 2022

Therefore we need to include the changes from #4405 and #4408 .

I've cherry-picked both (5c6b520 and 95253bf), and added my sign-off.

EDIT: meh, I booboo'd. I could have clicked "accept changed". What do you want me to do, @patrick-stephens ?

@patrick-stephens
Copy link
Contributor

patrick-stephens commented Jan 12, 2022

Therefore we need to include the changes from #4405 and #4408 .

I've cherry-picked both (5c6b520 and 95253bf), and added my sign-off.

EDIT: meh, I booboo'd. I could have clicked "accept changed". What do you want me to do, @patrick-stephens ?

I'm not sure I follow.
I pulled and rebuilt the PR so now ARM 32 is ok it seems:

$ docker build --build-arg FLB_TARBALL=https://github.com/fluent/fluent-bit/archive/v1.8.11.tar.gz -t backport-3177:arm32 -f ./dockerfiles/Dockerfile.arm32v7 ./dockerfiles/
$ docker run --rm -it backport-3177:arm32
WARNING: The requested image's platform (linux/arm/v7) does not match the detected host platform (linux/amd64) and no specific platform was requested
Fluent Bit v1.8.11
* Copyright (C) 2019-2021 The Fluent Bit Authors
* Copyright (C) 2015-2018 Treasure Data
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

[2022/01/12 13:51:32] [ info] [engine] started (pid=1)
[2022/01/12 13:51:32] [ info] [storage] version=1.1.5, initializing...
[2022/01/12 13:51:32] [ info] [storage] in-memory
[2022/01/12 13:51:32] [ info] [storage] normal synchronization mode, checksum disabled, max_chunks_up=128
[2022/01/12 13:51:32] [ info] [cmetrics] version=0.2.2
[2022/01/12 13:51:32] [ info] [sp] stream processor started
[0] cpu.local: [1641995493.512272071, {"cpu_p"=>4.687500, "user_p"=>4.187500, "system_p"=>0.500000, "cpu0.p_cpu"=>3.000000, "cpu0.p_user"=>2.000000, "cpu0.p_system"=>1.000000, "cpu1.p_cpu"=>8.000000, "cpu1.p_user"=>7.000000, "cpu1.p_system"=>1.000000, "cpu2.p_cpu"=>2.000000, "cpu2.p_user"=>1.000000, "cpu2.p_system"=>1.000000, "cpu3.p_cpu"=>10.000000, "cpu3.p_user"=>9.000000, "cpu3.p_system"=>1.000000, "cpu4.p_cpu"=>2.000000, "cpu4.p_user"=>2.000000, "cpu4.p_system"=>0.000000, "cpu5.p_cpu"=>2.000000, "cpu5.p_user"=>2.000000, "cpu5.p_system"=>0.000000, "cpu6.p_cpu"=>5.000000, "cpu6.p_user"=>4.000000, "cpu6.p_system"=>1.000000, "cpu7.p_cpu"=>25.000000, "cpu7.p_user"=>24.000000, "cpu7.p_system"=>1.000000, "cpu8.p_cpu"=>4.000000, "cpu8.p_user"=>3.000000, "cpu8.p_system"=>1.000000, "cpu9.p_cpu"=>0.000000, "cpu9.p_user"=>0.000000, "cpu9.p_system"=>0.000000, "cpu10.p_cpu"=>0.000000, "cpu10.p_user"=>0.000000, "cpu10.p_system"=>0.000000, "cpu11.p_cpu"=>0.000000, "cpu11.p_user"=>0.000000, "cpu11.p_system"=>0.000000, "cpu12.p_cpu"=>5.000000, "cpu12.p_user"=>4.000000, "cpu12.p_system"=>1.000000, "cpu13.p_cpu"=>3.000000, "cpu13.p_user"=>2.000000, "cpu13.p_system"=>1.000000, "cpu14.p_cpu"=>4.000000, "cpu14.p_user"=>4.000000, "cpu14.p_system"=>0.000000, "cpu15.p_cpu"=>0.000000, "cpu15.p_user"=>0.000000, "cpu15.p_system"=>0.000000}]

ARM 64 also looks good now:

$ docker build --build-arg FLB_TARBALL=https://github.com/fluent/fluent-bit/archive/v1.8.11.tar.gz -t backport-3177:arm64 -f ./dockerfiles/Dockerfile.arm64v8 ./dockerfiles/^C
$ docker run --rm -it backport-3177:arm64
WARNING: The requested image's platform (linux/arm64/v8) does not match the detected host platform (linux/amd64) and no specific platform was requested
Fluent Bit v1.8.11
* Copyright (C) 2019-2021 The Fluent Bit Authors
* Copyright (C) 2015-2018 Treasure Data
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

[2022/01/12 14:01:46] [ info] [engine] started (pid=1)
[2022/01/12 14:01:46] [ info] [storage] version=1.1.5, initializing...
[2022/01/12 14:01:46] [ info] [storage] in-memory
[2022/01/12 14:01:46] [ info] [storage] normal synchronization mode, checksum disabled, max_chunks_up=128
[2022/01/12 14:01:46] [ info] [cmetrics] version=0.2.2
[2022/01/12 14:01:46] [ info] [sp] stream processor started
[0] cpu.local: [1641996107.515537199, {"cpu_p"=>0.250000, "user_p"=>0.125000, "system_p"=>0.125000, "cpu0.p_cpu"=>1.000000, "cpu0.p_user"=>1.000000, "cpu0.p_system"=>0.000000, "cpu1.p_cpu"=>0.000000, "cpu1.p_user"=>0.000000, "cpu1.p_system"=>0.000000, "cpu2.p_cpu"=>1.000000, "cpu2.p_user"=>0.000000, "cpu2.p_system"=>1.000000, "cpu3.p_cpu"=>0.000000, "cpu3.p_user"=>0.000000, "cpu3.p_system"=>0.000000, "cpu4.p_cpu"=>0.000000, "cpu4.p_user"=>0.000000, "cpu4.p_system"=>0.000000, "cpu5.p_cpu"=>0.000000, "cpu5.p_user"=>0.000000, "cpu5.p_system"=>0.000000, "cpu6.p_cpu"=>1.000000, "cpu6.p_user"=>1.000000, "cpu6.p_system"=>0.000000, "cpu7.p_cpu"=>1.000000, "cpu7.p_user"=>1.000000, "cpu7.p_system"=>0.000000, "cpu8.p_cpu"=>0.000000, "cpu8.p_user"=>0.000000, "cpu8.p_system"=>0.000000, "cpu9.p_cpu"=>0.000000, "cpu9.p_user"=>0.000000, "cpu9.p_system"=>0.000000, "cpu10.p_cpu"=>0.000000, "cpu10.p_user"=>0.000000, "cpu10.p_system"=>0.000000, "cpu11.p_cpu"=>1.000000, "cpu11.p_user"=>1.000000, "cpu11.p_system"=>0.000000, "cpu12.p_cpu"=>0.000000, "cpu12.p_user"=>0.000000, "cpu12.p_system"=>0.000000, "cpu13.p_cpu"=>0.000000, "cpu13.p_user"=>0.000000, "cpu13.p_system"=>0.000000, "cpu14.p_cpu"=>0.000000, "cpu14.p_user"=>0.000000, "cpu14.p_system"=>0.000000, "cpu15.p_cpu"=>1.000000, "cpu15.p_user"=>1.000000, "cpu15.p_system"=>0.000000}]

@jonkerj is there any test you want me to run? It'll have to be a simple one just to verify the packages are the right versions, etc.

@jonkerj
Copy link
Author

jonkerj commented Jan 12, 2022

EDIT: meh, I booboo'd. I could have clicked "accept changed". What do you want me to do, @patrick-stephens ?

I'm not sure I follow. I pulled and rebuilt the PR so now ARM 32 is ok it seems:

After I cherry picked the changes to my branch by hand, I noticed you have suggested changes using GitHub UI, which I could have accepted. So I either revert the cherry picks, force-push, and accept your suggestions, or I discard them, as the branch is OK now.

@patrick-stephens
Copy link
Contributor

EDIT: meh, I booboo'd. I could have clicked "accept changed". What do you want me to do, @patrick-stephens ?

I'm not sure I follow. I pulled and rebuilt the PR so now ARM 32 is ok it seems:

After I cherry picked the changes to my branch by hand, I noticed you have suggested changes using GitHub UI, which I could have accepted. So I either revert the cherry picks, force-push, and accept your suggestions, or I discard them, as the branch is OK now.

Ah right, I'm happy with it as it stands. Up to @niedbalski to approve though.

@patrick-stephens
Copy link
Contributor

@edsiper so you're aware of a request for a new 1.8 release (at least container images)

@niedbalski niedbalski merged commit 929eeaf into fluent:1.8 Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport to v1.8.x Used to tag items that must be backported to such version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants