[DPE-5830] Bump to 24.04 #495
Conversation
zmraul
force-pushed
the
bump_to_24_04
branch
from
4894dd5 to
1445650
Compare
| @@ -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" | |||
There was a problem hiding this comment.
I like that change and that this is now provided by the snap!
| @@ -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 | |||
There was a problem hiding this comment.
Maybe add some # todo comments here?
There was a problem hiding this comment.
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 tempfile.NamedTemporaryFile( | ||
| mode="w+t", dir=self.charm.opensearch.paths.conf | ||
| ) as ca_tmp_file: |
There was a problem hiding this comment.
👍 good change. Just for consideration: there is also a self.charm.opensearch.paths.certs.
There was a problem hiding this comment.
Yes, I was between that one or conf. I think either way is fine since the files disappear once close() is called.
| # should not be updated unless https://github.com/juju/python-libjuju/issues/1093 is fixed | ||
| juju = "==3.5.0" | ||
| # websockets issue solved from 3.2.5.1 https://github.com/juju/python-libjuju/issues/1184 # TODO: restore to 3.5.0 |
There was a problem hiding this comment.
Nice to see this fixed.
There was a problem hiding this comment.
I think, in terms of changes, it seems okay. Covers the distro module and fixes the calls to our plugins or keytool. What I expect to see before given a final +1:
- CI passes on all tests -> only exceptions I would consider here are
test_storage, which is a bit flaky; and lib check - A ticket explicitly highlighting we need to go back and revisit the TODOs of this PR. Assign this ticket to yourself for this pulse. Keep it in "blocked" and list what we need to have so we can unblock
|
|
||
| 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) |
There was a problem hiding this comment.
nice, makes a lot of sense to run these scripts under the snap context. TBH we only use this logic here.
There was a problem hiding this comment.
After discussions with @zmraul, there is an ongoing issue with charmcraft and the test application charm: canonical/charmcraft#2055
Both moving the CI to 24.04 and updating opensearch CI once the charmcraft issue above has been fixed have been reported in the following respective tickets:
We can move forward with this PR, aware the relations/test_opensearch_provider.py will not be passing for the time being.
Update CI pipeline to enable 24.04 support.
Change list:
charmcraft.yamluses charmcraftst124 syntax to enable building on multiple basesopensearch.<bin-command>tmpfilesare now created inside the snap folders so the snap commands are able to access themjujuversion that fixes storage and websockets issuesTODO:
After this PR, some tasks will need addressing: