https://bz.apache.org/bugzilla/show_bug.cgi?id=66660
--- Comment #12 from Diego Rivera <diego.riv...@armedia.com> --- (In reply to Remy Maucherat from comment #10) > (In reply to Diego Rivera from comment #9) > > I already covered the downside of attempting to use the K8s API: the member > > list is compiled up front during membership startup as well, and is never > > updated afterwards at runtime, so I'd end up with an incomplete/inaccurate > > member list anyway (because, remember, the member pods may not be up or even > > exist yet). > > Both the DNS and K8 API membership providers retrieve the list of currently > active pods and work with that, so it is dynamic. The DNS one is a bit less > reliable, but it usually works ... Well, the DNSMembershipProvider implementation depends on the service's DNS entry returning all member pod IPs on query, which in some cases it does not since in K8s each Service gets its own IP, which is then routed over to the pod's IPs. This is by design to avoid DNS caches routing requests to pods which have disappeared when you really only should be accessing them via the Service. Thus, it wouldn't solve our predicament b/c the cluster would be made up of a single member: the service's IP address. The KubernetesMembershipProvider requires granting access to the K8s API, which may be out of the question in some deployments due to security concerns. The ideal solution to this scenario would be oblivious to whether it's being deployed in K8s, Docker Swarm, or any other orchestration/clustering infrastructure. After reading the code a little more calmly, these are the changes I would suggest which would solve the issue while having near-0 impact on the existing code's functionality (all in MemberImpl.java): * Delay the call to getByName() until getHost() is called * Remove setHost() altogether (not really needed anymore) * Change the MemberImpl.host field to transient * Change the MemberImpl.hostname field to non-transient * Update the static MemberImpl.getMember() method to use setHostname() instead of setHost(), since the host's IP would no longer be serialized (nor would we want it to, b/c we want to resolve the hostname each time it's required). >From where I'm sitting, the impact of the above changes would be that the serialized Member instances would be slightly larger since now they'd transport the hostname string vs. the actual IP address's byte[], and the slight DNS lookup impact from having to resolve the hostname whenever getHost() is called. If this impact is deemed unacceptable, I'm sure the code could be made smart enough to cache the value until the member is marked as failed (not sure how that would be done from MemberImpl's perspective, but I'm sure something can be worked out). Then again, I'd like an expert's opinion on whether the above changes would have any unforeseen impact beyond what I've described. If they don't, I'll be happy to spend some time on this and submit a patch proposal. Finally, this new functionality could be turned on/off with a flag (i.e. delayedLookup=true?) if there's enough aversion to the change becoming the default (which I can definitely understand). -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
