Update Juju process calls

[himax16]

This PR contains multiple fixes for calling Juju from the Sunbeam commands:

• Ensure that Juju is logged in before any command that requires Juju (3a97e32)
• Make sure pexpect.spawn calls are terminated properly (e3caef9)
• Consolidate Juju subprocess calls to JujuStepHelper._juju_cmd (dd1ada2)

This solves the issues in LP#2150473 and LP#2150662, where Sunbeam CLI commands hangs up when there is no logged in Juju account.

To solve this, we make sure that any Step that requires Juju calls are preceded with JujuLoginStep. We also wrap the pexpect.spawn calls for Juju interaction with context manager to prevent TTY errors. Additionally we also centralize the direct Juju calls through subprocess in the JujuStepHelper._juju_cmd.

[Copilot]

Pull request overview

This PR improves reliability of Sunbeam’s Juju interactions by ensuring commands run with a valid Juju session, consolidating Juju CLI invocation into a shared helper, and tightening subprocess/pexpect lifecycle handling.

Changes:

• Add a JujuLoginStep run before many CLI entrypoints that require Juju access.
• Consolidate many direct subprocess.run([...juju...]) usages into JujuStepHelper._juju_cmd() (with auth error detection).
• Update unit tests to reflect the new Juju login step sequencing and updated spawn/command execution behavior.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
sunbeam-python/sunbeam/core/juju.py	Introduces JujuAuthenticationError, auth-error detection, and extends _juju_cmd to support env/cwd/timeout and centralized error handling.
sunbeam-python/sunbeam/steps/juju.py	Refactors multiple Juju CLI calls to _juju_cmd and updates pexpect usage to context-managed spawn.
sunbeam-python/sunbeam/provider/maas/commands.py	Runs JujuLoginStep before certain MAAS provider commands that invoke Juju.
sunbeam-python/sunbeam/provider/local/commands.py	Runs JujuLoginStep before certain local provider commands that invoke Juju.
sunbeam-python/sunbeam/provider/commands.py	Runs JujuLoginStep before updating clusterd credentials.
sunbeam-python/sunbeam/features/interface/v1/base.py	Ensures feature enable/disable flows log into Juju first.
sunbeam-python/sunbeam/features/vault/feature.py	Logs into Juju before Vault operational commands.
sunbeam-python/sunbeam/features/validation/feature.py	Switches validation feature’s Juju subprocess calls to _juju_cmd via JujuStepHelper.
sunbeam-python/sunbeam/features/observability/feature.py	Logs into Juju before retrieving COS dashboard URL.
sunbeam-python/sunbeam/features/maintenance/commands.py	Logs into Juju before maintenance enable/disable actions.
sunbeam-python/sunbeam/features/loadbalancer/feature.py	Logs into Juju before load balancer operations when secrets are enabled.
sunbeam-python/sunbeam/features/ldap/feature.py	Logs into Juju before LDAP domain operations.
sunbeam-python/sunbeam/features/baremetal/feature.py	Logs into Juju before baremetal conductor-group operations.
sunbeam-python/sunbeam/commands/sso.py	Logs into Juju before SSO add/remove/update/purge-related flows.
sunbeam-python/sunbeam/commands/resize.py	Logs into Juju before resize plan execution.
sunbeam-python/sunbeam/commands/refresh.py	Logs into Juju before refresh/refresh-mysql/refresh-vault execution.
sunbeam-python/sunbeam/commands/proxy.py	Logs into Juju before applying proxy changes via Juju/Terraform plans.
sunbeam-python/sunbeam/commands/plans.py	Logs into Juju before launching a plan shell environment.
sunbeam-python/sunbeam/commands/openrc.py	Logs into Juju before retrieving admin openrc via Keystone action.
sunbeam-python/sunbeam/commands/launch.py	Logs into Juju before fetching credentials needed to launch an instance.
sunbeam-python/sunbeam/commands/generate_cloud_config.py	Logs into Juju before generating cloud config that depends on Juju/Keystone access.
sunbeam-python/sunbeam/commands/dashboard_url.py	Logs into Juju before retrieving Horizon dashboard URL.
sunbeam-python/tests/unit/sunbeam/steps/test_juju.py	Updates tests for context-managed pexpect.spawn and _juju_cmd consolidation.
sunbeam-python/tests/unit/sunbeam/provider/test_commands.py	Adjusts expectations for extra run_plan invocation (login step + actual plan).
sunbeam-python/tests/unit/sunbeam/features/test_loadbalancer.py	Adjusts mocking/expectations for the added Juju login plan call.
sunbeam-python/tests/unit/sunbeam/features/maintenance/test_commands.py	Adds patching for new run_plan call introduced by Juju login.
sunbeam-python/tests/unit/sunbeam/core/test_juju.py	Adds unit coverage for auth-error detection and expected exception behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[hmlanigan]

The commit messages and pr description focus on what was done. Please add why these changes were made. Or why you choose a certain approach versus another. In 2 months or so, that will be the more interesting information you may have forgotten.

Commit messages and PR descriptions are where we can be kind to our future selves when a bug comes up.

[hemanthnakkina]

I have an inline comment.

Can you check if something like this works or not instead of changing in all the places.

@property
 def juju_controller(self) -> JujuController | None:
     if not getattr(self, "_juju_logged_in", False) and self._juju_controller:
         run_plan([JujuLoginStep(self.juju_account)], console)
         self._juju_logged_in = True
     return self._juju_controller

[himax16]

 @property
 def juju_controller(self) -> JujuController | None:
     if not getattr(self, "_juju_logged_in", False) and self._juju_controller:
         run_plan([JujuLoginStep(self.juju_account)], console)
         self._juju_logged_in = True
     return self._juju_controller

After some testing, checking the status for "__juju_logged_in" involves the same procedure similar to what is in the is_skip method of JujuLoginStep. If we always force any call to deployment.juju_controller to run JujuLoginStep, it might be too expensive.

Thinking again about this I am changing them to an explicit Check class.

[Copilot]

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.

[gboutry]

def juju_controller(self) -> JujuController | None:
if not getattr(self, "_juju_logged_in", False) and self._juju_controller:
run_plan([JujuLoginStep(self.juju_account)], console)
self._juju_logged_in = True
return self._juju_controller

I agree with hima that this side effect on a property would be too heavy.

We should ideally come with a better way to ensure we're connected to the controller, maybe with a context manager?

But for this pull request, I'm fine with moving forward on this approach to fix the current pain points.

@himax16 can you raise a LP bug about this refactor? So that we don't lose track of it.