Skip to content

fix: do not create deployment scripts for diskless deployments - #657

Open
r00ta wants to merge 1 commit into
canonical:3.8from
r00ta:backport-0c89ae1c5147367a59079886a3612a3e14f3083f-3.8
Open

r00ta wants to merge 1 commit into
canonical:3.8from
r00ta:backport-0c89ae1c5147367a59079886a3612a3e14f3083f-3.8

Conversation

@r00ta

@r00ta r00ta commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Ephemeral deployments do not use deployment scripts.

Resolves LP#2166890

(cherry-picked from 0c89ae1)

…ical#646)

Ephemeral deployments do not use deployment scripts.

Resolves LP#2166890
@r00ta
r00ta requested a review from a team September 14, 2026 14:13
@r00ta
r00ta changed the base branch from master to 3.8 September 14, 2026 14:13

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[maas-code-reviewer review]

LLM-generated review from https://github.com/canonical/maas-code-reviewer.
Intended to assist a human reviewer, not replace one — suggestions may be
incorrect, please verify before acting.

The change correctly fixes the issue where an ephemeral deploy would either create an unnecessary deployment script set or retain a stale one from a previously aborted standard deployment. I've left a couple of comments regarding an identical issue that likely exists with current_installation_script_set and a suggestion to make the test simulation a bit more robust.

# ALLOCATED without clearing current_deployment_script_set.
# Clear it so an ephemeral deployment doesn't point at a stale
# script set.
self.current_deployment_script_set = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like current_installation_script_set suffers from the exact same issue. In _start_deployment (which is also called during start()), if self.ephemeral_deploy is True, self.current_installation_script_set is skipped but not cleared. If a standard deployment was previously aborted, the node will retain a stale current_installation_script_set during the ephemeral deploy. Consider clearing self.current_installation_script_set = None in an else branch inside _start_deployment.

leftover_script_set = factory.make_ScriptSet(
node=node, result_type=RESULT_TYPE.DEPLOYMENT
)
node.current_deployment_script_set = leftover_script_set

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To properly simulate the state of a node after an aborted deployment, consider calling node.save() after setting node.current_deployment_script_set = leftover_script_set and before calling node.start(admin). Otherwise, the stale script set is only set on the in-memory object and not actually persisted to the database before the test exercises the fix (although node.start eventually calls node.save(), persisting it beforehand ensures the pre-condition perfectly mimics reality).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants