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

awaited juju Actions that are complete or failed, don't reflect that status #719

Closed
addyess opened this issue Aug 23, 2022 · 6 comments · Fixed by #753
Closed

awaited juju Actions that are complete or failed, don't reflect that status #719

addyess opened this issue Aug 23, 2022 · 6 comments · Fixed by #753

Comments

@addyess
Copy link
Contributor

addyess commented Aug 23, 2022

action.wait() returns itself with updated action.results, but the action.data is stale.

this can result in situations where the results have valid data, but action.success may say "pending" or "running" when in reality it's "completed" or "failed".

@addyess
Copy link
Contributor Author

addyess commented Aug 23, 2022

#717 was an attempt to fix this but failed with further attention

@cderici
Copy link
Contributor

cderici commented Aug 29, 2022

@addyess could you please confirm that this is still happening, so I'll put it in my queue? Thanks!

@addyess
Copy link
Contributor Author

addyess commented Aug 29, 2022

@cderici yes unless there's a new juju release after 3.0.1, this issue still plagues us. I'd think something as simple as a test like this would fail

async def test_awaited_action_completed():
   ubuntu = juju.deploy("ubuntu")
   ...
   action = await ubuntu.units[0].run("sleep 30")
   before = datetime.now()
   result = await action.wait()
   assert result.success == "completed", "Shouldn't be 'pending' -- only completed or failed"
   after = datetime.now()
   # test (after - before) >= 30 seconds

@jneo8
Copy link

jneo8 commented Sep 14, 2022

Same issue on https://launchpad.net/charm-infra-node

@addyess
Copy link
Contributor Author

addyess commented Oct 21, 2022

I decided to test again with juju==3.0.2 and the results have shifted a bit

        before = datetime.now()
        action = await unit.run("sleep 30; echo 'Done'")
        action = await action.wait()
        delta = datetime.now() - before
        assert action.status == "completed", f"Should be running after 30, but was {action.status} after {delta}"

The test fails like this, so it indicates the sleep ran to completion

Should be running after 30, but was running after 0:00:33.946744

You can see it completed the echo

>>> action.results
{'return-code': 0, 'stdout': 'Done\n'}
>>> action.safe_data
{'model-uuid': '6d0284b0-6043-42d5-8...c7cc9584ee', 'id': '95', 'receiver': 'kubernetes-control-plane/0', 'name': 'juju-run', 'status': 'running', 'message': '', 'enqueued': '2022-10-21T17:30:25Z', 'started': '2022-10-21T17:30:25Z', 'completed': '0001-01-01T00:00:00Z'}
>>> action.data
{'model-uuid': '6d0284b0-6043-42d5-8...c7cc9584ee', 'id': '95', 'receiver': 'kubernetes-control-plane/0', 'name': 'juju-run', 'status': 'running', 'message': '', 'enqueued': '2022-10-21T17:30:25Z', 'started': '2022-10-21T17:30:25Z', 'completed': '0001-01-01T00:00:00Z'}

@addyess
Copy link
Contributor Author

addyess commented Oct 21, 2022

its interesting that in model.py

    async def get_action_output(self, action_uuid, wait=None):
        """ Get the results of an action by ID.

        :param str action_uuid: Id of the action
        :param int wait: Time in seconds to wait for action to complete.
        :return dict: Output from action
        :raises: :class:`JujuError` if invalid action_uuid
        """
        action = await self._get_completed_action(action_uuid, wait=wait)
        # ActionResult.output is None if the action produced no output
        return {} if action.output is None else action.output

the variable action ACTUALLY HAS the completed=True, but only the actrion.output is returned from this method

cderici added a commit to cderici/python-libjuju that referenced this issue Oct 21, 2022
cderici added a commit to cderici/python-libjuju that referenced this issue Oct 21, 2022
jujubot added a commit that referenced this issue Oct 21, 2022
#753

#### Description

This fixes the way we set the status of an action that's ran on a unit. Previously only the `action.output` was being returned and the actual status of the action was being retrieved from the `self.data[]` (as the Action object is a modelEntity), which is not reliable at the moment. This PR changes it to get it directly from the Action object returned by the Juju API when the action completes (with either `completed` or `failed`) status. So it should be completely accurate all the time.

Fixes #719

#### QA Steps

Following should pass %100.

```
tox -e integration -- tests/integration/test_unit.py::test_run
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants