daemon: fix race in config handler

[h3llix]

Fixes: #17321

[joamaki]

Thanks for the contribution! One minor comment.

How did you figure out that "ConfigPatchMutex" shouldn't be serialized? Wondering if there's other fields that should be skipped and whether there's a better way than manually skipping specific fields...

[joestringer]

How did you figure out that "ConfigPatchMutex" shouldn't be serialized? Wondering if there's other fields that should be skipped and whether there's a better way than manually skipping specific fields...

I had prompted this, because "ConfigPatchMutex" isn't a configuration option, it's a mutex used internally inside the daemon to synchronize state. So it doesn't make sense to me that we would try to serialize a synchronization primitive onto the wire. Interesting question though, for instance when I peruse the DaemonConfig structure I see a couple of other fields that aren't actually agent configuration options, they're just conveniently placed directly into this structure. I'm looking at things like DryMode which is only used for unit tests. Maybe CreationTime falls under this category also. It probably makes sense for us to collect these non-option fields together in a dedicated section maybe at the start of the DaemonConfig structure (or even in a dedicated structure that's embedded directly into DaemonConfig) so that it's easier to identify these and that they should not be serialized.

[h3llix]

@joestringer That makes sense. We could have embedded structs containing configurations which do not need to be serialized. Simply skipping the struct at the time of mapping.

[joestringer]

Fix LGTM, thanks.

[joestringer]

test-me-please

Job 'Cilium-PR-K8s-1.16-net-next' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sServicesTest Checks service across nodes Tests NodePort BPF Tests with direct routing Tests NodePort

Failure Output

FAIL: Can not connect to service "tftp://192.168.36.11:31670/hello" from outside cluster (1/10)

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.16-net-next so I can create a new GitHub issue to track it.

[joamaki]

LGTM now, thanks!

[joestringer]

ci-aks failure is likely a variation of the flake #16938.
test-1.16-netnext failure looks like #12511.
ci-l4lb failure appears to be #17305.

No need to block this PR on the test failures since they are unrelated to the PR.