fix: event read from syslog when syslog entry too long

[matejvasek]

When labels map is too big we may get syslog entry truncated.
This breaks JSON parsing making event loading impossible.

ERRO[0016] Unable to decode event: unexpected end of JSON input

By default the limit is 64KiB, this PR set it to 0 == unlimited.

fix: increased event syslog deserialization buffer 

related to:

• Cannot read/write long events from journald (affects docker compat API /containers/(id or name)/wait) #16834
• Allow containers to mount tmpfs_t file systems container-selinux#196

[matejvasek]

/cc @rhatdan @mheon

[matejvasek]

/kind bug

[matejvasek]

This affects container wait endpoint of Docker Compat API, since it relies on events.
It is quite a problem on 4.3+,
on 4.2 it's all right since 4.2 is not dumping labels to log.

[matejvasek]

This is imo worth backporting to 4.3.

[rhatdan]

@giuseppe @Luap99 @mheon PTAL
I wish there was a way to figure out the max, rather then hard coding it.

[matejvasek]

@giuseppe @Luap99 @mheon PTAL
I wish there was a way to figure out the max, rather then hard coding it.

640K ought to be enough for anybody.

[matejvasek]

My specific case needs 104059 bytes.

[Luap99]

From sd_journal_get_data(3)

sd_journal_set_data_threshold() may be used to change the data field size threshold for data returned by sd_journal_get_data(), sd_journal_enumerate_data() and sd_journal_enumerate_unique(). This threshold is a hint only: it indicates that the client program is interested only in the initial parts of the data fields, up to the threshold in size — but the library might still return larger data objects. That means applications should not rely exclusively on this setting to limit the size of the data fields returned, but need to apply an explicit size limit on the returned data as well. This threshold defaults to 64K by default. To retrieve the complete data fields this threshold should be turned off by setting it to 0, so that the library always returns the complete data objects. It is recommended to set this threshold as low as possible since this relieves the library from having to decompress large compressed data objects in full.

Given that we always need to full information we should set it to 0 instead, at least according to this description I have not tested it.

@matejvasek You should also add a test with labels > 64 KiB to make sure we do not regress again.

[matejvasek]

@Luap99 I did add a test at first, but it was failing in CI for some reason so I reverted it.

[matejvasek]

The test worked locally perfectly fine, and I cannot debug what is wrong in CI.

[matejvasek]

I set the threshold to 0 as suggested @Luap99 .

[matejvasek]

@giuseppe @Luap99 @mheon
Any idea why test failing (timing out) in CI? I can only think of events_logger="none", but I doubt that is used there.

[Luap99]

I think API test force --events-backend file. I suggest you create a new test in test/system/090-events.bats, I it is enough to create create a container with a big label and then podman events should fail right now?

[matejvasek]

@Luap99 but the file backend should work too, although it wouldn't test the actuall issue with journald.
Only the none backend should timeout.

[Luap99]

@Luap99 but the file backend should work too, although it wouldn't test the actuall issue with journald. Only the none backend should timeout.

Yeah I am not sure what the API tests use, I recommend to use the system tests and run this for both file and journald driver. This makes it more explicit what is actually being tested.

[mheon]

Code changes LGTM

[matejvasek]

@Luap99 I am close to finishing tests in test/system/090-events.bats.
It has however one tiny issue, the podman CLI and/or testing framework cannot handle big labes 🤣 .

# Error: unable to process labels: bufio.Scanner: token too long

[matejvasek]

@Luap99 @mheon PTAL

[matejvasek]

@Luap99 @mheon are latest test failures flakes? I don't see how they are related to my changes.

[rhatdan]

LGTM

[giuseppe]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, matejvasek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [giuseppe]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

/lgtm

[matejvasek]

@rhatdan @mheon will be this backported to 4.3?

[mheon]

Probably not, we're starting to work on 4.4 for a late January release now.

[matejvasek]

Probably not, we're starting to work on 4.4 for a late January release now.

@mheon
Will Fedora use it soon? What about RHEL?

[mheon]

For Fedora, it should land as soon as it releases (minus delay waiting for the bodhi updates to make it to stable). For RHEL, RHEL 8.8 will get it.

[matejvasek]

@mheon
I am asking because this is affect not only podman events but also Docker compat. API.
Things might not work for people using podman as docker daemon replacement.

[mheon]

How so? Would this be considered a breaking change?

[matejvasek]

@mheon
The wait endpoint must be able to detect transition between running->exited.
In order to do that we use events (I implemented that, polling is not reliable).

Without this PR the events are not written and/or read.
For example this causes freeze for pack CLI.

[matejvasek]

We noticed problems after #15633 (4.3).
In that PR we started writing labels for start and exit events.

[mheon]

Ack, OK. I don't think we're considering another 4.3 release, but if we do this should definitely be in consideration.

[rhatdan]

Yes no 4.3 updates, you will get it in 4.4. We should have RC1 available this week.

[matejvasek]

@rhatdan RHEL 8.7 seems to use 4.2 and RHEL 8.8 will use 4.4 so we are good I guess?

[matejvasek]

Or will any RHEL use 4.3?

[matejvasek]

I noticed v4.3.1-rhel branch exists, how it is used?

[rhatdan]

It is not used, 4.3 was never released to RHEL, this branch was created in anticipation of releasing it to RHEL, but since the releases fell in the wrong way we decided to skip that release. RHEL8.8 and 9.2 will get 4.4

[mheon]

4.3.1-rhel was used for a CentOS stream release or two, I think. It won't make it into RHEL proper.

[matejvasek]

@rhatdan @mheon thanks, that sounds good!

What about the SELinux policy pkg container-selinux v2.198.0 will it available in RHEL and Fedora?

[matejvasek]

@rhatdan @mheon fedora-coreos used by podman machine still uses 4.3.1.
It is an issue for people on macOS.
Maybe we should have backported after all? Or will fedora-coreos receive 4.4 soon?