Fix specifying multiple interfaces for egress masquerade with enable-masquerade-to-route-source=false #36103
Conversation
bae3304 to
cc378d4
Compare
cc378d4 to
407ac01
Compare
`EgressMasqueradeInterfaces` config property removed as unused. Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
407ac01 to
24cc392
Compare
|
/test |
MasqueradeInterfaces param passed into Cilium as quoted string and Viper GetStringSlice method always return it as one element slice. The commit changes MasqueradeInterfaces Viper param type and parsing logic. Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
When `NodeIpsetNeeded` param enabled, iptables rules might include MasqueradeInterfaces (if configured) and pass them as output interface (`-o`) param for `iptables` command. Iptables output interface param accepts single interface instead of list. The commit fixes iptables commands construction logic (to consider multiple egress masquerade interfaces) and extracts the logic in a separate function. Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
When `EnableMasqueradeRouteSource` param disabled, iptables rules might include MasqueradeInterfaces (if configured) and pass them as output interface (`-o`) param for `iptables` command. Iptables output interface param accepts single interface instead of list. The commit fixes iptables commands construction logic (to consider multiple egress masquerade interfaces) and extracts the logic in a separate function. Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
24cc392 to
9c52c85
Compare
|
/test |
|
@viktor-kurchenko , what is the difference between the previous ruleset and the current ones? IIUC, an example of a previous post rule with multiple interfaces is:
while now, it seems like we get 2 post rules with a single interface each one of them: Is the above correct? And same for the AllEgressMasqueradeCmds, having a single rule with comma-separated values or having multiple rules with a single interface value per rule. If it is the case, what is the difference betwen both approaches? |
Yes, you are right. I've successfully tested the fix with EKS AL2023. |
|
LGTM - thank you for the clarification and for the fix! |
|
@viktor-kurchenko One more small follow-up - would you also be able to review the wording in |
Hey @joestringer, I had a discussion with @auriaave about this quote. |
I think that's a reasonable start point yeah. If we think it should work, then we shouldn't tell people it doesn't work. I understand if we haven't yet provisioned a mixed node cluster to try it out. We could also maybe publish the intended new configuration options in the Cilium docs there, then community members can try it out and report whether the options worked well or not. |
So, what if we update only OSS doc for now? |
The commit removes EKS cluster mixed node support restriction since egress masquerade supports multiple interfaces (see fix: #36103). Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
The commit removes EKS cluster mixed node support restriction since egress masquerade supports multiple interfaces (see fix: #36103). Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
The commit removes EKS cluster mixed node support restriction since egress masquerade supports multiple interfaces (see fix: cilium#36103). Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
This PR introduces fix for
egressMasqueradeInterfacesmultiple interfaces support.Multi-interface support can be helpful when cluster contains nodes with different operation systems and network interfaces might have different names
E.g.: EKS cluster upgrade to Amazon Linux 2023 that uses
ensinstead ofeth.Usage example via Cilium CLI:
Please see per commit for full details.