[DPE-6575] Add microk8s testing #21
Conversation
…ly calculate number of brokers
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| async def microk8s(ops_test: OpsTest) -> SimpleNamespace: |
There was a problem hiding this comment.
praise that's really nice! well done!
There was a problem hiding this comment.
Yeah this is excellent, I'll definitely co-opt it at some point.
src/charm.py
Outdated
| # We may be sitting behind k8s, in this case, we need to do a more careful calculation | ||
| # of the host count, and hence, the replication factor | ||
| if client and client.describe_cluster(): | ||
| replication_factor = len(client.describe_cluster().get("brokers", 1)) - 1 |
There was a problem hiding this comment.
question I would maybe put maximum number of 3 (as long as we have capacity). If we have a deployment of 7 brokers, having replication factor of 7 seems wild. Or am I missing something here?
There was a problem hiding this comment.
actually we do have a limitation, but later in the game. I can add it here instead, it makes more sense.
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| async def microk8s(ops_test: OpsTest) -> SimpleNamespace: |
There was a problem hiding this comment.
Yeah this is excellent, I'll definitely co-opt it at some point.
tests/integration/test_charm.py
Outdated
| model_db = None | ||
|
|
||
|
|
||
| DEPLOY_MARKS = [ | ||
| ( | ||
| pytest.param( | ||
| use_tls, | ||
| cloud, | ||
| id=str(use_tls) + f"-{cloud}", | ||
| marks=pytest.mark.group(str(use_tls) + f"-{cloud}"), | ||
| ) | ||
| ) | ||
| for use_tls in [True, False] | ||
| for cloud in ["vm", MICROK8S_CLOUD_NAME] | ||
| ] | ||
|
|
||
| K8S_MARKS = [ | ||
| ( | ||
| pytest.param( | ||
| use_tls, | ||
| cloud, | ||
| id=str(use_tls) + f"-{cloud}", | ||
| marks=pytest.mark.group(str(use_tls) + f"-{cloud}"), | ||
| ) | ||
| ) | ||
| for use_tls in [True, False] | ||
| } | ||
| USE_TLS = list(TLS_MARK.values()) | ||
| for cloud in [MICROK8S_CLOUD_NAME] | ||
| ] | ||
|
|
||
| VM_MARKS = [ | ||
| ( | ||
| pytest.param( | ||
| use_tls, | ||
| cloud, | ||
| id=str(use_tls) + f"-{cloud}", | ||
| marks=pytest.mark.group(str(use_tls) + f"-{cloud}"), | ||
| ) | ||
| ) | ||
| for use_tls in [True, False] | ||
| for cloud in ["vm"] | ||
| ] |
There was a problem hiding this comment.
todo/nitpick: Please could we move these to conftest so they're not clogging up the test file?
There was a problem hiding this comment.
I am moving it to helpers.py
phvalguima
dismissed stale reviews from marcoppenheimer and deusebio
via
5087773
This PR extends the integration testing to also cover kafka k8s charm. It tests both with and without TLS.
It also fixes an issue between a VM-based benchmark related to a k8s-based broker: we get the incorrect number of brokers as we only see a single endpoint - the nodeport one.
This PR extends the Kafka client lib to then expose the brokers' metadata. With that metadata, we can recover the correct number of brokers in both VM and k8s based deployments instead.