[DPE 5235] support external clients #32
Conversation
…ed - note charm is flakey
MiaAltieri
changed the title
MiaAltieri
force-pushed
the
DPE-5235-support-external-clients
branch
from
0120a45 to
1fd1850
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 6/edge #32 +/- ##
==========================================
- Coverage 48.57% 45.70% -2.88%
==========================================
Files 4 4
Lines 492 547 +55
==========================================
+ Hits 239 250 +11
- Misses 253 297 +44 ☔ View full report in Codecov by Sentry. |
MiaAltieri
force-pushed
the
DPE-5235-support-external-clients
branch
from
1fd1850 to
7110ca0
Compare
MiaAltieri
force-pushed
the
DPE-5235-support-external-clients
branch
from
7110ca0 to
bf4e3bf
Compare
MiaAltieri
force-pushed
the
DPE-5235-support-external-clients
branch
from
a256dac to
25f2b05
Compare
MiaAltieri
force-pushed
the
DPE-5235-support-external-clients
branch
from
21c6f00 to
53635c0
Compare
src/node_port.py
Outdated
| @@ -154,4 +155,61 @@ def delete_unit_service(self) -> None: | |||
| else: | |||
| raise | |||
|
|
|||
| @property | |||
There was a problem hiding this comment.
functions brought over and modified from MySQL Router, PGBouncer, Kafka. (they were all slightly different so I tried to converge them + make them work for mine)
…+ external clients provided with all ports
|
|
||
|
|
||
| @pytest.mark.group(1) | ||
| @pytest.mark.abort_on_fail | ||
| async def test_build_and_deploy(ops_test: OpsTest) -> None: | ||
| """Build and deploy a sharded cluster.""" | ||
| application_charm = await ops_test.build_charm("tests/integration/application/") |
There was a problem hiding this comment.
We should put this in a fixture like we're doing in mongodb vm
src/charm.py
Outdated
| for unit in self.peers_units: | ||
| hosts = set() | ||
| for unit in self.get_units(): | ||
| unit_ip = self.node_port_manager.get_node_ip(unit.name) |
There was a problem hiding this comment.
We are not checking on unit_ip is None given it is the case on ApiError.status_code == 403, which will effectively add a host of the form None:port
We are also not handling exceptions of get_node_id / get_node_port. Is this intended?
There was a problem hiding this comment.
We are not checking on unit_ip is None given it is the case on ApiError.status_code == 403, which will effectively add a host of the form None:port
Good points Mehdi. This function should certainly be updated. We must make changes on VM side PR here
There was a problem hiding this comment.
I think the linked PR is not the right one on the topic
There was a problem hiding this comment.
It is - the changes you requested require that we raise and error here and then handle it in the provider side?
MiaAltieri
force-pushed
the
DPE-5235-support-external-clients
branch
from
ae54d7d to
194d872
Compare
MiaAltieri
force-pushed
the
DPE-5235-support-external-clients
branch
from
194d872 to
70fb8b3
Compare
MiaAltieri
force-pushed
the
DPE-5235-support-external-clients
branch
from
945c449 to
e109d49
Compare
| Note this only removes users that this application of Charmed MongoDB is responsible for | ||
| managing. It won't remove: | ||
| 1. users created from other applications | ||
| 2. users created from other mongos routers. |
There was a problem hiding this comment.
Is this lib shared between mongod and mongos?
I think we should clean up on mongod and mongos a bit differently.
We know the pattern of routers. We can additionally clean up users created by routers on Mongod when the router relation is gone.
There was a problem hiding this comment.
Thank you for this - that behaviour exists in MongoDB VM charms, but not in K8s since routers handles the users differently I filed an issue here - @Mehdi-Bendriss and I will work to prioritise the issue and get it addressed
Issue
We support toggling of external connectivity, but we do not give clients external access
Solution
Support providing all clients an a URI that is reachable outside of K8s
Future PR
an additional PR should be made for providing ext clients with all available ext ports for their correct hosts.
This is currently not supported since URI handling in
mongo.py/mongos.py/mongodb.pysince they only take a single port as an argument