canonical / data-integrator

Juju charm to provide DB access for apps outside of Juju.
https://charmhub.io/data-integrator
Apache License 2.0
3 stars 4 forks source link

Add integration tests with pgbouncer(-k8s) <> data-integrator #15

Closed taurus-forever closed 1 year ago

taurus-forever commented 1 year ago

New tests are based on PostgreSQL tests. Also expand test app to use port from relation and not default one.

Plus fixed warning for deprecated wait_for_active:

...site-packages/juju/model.py:2444: DeprecationWarning: wait_for_active is deprecated; use status warnings.warn("wait_for_active is deprecated; use status", DeprecationWarning)

taurus-forever commented 1 year ago

@welpaolo @dragomirp We have a conceptual test design issue for subordinate pgbouncer VM test. See https://github.com/canonical/data-integrator/actions/runs/4389869316/jobs/7687863056

... psycopg2.OperationalError: connection to server at "localhost" (127.0.0.1), port 6432 failed: Connection refused Is the server running on that host and accepting TCP/IP connections?

PgBouncer returns localhost as a connection address (as it is designed to run inside principal app charm). While in the current design test 'app' is not related with pgbouncer at all. PgBouncer relates with data-integrator only and test app 'steals' connection credentials from data-integrator relation manually (simulating end-users behavior of data-integrator).

Q: how pgbouncer VM should be operated to provide access to 3rd party non-juju applications via data-integrator? Should pgbouncer switch from localhost to pgbouncer unit/app address if related with data-integrator? TLS...?

dragomirp commented 1 year ago

Q: how pgbouncer VM should be operated to provide access to 3rd party non-juju applications via data-integrator? Should pgbouncer switch from localhost to pgbouncer unit/app address if related with data-integrator? TLS...?

Serving on the unit IP in general or just for the data-integrator should be feasible. About TLS the question would be what expectation is set for other relations if we only intend to use it with data-integrator.

taurus-forever commented 1 year ago

@dragomirp thank you for pushing this forward. pgb-k8s test is now happy. Can you please consider to mute pgb-vm test here (to submit this). I believe we will have more time to sync on the proper behavior for PGB-VM case "app is outside of Juju". No reason to keep the PR waiting.

We will enable PGB-VM test when subordinate PGB-VM allows external IP connections. What do you think?

P.S. to make integration-k8s-mysql happy, switch it from latest/edge to 8.0/edge tnx!

dragomirp commented 1 year ago

Skipped the VM PGB test, switched channel for MySQL and merged main.

taurus-forever commented 1 year ago

Thank you TEAM! Finally merged! It was a bumpy ride!