Skip to content

Take architecture from source config while copying#18102

Draft
tugbataluy wants to merge 2 commits intocanonical:mainfrom
tugbataluy:take_architecture_from_source_config_while_copying
Draft

Take architecture from source config while copying#18102
tugbataluy wants to merge 2 commits intocanonical:mainfrom
tugbataluy:take_architecture_from_source_config_while_copying

Conversation

@tugbataluy
Copy link
Copy Markdown
Contributor

@tugbataluy tugbataluy commented Apr 10, 2026

Checklist

Description

Copying an instance via the API without specifying Architecture in the request returns an "Architecture is not supported" error as mentioned in #18100. The API should inherit the architecture from the source instance automatically, as it cannot be changed during copy.

Root cause

In instancesPost(), the SourceTypeCopy case populates req.Type and req.Profiles from the source instance but does not populate req.Architecture. The SourceTypeImage case does set it.

Fix

Populate req.Architecture from the source instance in the SourceTypeCopy case, consistent with how req.Type is already handled on the line above.

Fixes #18100

…n instance

Signed-off-by: tugbataluy <tugba.taluy@canonical.com>
…ainer copying test.

Signed-off-by: tugbataluy <tugba.taluy@canonical.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes instance copy requests that omit architecture by inheriting the architecture from the source instance (consistent with existing behavior for type and image-based creation), addressing API clients that build InstancesPost manually.

Changes:

  • Populate req.Architecture from the source instance in the SourceTypeCopy code path.
  • Add an integration test assertion intended to validate that copied containers retain the source architecture.

Reviewed changes

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

File Description
lxd/instances_post.go Sets the architecture on copy requests based on the source instance to prevent “Architecture is not supported” errors when the field is omitted.
test/suites/container_copy.sh Adds a new sub-test around architecture inheritance for copied containers (but currently does not reproduce the API scenario described in the issue).

Comment on lines +134 to +137
sub_test "Copied container inherits the source architecture"
source_arch="$(lxc query /1.0/instances/c1 | jq --raw-output '.architecture')"
target_arch="$(lxc query /1.0/instances/c2 | jq --raw-output '.architecture')"
[ "${source_arch}" = "${target_arch}" ]
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

This new sub-test does not actually exercise the reported API behavior (copy request without an Architecture field). lxc copy (CLI) fetches the source instance and calls CopyInstance, which sends InstancePut including the source architecture, so this test would still pass even before the server-side fix. To validate the fix, perform the copy using a raw API request (e.g. lxc query --request POST /1.0/instances -d ...) that omits architecture (and ideally omits type too), and assert the operation succeeds and the target instance is created with the source architecture.

Copilot uses AI. Check for mistakes.
Comment on lines +135 to +136
source_arch="$(lxc query /1.0/instances/c1 | jq --raw-output '.architecture')"
target_arch="$(lxc query /1.0/instances/c2 | jq --raw-output '.architecture')"
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The jq extraction uses --raw-output on .architecture, which will yield the literal string "null" if the field is missing; comparing two "null" strings would incorrectly pass. Consider using jq -er '.architecture' (and/or explicitly asserting it is non-empty) and declare source_arch/target_arch as local to match the rest of the function’s scoping style.

Suggested change
source_arch="$(lxc query /1.0/instances/c1 | jq --raw-output '.architecture')"
target_arch="$(lxc query /1.0/instances/c2 | jq --raw-output '.architecture')"
local source_arch
local target_arch
source_arch="$(lxc query /1.0/instances/c1 | jq -er '.architecture')"
target_arch="$(lxc query /1.0/instances/c2 | jq -er '.architecture')"

Copilot uses AI. Check for mistakes.
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 this pull request may close these issues.

lxc copy API requires architecture in request

2 participants