Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorporate public charm listing suggestions #25

Merged
merged 11 commits into from
Nov 30, 2023
19 changes: 9 additions & 10 deletions .github/workflows/integration_test.yaml
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
name: Integration tests

on:
pull_request:

workflow_call:
jobs:
integration-test-microk8s:
name: Integration tests (microk8s)
strategy:
fail-fast: false
matrix:
tox-environments:
- integration-charm
- integration-policy
runs-on: ubuntu-latest
steps:
- name: Checkout
Expand All @@ -15,13 +20,7 @@ jobs:
with:
juju-channel: 3.1/stable
provider: microk8s
microk8s-addons: "ingress storage dns rbac registry"
channel: 1.25-strict/stable
- name: Run integration tests
# set a predictable model name so it can be consumed by charm-logdump-action
run: tox -e integration -- --model testing
- name: Dump logs
uses: canonical/charm-logdump-action@main
if: failure()
with:
app: trino-k8s
model: testing
run: tox -e ${{ matrix.tox-environments }}
2 changes: 1 addition & 1 deletion .github/workflows/test_and_publish_charm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ jobs:
integration-test-provider: microk8s
integration-test-provider-channel: 1.25-strict/stable
integration-test-juju-channel: 3.1/stable
integration-test-modules: '["test_charm"]'
integration-test-modules: '["test_charm", "test_policy"]'
19 changes: 11 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,22 @@ This charm is used to deploy Trino Server in a k8s cluster. For local deployment
### Install Microk8s
```
# Install Microk8s from snap:
sudo snap install microk8s --classic --channel=1.25
sudo snap install microk8s --channel 1.25-strict/stable

# Add the 'ubuntu' user to the Microk8s group:
sudo usermod -a -G microk8s ubuntu
# Add your user to the Microk8s group:
AmberCharitos marked this conversation as resolved.
Show resolved Hide resolved
sudo usermod -a -G snap_microk8s $USER

# Give the 'ubuntu' user permissions to read the ~/.kube directory:
sudo chown -f -R ubuntu ~/.kube
# Switch to microk8s group
newgrp snap_microk8s

# Create the 'microk8s' group:
newgrp microk8s
# Create the ~/.kube/ directory and load microk8s configuration
mkdir -p ~/.kube/ && microk8s config > ~/.kube/config

# Enable the necessary Microk8s addons:
microk8s enable hostpath-storage dns
sudo microk8s enable hostpath-storage dns

# Set up a short alias for Kubernetes CLI:
sudo snap alias microk8s.kubectl kubectl
```
### Install Charmcraft
```
Expand Down
6 changes: 0 additions & 6 deletions config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@ options:
Valid values: info, debug, warn, error.
default: info
type: string
application-name:
description: |
The name of the application service to be used as internal host
when providing a jdbc string to Ranger for the policy relation.
default: trino-k8s
type: string
int-comms-secret:
description: |
The secret Trino uses to communicate between nodes.
Expand Down
5 changes: 3 additions & 2 deletions lib/charms/nginx_ingress_integrator/v0/nginx_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 3
LIBPATCH = 4

__all__ = ["require_nginx_route", "provide_nginx_route"]

Expand Down Expand Up @@ -172,7 +172,8 @@ def _config_reconciliation(self, _event: typing.Any = None) -> None:

# C901 is ignored since the method has too many ifs but wouldn't be
# necessarily good to reduce to smaller methods.
def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches # noqa: C901
# E501: line too long
def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,too-many-arguments # noqa: C901,E501
*,
charm: ops.charm.CharmBase,
service_hostname: str,
Expand Down
46 changes: 15 additions & 31 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class TrinoK8SCharm(CharmBase):
"""Charm the service.

Attrs:
_state: used to store data that is persisted across invocations.
state: used to store data that is persisted across invocations.
external_hostname: DNS listing used for external connections.
"""

Expand All @@ -67,7 +67,7 @@ def __init__(self, *args):
"""
super().__init__(*args)
self.name = "trino"
self._state = State(self.app, lambda: self.model.get_relation("peer"))
self.state = State(self.app, lambda: self.model.get_relation("peer"))
self.connector = TrinoConnector(self)
self.policy = PolicyRelationHandler(self)

Expand Down Expand Up @@ -103,7 +103,7 @@ def _on_install(self, event):
Args:
event: The event triggered when the relation changed.
"""
self.unit.status = MaintenanceStatus("installing trino")
self.unit.status = MaintenanceStatus(f"{self.name} unit provisioned.")

@log_event_handler(logger)
def _on_pebble_ready(self, event: PebbleReadyEvent):
Expand Down Expand Up @@ -131,7 +131,8 @@ def _on_peer_relation_changed(self, event):
Args:
event: The event triggered when the peer relation changed
"""
if not self.ready_to_start():
if not self.state.is_ready():
self.unit.status = WaitingStatus("Waiting for peer relation.")
event.defer()
return

Expand All @@ -148,7 +149,7 @@ def _on_peer_relation_changed(self, event):
self.unit.status = BlockedStatus(str(err))
return

target_connectors = self._state.connectors or {}
target_connectors = self.state.connectors or {}
if target_connectors == current_connectors:
return

Expand All @@ -162,17 +163,11 @@ def _get_current_connectors(self, container):

Returns:
properties: A dictionary of existing connector configurations

Raises:
RuntimeError: Failed to return property files
"""
properties = {}
out, err = container.exec(["ls", CATALOG_PATH]).wait_output()
if err:
raise RuntimeError(f"Could not return files: {err}")

files = out.strip().split("\n")
property_names = [file_name.split(".")[0] for file_name in files]
files = container.list_files(CATALOG_PATH, pattern="*.properties")
file_names = [f.name for f in files]
property_names = [file_name.split(".")[0] for file_name in file_names]

for item in property_names:
path = f"{CATALOG_PATH}/{item}.properties"
Expand Down Expand Up @@ -211,18 +206,6 @@ def _restart_trino(self, container):
container.restart(self.name)
self.unit.status = ActiveStatus()

def ready_to_start(self):
"""Check if peer relation established.

Returns:
True if peer relation established, else False.
"""
if not self._state.is_ready():
self.unit.status = WaitingStatus("Waiting for peer relation.")
return False

return True

@log_event_handler(logger)
def _on_restart(self, event):
"""Restart Trino, action handler.
Expand Down Expand Up @@ -270,9 +253,9 @@ def _enable_password_auth(self, container):

def _create_truststore_password(self):
"""Create truststore password if it does not exist."""
if not self._state.truststore_password:
if not self.state.truststore_password:
truststore_password = generate_password()
self._state.truststore_password = truststore_password
self.state.truststore_password = truststore_password

def _open_service_port(self):
"""Open port 8080 on Trino coordinator."""
Expand Down Expand Up @@ -315,11 +298,11 @@ def _create_environment(self):
"OAUTH_CLIENT_ID": self.config.get("google-client-id"),
"OAUTH_CLIENT_SECRET": self.config.get("google-client-secret"),
"WEB_PROXY": self.config.get("web-proxy"),
"SSL_PWD": self._state.truststore_password,
"SSL_PWD": self.state.truststore_password,
"SSL_PATH": f"{CONF_PATH}/truststore.jks",
"CHARM_FUNCTION": self.config["charm-function"],
"DISCOVERY_URI": self.config["discovery-uri"],
"APPLICATION_NAME": self.config["application-name"],
"APPLICATION_NAME": self.app.name,
}
return env

Expand All @@ -334,7 +317,8 @@ def _update(self, event):
event.defer()
return

if not self.ready_to_start():
if not self.state.is_ready():
self.unit.status = WaitingStatus("Waiting for peer relation.")
event.defer()
return

Expand Down
20 changes: 10 additions & 10 deletions src/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ def _add_cert_to_truststore(self, container, conn_name):
"-keystore",
"truststore.jks",
"-storepass",
self.charm._state.truststore_password,
self.charm.state.truststore_password,
"-noprompt",
]
try:
container.exec(command, working_dir=CONF_PATH).wait_output()
container.exec(command, working_dir=CONF_PATH).wait()
except ExecError as e:
expected_error_string = f"alias <{conn_name}> already exists"
if expected_error_string in str(e.stdout):
Expand Down Expand Up @@ -153,12 +153,12 @@ def _add_connector_to_state(self, config, conn_name):
config: The configuration of the connector
conn_name: The name of the connector
"""
if self.charm._state.connectors:
connectors = self.charm._state.connectors
if self.charm.state.connectors:
connectors = self.charm.state.connectors
else:
connectors = {}
connectors[conn_name] = config
self.charm._state.connectors = connectors
self.charm.state.connectors = connectors

def _delete_cert_from_truststore(self, container, conn_name):
"""Delete CA from JKS truststore.
Expand All @@ -179,11 +179,11 @@ def _delete_cert_from_truststore(self, container, conn_name):
"-keystore",
"truststore.jks",
"-storepass",
self.charm._state.truststore_password,
self.charm.state.truststore_password,
"-noprompt",
]
try:
container.exec(command, working_dir=CONF_PATH).wait_output()
container.exec(command, working_dir=CONF_PATH).wait()
except ExecError as e:
expected_error_string = f"Alias <{conn_name}> does not exist"
if expected_error_string in str(e.stdout):
Expand Down Expand Up @@ -213,7 +213,7 @@ def _on_remove_connector(self, event: ActionEvent):
)
return

if not self.charm._state.connectors[conn_name]:
if not self.charm.state.connectors[conn_name]:
event.fail(
f"Failed to remove {conn_name}, connector does not exist"
)
Expand All @@ -230,9 +230,9 @@ def _on_remove_connector(self, event: ActionEvent):

container.remove_path(path=path)

existing_connectors = self.charm._state.connectors
existing_connectors = self.charm.state.connectors
del existing_connectors[conn_name]
self.charm._state.connectors = existing_connectors
self.charm.state.connectors = existing_connectors

self.charm._restart_trino(container)
event.set_results({"result": "connector successfully removed"})
Loading
Loading