@dylandreimerink Please take a look and let me know whether you're OK with these incremental changes. I don't think they're entirely taking the loader to the right direction, so if you have a better idea what steps we should take let me know and I'll refactor this.

/test

/test

A config flag starting with a verb (Derive in this case) typically conveys that this is a boolean, though. (e.g. derive the masq IP or not?) Also e2manywords; let's call this what it is: MasqueradeDevice. The fact that it's (currently) used to derive the masq IP can go in a comment on the field, though if it's extended later, that will go stale.

I would like to keep it as it was originally (as bad as it was) and not start changing it now. MasqueradeDevice is also confusing as this is about overriding the BPF masquerade IP address by forcing it to be derived from the given device (e.g. not about what device to "use", but how to find the IP). I actually want to keep the option badly named and obscure as it's a hidden option and only used in a very specific deployment by one user of Cilium to work around an odd setup and we want to get rid of it as soon as we can.

Unless we can tackle #17158 in the very short term (1.16) and it's actually planned, let's not assume that this is temporary and just document this as a feature/requirement of the loader.

Good point, I'll adjust the comments.
Will check with @brb on how feasible it is to actually get rid of this in v1.16.

One more thing, this string can be empty. It's not clear what the caller should expect when it's left empty. If it's optional, it needs to be specified why.

Will add comments to explain that default is empty which means the address is taken from the actual device and not overridden by this.

Adding reconciliation later is perfectly fine, of course. Though I'd prefer if the query happens as early as possible so we can get rid of the stringly input param asap and pass around link objects instead. Not sure if there's some kind of startup ordering here, I assume this device exists before the agent starts?

Yes for this particular hidden option it is assumed that the device exists before the agent starts.
And agreed on querying much earlier. This will follow, but I didn't yet want to start making guesses on how we want to structure it.

We could also just forget this PR and incorporate it into a follow-up that @dylandreimerink is working on that adds the loader reconciliation on device changes?

One more thing, this string can be empty. It's not clear what the caller should expect when it's left empty. If it's optional, it needs to be specified why.

A Config struct is populated via Hive from command-line flags and config files and the defaults for it are specified when defining the flag. Only in tests is it manually populated (well we do still have the exception in cilium-dbg but that'll go away). I added comments on the defaults and the optionality.

/test

/test

/test

CI triage (WIP)

• Clustermesh Upgrade failed waiting for the clustermesh to become ready. Could be a real issue or a flake, investigating. https://github.com/cilium/cilium/actions/runs/8231576562/job/22507953395#step:24:3642 clustermesh-apiserver 1 pods of Deployment clustermesh-apiserver are not ready
• Clustermesh conformance:
  • (6, vxlan, ipv4, disabled, none, clustermesh, cluster, cluster... also failed waiting for the mesh, this time both hubble relay and clustermesh apiserver:

Errors:                hubble-relay             hubble-relay             1 pods of Deployment hubble-relay are not ready
                       clustermesh-apiserver    clustermesh-apiserver    1 pods of Deployment clustermesh-apiserver are not ready

• Installation and Connectivity Test (2, disabled, dual, wireguard, none, clustermesh, migration, m... failed all the external tests. Could be a real issue or a flake, investigating.
• ci-eks failed with the following in https://github.com/cilium/cilium/actions/runs/8231576176/job/22507185058#step:24:236. Not a known flake afaik.

Waiting until key rotation starts (seeing 1 keys)
Waiting until key rotation starts (seeing 1 keys)
Waiting until key rotation completes (seeing 2 keys)
Waiting until key rotation completes (seeing 2 keys)
Error from server: error dialing backend: dial tcp 192.168.183.34:10250: i/o timeout

• ci ginkgo has many failed tests, typically FAIL: Kubernetes DNS did not become ready in time - failing to get the cluster into a healthy state initially
• ci-e2e also has lots of failed tests, lots of timeouts in connectivity tests; after which at some point seemingly all the sysdumps overwhelm the k8s api server.

Given the amount of redness in these tests I don't think these are just flakes.

/test