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

[DPE-5830] Bump to 24.04 #495

Merged
merged 36 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
83ed30c
add Ubuntu 24.04 to charmcraft.yaml
reneradoi Oct 28, 2024
772f0a5
remove build-on and run-on parameters from charmcraft.yaml
reneradoi Oct 28, 2024
5d27abd
fix charmcraft.yaml syntax for using ubuntu 24.04 with charmcraft 3
reneradoi Oct 28, 2024
53897c2
install 24.04 snap
reneradoi Oct 30, 2024
b8cb252
switch to snap usage and apply charmcraft patch
zmraul Nov 25, 2024
0668479
add 24 base patch
zmraul Nov 25, 2024
337b880
switch to correct snap
zmraul Nov 25, 2024
64cb585
lint
zmraul Nov 25, 2024
3f3c999
update with keytool app from snap
zmraul Nov 26, 2024
cf24129
use snap folder for tmp dirs
zmraul Nov 26, 2024
3a3c019
test charmcraft 3
zmraul Nov 26, 2024
25aad14
test collect bases fix
zmraul Nov 27, 2024
1dcaa19
fix unit tests
zmraul Nov 27, 2024
d121e8d
mend
zmraul Nov 27, 2024
f4aecff
fix unit test
zmraul Nov 27, 2024
68fb074
update cache
zmraul Dec 3, 2024
eec8311
add cloned PR process to 24.04 base
zmraul Dec 4, 2024
2633109
force patch
zmraul Dec 4, 2024
4a6aa94
switch command
zmraul Dec 4, 2024
28aaaaf
authenticate
zmraul Dec 4, 2024
cfce9ca
add automated PR creation
zmraul Dec 4, 2024
272761e
switch base branch
zmraul Dec 4, 2024
4a4c8dc
switch base branch
zmraul Dec 4, 2024
cacae9c
integrate with charmcraftst124
zmraul Dec 5, 2024
1065ff7
fix application charm app
zmraul Dec 5, 2024
7b1ac8f
fix application charm app
zmraul Dec 5, 2024
754b6e4
fix lint
zmraul Dec 5, 2024
e07cd94
test websockets issue
zmraul Dec 6, 2024
2b96dd6
test
zmraul Dec 6, 2024
5f47f14
test
zmraul Dec 6, 2024
494cbfa
run scripts internally
zmraul Dec 9, 2024
4349a7b
update juju
zmraul Dec 9, 2024
b616601
move temporary files to conf folde
zmraul Dec 11, 2024
e75277b
fix unit tests
zmraul Dec 11, 2024
1445650
rebase
zmraul Dec 11, 2024
9821ba6
wait for idle before adding client relation
zmraul Dec 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ on:
jobs:
lint:
name: Lint
uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v22.0.0
uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v23.1.1

unit-test:
name: Unit test charm
Expand Down Expand Up @@ -114,10 +114,11 @@ jobs:
path:
- .
- ./tests/integration/relations/opensearch_provider/application-charm/
uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v22.0.0
uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@beta-charmcraftst124
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some # todo comments here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The big "TODO" is on charmcraft.yaml explaining why all of this is needed. Once we generally update how charmcraft is done, all of this other places will be forced to change by necessity.

with:
path-to-charm-directory: ${{ matrix.path }}
cache: true
cache: false
# charmcraft-snap-revision: 5303

integration-test:
strategy:
Expand All @@ -135,12 +136,12 @@ jobs:
- lint
- unit-test
- build
uses: canonical/data-platform-workflows/.github/workflows/integration_test_charm.yaml@v22.0.0
uses: canonical/data-platform-workflows/.github/workflows/integration_test_charm.yaml@v23.1.1
with:
juju-agent-version: ${{ matrix.juju.agent }}
juju-snap-channel: ${{ matrix.juju.snap_channel }}
_beta_allure_report: ${{ matrix.juju.allure_report }}
artifact-prefix: packed-charm-cache-true
artifact-prefix: packed-charm-cache-false
cloud: lxd
secrets:
# GitHub appears to redact each line of a multi-line secret
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ jobs:

build:
name: Build charm
uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v22.0.0
uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@beta-charmcraftst124

release:
name: Release charm
needs:
- build
uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v22.0.0
uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@beta-charmcraftst124
with:
channel: 2/edge
artifact-prefix: ${{ needs.build.outputs.artifact-prefix }}
Expand Down
9 changes: 9 additions & 0 deletions charmcraft-24.04.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
@@ -2,7 +2,7 @@
# See LICENSE file for licensing details.

type: charm
-base: [email protected]
+base: [email protected]
platforms:
amd64:
parts:
19 changes: 12 additions & 7 deletions charmcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,18 @@
# See LICENSE file for licensing details.

