diff --git a/charms/tempest-k8s/charmcraft.yaml b/charms/tempest-k8s/charmcraft.yaml index 5b1ead80..40b06c17 100644 --- a/charms/tempest-k8s/charmcraft.yaml +++ b/charms/tempest-k8s/charmcraft.yaml @@ -106,6 +106,14 @@ actions: - found in test list "list1" - AND match regex "one" or "two" - AND don't match regex "three" + + A summary of the results will be printed. + The complete results may be large, + and will be written to /var/lib/tempest/workspace/tempest-validation.log + in the workload container. + This can be copied to your local machine using juju, + for example: + juju scp --container tempest tempest/0:/var/lib/tempest/workspace/tempest-validation.log tempest-validation.log additionalProperties: false params: regex: diff --git a/charms/tempest-k8s/src/charm.py b/charms/tempest-k8s/src/charm.py index 02bd5458..49920e23 100755 --- a/charms/tempest-k8s/src/charm.py +++ b/charms/tempest-k8s/src/charm.py @@ -52,16 +52,19 @@ from ops_sunbeam.config_contexts import ( ) from utils.constants import ( CONTAINER, + TEMPEST_ADHOC_OUTPUT, TEMPEST_CONCURRENCY, TEMPEST_CONF, TEMPEST_HOME, TEMPEST_LIST_DIR, - TEMPEST_OUTPUT, TEMPEST_READY_KEY, TEMPEST_TEST_ACCOUNTS, TEMPEST_WORKSPACE, TEMPEST_WORKSPACE_PATH, ) +from utils.types import ( + TempestEnvVariant, +) from utils.validators import ( validated_schedule, ) @@ -187,7 +190,9 @@ class TempestOperatorCharm(sunbeam_charm.OSBaseOperatorCharmK8S): handlers.append(self.grafana) return handlers - def _get_environment_for_tempest(self) -> Dict[str, str]: + def _get_environment_for_tempest( + self, variant: TempestEnvVariant + ) -> Dict[str, str]: """Return a dictionary of environment variables. To be used with pebble commands that run tempest discover, etc. @@ -209,10 +214,10 @@ class TempestOperatorCharm(sunbeam_charm.OSBaseOperatorCharmK8S): "TEMPEST_CONF": TEMPEST_CONF, "TEMPEST_HOME": TEMPEST_HOME, "TEMPEST_LIST_DIR": TEMPEST_LIST_DIR, - "TEMPEST_OUTPUT": TEMPEST_OUTPUT, "TEMPEST_TEST_ACCOUNTS": TEMPEST_TEST_ACCOUNTS, "TEMPEST_WORKSPACE": TEMPEST_WORKSPACE, "TEMPEST_WORKSPACE_PATH": TEMPEST_WORKSPACE_PATH, + "TEMPEST_OUTPUT": variant.output_path(), } def get_unit_data(self, key: str) -> Optional[str]: @@ -239,7 +244,9 @@ class TempestOperatorCharm(sunbeam_charm.OSBaseOperatorCharmK8S): ) return - env = self._get_environment_for_tempest() + # This is environment sent to the scheduler service, + # for periodic checks. + env = self._get_environment_for_tempest(TempestEnvVariant.PERIODIC) pebble = self.pebble_handler() try: pebble.init_tempest(env) @@ -288,19 +295,31 @@ class TempestOperatorCharm(sunbeam_charm.OSBaseOperatorCharmK8S): exclude_regex: str = event.params["exclude-regex"].strip() test_list: str = event.params["test-list"].strip() - env = self._get_environment_for_tempest() + env = self._get_environment_for_tempest(TempestEnvVariant.ADHOC) try: - output = self.pebble_handler().run_tempest_tests( + summary = self.pebble_handler().run_tempest_tests( regexes, exclude_regex, test_list, serial, env ) except RuntimeError as e: - event.fail(str(e)) - # still print the message, - # because it could be a lot of output from tempest, - # and we want it neatly formatted - print(e) + # put the message in set_results instead of event.fail, + # because event.fail message is not always displayed to the user: + # https://bugs.launchpad.net/juju/+bug/2052765 + event.set_results({"error": str(e)}) + event.fail() return - print(output) + event.set_results( + { + "summary": summary, + "info": ( + "For detailed results, copy the log file from the container by running:\n" + + self.get_copy_log_cmd() + ), + } + ) + + def get_copy_log_cmd(self) -> str: + """Get the juju command to copy the ad-hoc tempest log locally.""" + return f"$ juju scp -m {self.model.name} --container {CONTAINER} {self.unit.name}:{TEMPEST_ADHOC_OUTPUT} validation.log" def _on_get_lists_action(self, event: ops.charm.ActionEvent) -> None: """List tempest test lists action.""" diff --git a/charms/tempest-k8s/src/handlers.py b/charms/tempest-k8s/src/handlers.py index 51974161..ae1b35f3 100644 --- a/charms/tempest-k8s/src/handlers.py +++ b/charms/tempest-k8s/src/handlers.py @@ -41,9 +41,10 @@ from utils.constants import ( OPENSTACK_PROJECT, OPENSTACK_ROLE, OPENSTACK_USER, + TEMPEST_ADHOC_OUTPUT, TEMPEST_HOME, TEMPEST_LIST_DIR, - TEMPEST_OUTPUT, + TEMPEST_PERIODIC_OUTPUT, ) logger = logging.getLogger(__name__) @@ -241,7 +242,7 @@ class TempestPebbleHandler(sunbeam_chandlers.ServicePebbleHandler): ] try: - output = self.execute( + summary = self.execute( args, user="tempest", group="tempest", @@ -249,14 +250,14 @@ class TempestPebbleHandler(sunbeam_chandlers.ServicePebbleHandler): exception_on_error=True, environment=env, ) - except ops.pebble.ExecError as e: - if e.stdout: - output = f"{e.stdout}\n\n{e.stderr}" - else: - output = e.stderr - raise RuntimeError(output) + except ops.pebble.ExecError: + raise RuntimeError( + "Error during test execution.\n" + "For more information, copy log file from container by running:\n" + + self.charm.get_copy_log_cmd() + ) - return output + return summary class TempestUserIdentityRelationHandler(sunbeam_rhandlers.RelationHandler): @@ -576,7 +577,14 @@ class LoggingRelationHandler(sunbeam_rhandlers.RelationHandler): recursive=True, relation_name=self.relation_name, alert_rules_path="src/loki_alert_rules", - logs_scheme={"tempest": {"log-files": [TEMPEST_OUTPUT]}}, + logs_scheme={ + "tempest": { + "log-files": [ + TEMPEST_PERIODIC_OUTPUT, + TEMPEST_ADHOC_OUTPUT, + ] + } + }, ) return interface diff --git a/charms/tempest-k8s/src/templates/tempest-run-wrapper.j2 b/charms/tempest-k8s/src/templates/tempest-run-wrapper.j2 index 01033fc0..50c7d8d7 100644 --- a/charms/tempest-k8s/src/templates/tempest-run-wrapper.j2 +++ b/charms/tempest-k8s/src/templates/tempest-run-wrapper.j2 @@ -1,17 +1,39 @@ #!/bin/bash # Do not change this file, this file is managed by juju. -# set -e is important, to ensure the script bails out -# if there are issues, such as lock not acquired, -# or failure in one of the tempest steps. -set -ex - -(flock -n 9 || (echo "lock could not be acquired"; exit 1) - -discover-tempest-config --test-accounts "$TEMPEST_TEST_ACCOUNTS" --out "$TEMPEST_CONF" +(flock -n 9 || { + # curly braces are important here, as they retain the same execution environment. + # we don't want to start a new subshell + echo "Lock could not be acquired, another test run is in progress." + exit 1 +} +# log everything to a tmpfile TMP_FILE="$(mktemp)" -tempest run --workspace "$TEMPEST_WORKSPACE" "$@" 2>&1 | tee "$TMP_FILE" + +echo ":: discover-tempest-config" >> "$TMP_FILE" +if discover-tempest-config --test-accounts "$TEMPEST_TEST_ACCOUNTS" --out "$TEMPEST_CONF" >> "$TMP_FILE" 2>&1; then + echo ":: tempest run" >> "$TMP_FILE" + tempest run --workspace "$TEMPEST_WORKSPACE" "$@" >> "$TMP_FILE" 2>&1 +else + echo ":: skipping tempest run because discover-tempest-config had errors" >> "$TMP_FILE" +fi + +# tempest and discover-tempest-config can output escape sequences, +# so remove them to neaten the output. +sed $'s/\033\[[0-9;]*m//g' -i "$TMP_FILE" + +# After everything, move it to the actual output. +# This ensures we don't have issues with logging libs pushing partial files, +# if we were to stream to the final output. mv "$TMP_FILE" "$TEMPEST_OUTPUT" +SUMMARY="$(awk '/^Totals$/,/Sum of execute/ { print }' < "$TEMPEST_OUTPUT")" +if [[ -n "$SUMMARY" ]]; then + echo "$SUMMARY" +else + echo "Error running the tests, please view the log file" + exit 1 +fi + ) 9>/var/lock/tempest diff --git a/charms/tempest-k8s/src/utils/constants.py b/charms/tempest-k8s/src/utils/constants.py index 1c40d56e..c794502e 100644 --- a/charms/tempest-k8s/src/utils/constants.py +++ b/charms/tempest-k8s/src/utils/constants.py @@ -21,7 +21,8 @@ TEMPEST_CONF = f"{TEMPEST_WORKSPACE_PATH}/etc/tempest.conf" TEMPEST_TEST_ACCOUNTS = f"{TEMPEST_WORKSPACE_PATH}/test_accounts.yaml" TEMPEST_LIST_DIR = "/tempest_test_lists" # this file will contain the output from tempest's latest test run -TEMPEST_OUTPUT = f"{TEMPEST_WORKSPACE_PATH}/tempest-output.log" +TEMPEST_PERIODIC_OUTPUT = f"{TEMPEST_WORKSPACE_PATH}/tempest-periodic.log" +TEMPEST_ADHOC_OUTPUT = f"{TEMPEST_WORKSPACE_PATH}/tempest-validation.log" # This is the workspace name registered with tempest. # It will be saved in a file in $HOME/.tempest/ TEMPEST_WORKSPACE = "tempest" diff --git a/charms/tempest-k8s/src/utils/types.py b/charms/tempest-k8s/src/utils/types.py new file mode 100644 index 00000000..db9d726f --- /dev/null +++ b/charms/tempest-k8s/src/utils/types.py @@ -0,0 +1,37 @@ +# Copyright 2024 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Types for the tempest charm.""" +from enum import ( + Enum, +) + +from .constants import ( + TEMPEST_ADHOC_OUTPUT, + TEMPEST_PERIODIC_OUTPUT, +) + + +class TempestEnvVariant(Enum): + """Represent a variant of the standard tempest environment.""" + + PERIODIC = 1 + ADHOC = 2 + + def output_path(self) -> str: + """Return the correct tempest output path.""" + return ( + TEMPEST_PERIODIC_OUTPUT + if self.value == self.PERIODIC.value + else TEMPEST_ADHOC_OUTPUT + ) diff --git a/charms/tempest-k8s/tests/unit/test_tempest_charm.py b/charms/tempest-k8s/tests/unit/test_tempest_charm.py index e9a10906..2c36a0a2 100644 --- a/charms/tempest-k8s/tests/unit/test_tempest_charm.py +++ b/charms/tempest-k8s/tests/unit/test_tempest_charm.py @@ -25,9 +25,14 @@ import ops_sunbeam.test_utils as test_utils import yaml from utils.constants import ( CONTAINER, + TEMPEST_ADHOC_OUTPUT, TEMPEST_HOME, + TEMPEST_PERIODIC_OUTPUT, TEMPEST_READY_KEY, ) +from utils.types import ( + TempestEnvVariant, +) TEST_TEMPEST_ENV = { "OS_REGION_NAME": "RegionOne", @@ -44,7 +49,7 @@ TEST_TEMPEST_ENV = { "TEMPEST_CONF": "/var/lib/tempest/workspace/etc/tempest.conf", "TEMPEST_HOME": "/var/lib/tempest", "TEMPEST_LIST_DIR": "/tempest_test_lists", - "TEMPEST_OUTPUT": "/var/lib/tempest/workspace/tempest-output.log", + "TEMPEST_OUTPUT": "/var/lib/tempest/workspace/tempest-validation.log", "TEMPEST_TEST_ACCOUNTS": "/var/lib/tempest/workspace/test_accounts.yaml", "TEMPEST_WORKSPACE": "tempest", "TEMPEST_WORKSPACE_PATH": "/var/lib/tempest/workspace", @@ -248,8 +253,10 @@ class TestTempestOperatorCharm(test_utils.CharmTestCase): "test-list": "", } self.harness.charm._on_validate_action(action_event) - action_event.fail.assert_called_with( - "'test(' is an invalid regex: missing ), unterminated subpattern at position 4" + action_event.fail.assert_called_once() + self.assertEqual( + "'test(' is an invalid regex: missing ), unterminated subpattern at position 4", + action_event.set_results.call_args.args[0]["error"], ) self.harness.remove_relation(logging_rel_id) @@ -281,8 +288,10 @@ class TestTempestOperatorCharm(test_utils.CharmTestCase): "test-list": "nonexistent", } self.harness.charm._on_validate_action(action_event) - action_event.fail.assert_called_with( - "'nonexistent' is not a known test list. Please run list-tests action to view available lists." + action_event.fail.assert_called_once() + self.assertEqual( + "'nonexistent' is not a known test list. Please run list-tests action to view available lists.", + action_event.set_results.call_args.args[0]["error"], ) self.harness.remove_relation(logging_rel_id) @@ -401,7 +410,7 @@ class TestTempestOperatorCharm(test_utils.CharmTestCase): action_event.fail.assert_called_once() self.assertIn( "No filter parameters provided", - action_event.fail.call_args.args[0], + action_event.set_results.call_args.args[0]["error"], ) exec_mock.assert_not_called() @@ -610,3 +619,12 @@ class TestTempestOperatorCharm(test_utils.CharmTestCase): self.harness.charm.set_tempest_ready = mock.Mock() self.harness.charm._on_upgrade_charm(mock.Mock()) self.harness.charm.set_tempest_ready.assert_called_once_with(False) + + def test_tempest_env_variant(self): + """Test env variant for tempest returns correct path.""" + self.assertEqual( + TempestEnvVariant.PERIODIC.output_path(), TEMPEST_PERIODIC_OUTPUT + ) + self.assertEqual( + TempestEnvVariant.ADHOC.output_path(), TEMPEST_ADHOC_OUTPUT + )