fix problem with disabling tls #2295
Conversation
Result of fdb-kubernetes-operator-pr on Linux RHEL 9
|
| var ipAddress fdbv1beta2.ProcessAddress | ||
| for _, addr := range addresses { | ||
| if addr.Flags["tls"] == cluster.Spec.MainContainer.EnableTLS { | ||
| if mixedTls || (addr.Flags["tls"] == cluster.Spec.MainContainer.EnableTLS) { |
There was a problem hiding this comment.
I'm not totally sure in which cases that should help. In cased of "mixed TLS" the processes should have the right listen address anyways (either tls or non-tls, depending on the migration path). Would be nice if you could add some additional information why this change is needed. What are the risks of this change?
There was a problem hiding this comment.
Sorry still very much a WIP, I want to do a few more tests to be sure this solves it on some edge cases. I haven't looked much at the risks yet (I did figure the generation condition would protect against most of them) - wanted to make sure it works first haha.
When I was doing some tests disabling TLS, the listen address was still using TLS when the maincontainer.enableTls was set to false, and this TLS-match-condition caused the coordinator addresses to be seen as invalid, and the operator wasn't able to select coordinators. It might be a different or FDB bug that the addresses were still on TLS though
Description
This lifts some constraints on coordinator selection, hopefully fixes #871
(ran subset of e2e tests which reconciled, will see if the rest do)
Type of change
Please select one of the options below.
Discussion
Are there any design details that you would like to discuss further?
Testing
Please describe the tests that you ran to verify your changes. Unit tests?
Manual testing?
Do we need to perform additional testing once this is merged, or perform in a larger testing environment?
Documentation
Did you update relevant documentation within this repository?
If this change is adding new functionality, do we need to describe it in our user manual?
If this change is adding or removing subreconcilers, have we updated the core technical design doc to reflect that?
If this change is adding new safety checks or new potential failure modes, have we documented and how to debug potential issues?
Follow-up
Are there any follow-up issues that we should pursue in the future?
Does this introduce new defaults that we should re-evaluate in the future?