Add COS integration to Openstack #266
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbartz
changed the title
…gration' into ISD-1824-cos-integration-openstack # Conflicts: # src/openstack_cloud/openstack_manager.py # templates/openstack-userdata.sh.j2
cbartz
changed the base branch from
feat/create-openstack-runner
to
feat/openstack-integration
cbartz
changed the title
cbartz
commented
Apr 24, 2024
Comment on lines
+1271
to
+1294
| try: | ||
| self._pull_file( | ||
| ssh_conn=ssh_conn, | ||
| remote_path=str(METRICS_EXCHANGE_PATH / "pre-job-metrics.json"), | ||
| local_path=str(storage.path / "pre-job-metrics.json"), | ||
| max_size=MAX_METRICS_FILE_SIZE, | ||
| ) | ||
| self._pull_file( | ||
| ssh_conn=ssh_conn, | ||
| remote_path=str(METRICS_EXCHANGE_PATH / "post-job-metrics.json"), | ||
| local_path=str(storage.path / "post-job-metrics.json"), | ||
| max_size=MAX_METRICS_FILE_SIZE, | ||
| ) | ||
| return | ||
| except _PullFileError as exc: | ||
| logger.warning( | ||
| "Failed to pull metrics for %s: %s . Will not be able to issue all metrics", | ||
| instance_name, | ||
| exc, | ||
| ) | ||
| return | ||
| except _SSHError as exc: | ||
| logger.info("Failed to pull metrics for %s: %s", instance_name, exc) | ||
| continue |
There was a problem hiding this comment.
The downside here is that if only the pre-job-metrics.json is pulled, the runner is considered crashed (see logic in _issue_reconciliation_metric), we may want to revisit this. But for now I kept it the same as for LXD.
…gration' into ISD-1824-cos-integration-openstack # Conflicts: # src/charm.py # src/openstack_cloud/openstack_manager.py # templates/openstack-userdata.sh.j2
yhaliaw
reviewed
Apr 24, 2024
yanksyoon
approved these changes
Apr 24, 2024
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Applicable spec: ISD-145 (internal)
Overview
Add cos integration to the openstack branch. The Openstack code is going to be refactored (see ISD 134), so there are parts that contain code duplication and should be addressed in a follow-on.
Integration tests are not included in this PR and will be added in a future PR.
The log retrieval code will be adapted in a future PR to retrieve logs periodically (also for LXD) and is not included here.
Rationale
This is necessary for observability.
Juju Events Changes
n/a
Module Changes
metricspackage.shared_fsimplements it. This is done to reuse therunnermetrics module (see also ISD-145).OpenStackRunnerManageris adapted to output runner related metrics.runner_logsis split into general and lxd related codeopenstack-userdata.j2) is adapted to write out metrics and pre-job content. system shutdown is removed to be able to retrieve metrics (otherwise the instance would have to be restarted).Library Changes
Checklist
src-docsurgent,trivial,complex)The Openstack documentation will be added as soon as we have a MVP landed.