system service: unset listen fds on tcp

[vrothberg]

Disable leaking the LISTEN_* variables into containers which are
observed to be passed by systemd even without being socket activated as
described in https://access.redhat.com/solutions/6512011.

[NO NEW TESTS NEEDED] - Ultimately, the solution 6512011 should be updated.

Fixes: bugzilla.redhat.com/show_bug.cgi?id=2180483

@Luap99 PTAL. test/e2e/systemd_activate_test.go does not set LISTEN_FDS, so there's no test I can plug into.

Does this PR introduce a user-facing change?

Fix an issue in `system service` when listing on TCP and LISTEN_FDS are set.

[Luap99]

Why only for tcp? We should unset all three LISTEN_ envs after this block for everything, there is no reason to leak them ever into the container when started via service.

I still would like to understand why tcp misbehaves in that case?

[Luap99]

LISTEN_FDS are usually not passed but I suspect in the BZ it's happening as a systemd.socket with the same name exists; even if not triggered via the socket

This sounds like a systemd bug to me.

[vrothberg]

Why only for tcp? We should unset all three LISTEN_ envs after this block for everything, there is no reason to leak them ever into the container when started via service.

That is a good idea.

I still would like to understand why tcp misbehaves in that case?

I also smell a systemd bug.

[vrothberg]

I also smell a systemd bug.

Ah well. I don't think so anymore. It does not happen when socket isn't active. For that to happen, it must 1) be removed from the service dependencies and 2) be stopped.

[Luap99]

Agreed I think systemd is behaving as documented here. The .service unit has a implicit dependency on the socket. So if you start the service it will automatically trigger the socket unit and pass the fds down.

Socket activated services are automatically ordered after their activating .socket units via an automatic After= dependency. Services also pull in all .socket units listed in Sockets= via automatic Wants= and After= dependencies.

I think this patch to unset the env is correct but I still do not understand why that is causing issues with dropping tcp packets.

[vrothberg]

I think this patch to unset the env is correct but I still do not understand why that is causing issues with dropping tcp packets.

That is indeed puzzling.

[vrothberg]

Good to go from my end. Apologies, I did not manage to reproduce the issues outside this very specific scenario of manually editing the installed .service.

[Luap99]

LGTM

[vrothberg]

@flouthoc PTAL

[flouthoc]

LGTM
/lgtm
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, Luap99, vrothberg

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 [Luap99,flouthoc,vrothberg]

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

[vrothberg]

/cherry-pick v4.6

[openshift-cherrypick-robot]

@vrothberg: new pull request created: #19205

In response to this:

/cherry-pick v4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.