type: charm
bases:
- build-on:
- name: "ubuntu"
channel: "22.04"
run-on:
- name: "ubuntu"
channel: "22.04"
# base: [email protected]
# platforms:
# amd64:
# Use upcoming ST124 syntax
# To pack this charm, a temporary compatibility wrapper https://github.com/canonical/charmcraftst124
# is required until ST124 support is added to charmcraft
# (ST124 syntax is needed to enable multi-base charms with Ubuntu 24.04. We use ST124 syntax across
# all of our charms [even those that aren't multi base] for consistency and to simplify CI/CD
# maintenance & tooling)
platforms:
[email protected]:amd64:
[email protected]:amd64:
parts:
files:
plugin: dump
Expand Down
2 changes: 1 addition & 1 deletion lib/charms/opensearch/v0/constants_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@


# Opensearch Snap revision
OPENSEARCH_SNAP_REVISION = 58 # Keep in sync with `workload_version` file
OPENSEARCH_SNAP_REVISION = 62 # Keep in sync with `workload_version` file

# User-face Backup ID format
OPENSEARCH_BACKUP_ID_FORMAT = "%Y-%m-%dT%H:%M:%SZ"
Expand Down
8 changes: 4 additions & 4 deletions lib/charms/opensearch/v0/opensearch_distro.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,23 +197,23 @@ def is_node_up(self, host: Optional[str] = None) -> bool:
return False

def run_bin(self, bin_script_name: str, args: str = None, stdin: str = None) -> str:
"""Run opensearch provided bin command, relative to OPENSEARCH_BIN.
"""Run opensearch provided bin command, through the snap.

Args:
bin_script_name: opensearch script located in OPENSEARCH_BIN to be executed
args: arguments passed to the script
stdin: string input to be passed on the standard input of the subprocess.
"""
script_path = f"{self.paths.bin}/{bin_script_name}"
return self._run_cmd(script_path, args, stdin=stdin)
opensearch_command = f"opensearch.{bin_script_name}"
return self._run_cmd(opensearch_command, args, stdin=stdin)

def run_script(self, script_name: str, args: str = None):
"""Run script provided by Opensearch in another directory, relative to OPENSEARCH_HOME."""
script_path = f"{self.paths.home}/{script_name}"
if not os.access(script_path, os.X_OK):
self._run_cmd(f"chmod a+x {script_path}")

self._run_cmd(f"{script_path}", args)
self._run_cmd(f"snap run --shell opensearch.daemon -- {script_path}", args)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, makes a lot of sense to run these scripts under the snap context. TBH we only use this logic here.


def request( # noqa
self,
Expand Down
4 changes: 2 additions & 2 deletions lib/charms/opensearch/v0/opensearch_keystore.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def __init__(self, charm):
"""Creates the keystore manager class."""
self._charm = charm
self._opensearch = charm.opensearch
self._keytool = charm.opensearch.paths.jdk + "/bin/keytool"
self._keytool = "opensearch.keytool"
self._keystore = ""
self._password = None

Expand Down Expand Up @@ -132,7 +132,7 @@ class OpenSearchKeystore(Keystore):
def __init__(self, charm):
"""Creates the keystore manager class."""
super().__init__(charm)
self._keytool = "opensearch-keystore"
self._keytool = "keystore"

def add(self, entries: Dict[str, str]) -> None:
"""Adds a given key to the "opensearch" keystore."""
Expand Down
6 changes: 3 additions & 3 deletions lib/charms/opensearch/v0/opensearch_plugin_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ def _install_plugin(self, plugin: OpenSearchPlugin) -> bool:
if missing_deps:
raise OpenSearchPluginMissingDepsError(plugin.name, missing_deps)

self._opensearch.run_bin("opensearch-plugin", f"install --batch {plugin.name}")
self._opensearch.run_bin("plugin", f"install --batch {plugin.name}")
except KeyError as e:
raise OpenSearchPluginMissingConfigError(e)
except OpenSearchCmdError as e:
Expand Down Expand Up @@ -418,7 +418,7 @@ def _remove_if_needed(self, plugin: OpenSearchPlugin) -> bool:
def _remove_plugin(self, plugin: OpenSearchPlugin) -> bool:
"""Remove a plugin without restarting the node."""
try:
self._opensearch.run_bin("opensearch-plugin", f"remove {plugin.name}")
self._opensearch.run_bin("plugin", f"remove {plugin.name}")
except OpenSearchCmdError as e:
if "not found" in str(e):
logger.info(f"Plugin {plugin.name} to be deleted, not found. Continuing...")
Expand All @@ -429,6 +429,6 @@ def _remove_plugin(self, plugin: OpenSearchPlugin) -> bool:
def _installed_plugins(self) -> List[str]:
"""List plugins."""
try:
return self._opensearch.run_bin("opensearch-plugin", "list").split("\n")
return self._opensearch.run_bin("plugin", "list").split("\n")
except OpenSearchCmdError as e:
raise OpenSearchPluginError("Failed to list plugins: " + str(e))
31 changes: 17 additions & 14 deletions lib/charms/opensearch/v0/opensearch_tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def __init__(
self.peer_relation = peer_relation
self.jdk_path = jdk_path
self.certs_path = certs_path
self.keytool = self.jdk_path + "/bin/keytool"
self.keytool = "opensearch.keytool"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that change and that this is now provided by the snap!

self.certs = TLSCertificatesRequiresV3(charm, TLS_RELATION, expiry_notification_time=23)

self.framework.observe(
Expand Down Expand Up @@ -513,8 +513,6 @@ def _create_keystore_pwd_if_not_exists(self, scope: Scope, cert_type: CertType,

def store_new_ca(self, secrets: Dict[str, Any]) -> bool: # noqa: C901
"""Add new CA cert to trust store."""
keytool = f"sudo {self.jdk_path}/bin/keytool"

if not (deployment_desc := self.charm.opensearch_peer_cm.deployment_desc()):
return False

Expand All @@ -532,7 +530,7 @@ def store_new_ca(self, secrets: Dict[str, Any]) -> bool: # noqa: C901

try:
run_cmd(
f"""{keytool} -changealias \
f"""{self.keytool} -changealias \
-alias {alias} \
-destalias old-{alias} \
-keystore {store_path} \
Expand All @@ -549,13 +547,15 @@ def store_new_ca(self, secrets: Dict[str, Any]) -> bool: # noqa: C901
):
raise

with tempfile.NamedTemporaryFile(mode="w+t") as ca_tmp_file:
with tempfile.NamedTemporaryFile(
mode="w+t", dir=self.charm.opensearch.paths.conf
) as ca_tmp_file:
Comment on lines +550 to +552
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 good change. Just for consideration: there is also a self.charm.opensearch.paths.certs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was between that one or conf. I think either way is fine since the files disappear once close() is called.

ca_tmp_file.write(secrets.get("ca-cert"))
ca_tmp_file.flush()

try:
run_cmd(
f"""{keytool} -importcert \
f"""{self.keytool} -importcert \
-trustcacerts \
-noprompt \
-alias {alias} \
Expand Down Expand Up @@ -604,7 +604,6 @@ def read_stored_ca(self, alias: str = "ca") -> Optional[str]:

def remove_old_ca(self) -> None:
"""Remove old CA cert from trust store."""
keytool = f"sudo {self.jdk_path}/bin/keytool"
ca_trust_store = f"{self.certs_path}/ca.p12"
old_alias = "old-ca"

Expand All @@ -613,7 +612,7 @@ def remove_old_ca(self) -> None:

try:
run_cmd(
f"""{keytool} \
f"""{self.keytool} \
-list \
-keystore {ca_trust_store} \
-storepass {store_pwd} \
Expand All @@ -628,7 +627,7 @@ def remove_old_ca(self) -> None:
old_ca_content = self.read_stored_ca(alias=old_alias)

run_cmd(
f"""{keytool} \
f"""{self.keytool} \
-delete \
-keystore {ca_trust_store} \
-storepass {store_pwd} \
Expand Down Expand Up @@ -672,12 +671,16 @@ def store_new_tls_resources(self, cert_type: CertType, secrets: Dict[str, Any]):
except OSError:
pass

tmp_key = tempfile.NamedTemporaryFile(mode="w+t", suffix=".pem")
tmp_key = tempfile.NamedTemporaryFile(
mode="w+t", suffix=".pem", dir=self.charm.opensearch.paths.conf
)
tmp_key.write(secrets.get("key"))
tmp_key.flush()
tmp_key.seek(0)

tmp_cert = tempfile.NamedTemporaryFile(mode="w+t", suffix=".cert")
tmp_cert = tempfile.NamedTemporaryFile(
mode="w+t", suffix=".cert", dir=self.charm.opensearch.paths.conf
)
tmp_cert.write(secrets.get("cert"))
tmp_cert.flush()
tmp_cert.seek(0)
Expand Down Expand Up @@ -714,7 +717,7 @@ def all_tls_resources_stored(self, only_unit_resources: bool = False) -> bool:
return False

# to make sure the content is processed correctly by openssl, temporary store it in a file
tmp_ca_file = tempfile.NamedTemporaryFile(mode="w+t")
tmp_ca_file = tempfile.NamedTemporaryFile(mode="w+t", dir=self.charm.opensearch.paths.conf)
tmp_ca_file.write(current_ca)
tmp_ca_file.flush()
tmp_ca_file.seek(0)
Expand Down Expand Up @@ -819,12 +822,12 @@ def reload_tls_certificates(self):
# using the SSL API requires authentication with app-admin cert and key
admin_secret = self.charm.secrets.get_object(Scope.APP, CertType.APP_ADMIN.val)

tmp_cert = tempfile.NamedTemporaryFile(mode="w+t")
tmp_cert = tempfile.NamedTemporaryFile(mode="w+t", dir=self.charm.opensearch.paths.conf)
tmp_cert.write(admin_secret["cert"])
tmp_cert.flush()
tmp_cert.seek(0)

tmp_key = tempfile.NamedTemporaryFile(mode="w+t")
tmp_key = tempfile.NamedTemporaryFile(mode="w+t", dir=self.charm.opensearch.paths.conf)
tmp_key.write(admin_secret["key"])
tmp_key.flush()
tmp_key.seek(0)
Expand Down
Loading
Loading