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

Respect storage pool configuration when moving instance between projects #12412

Merged

Conversation

MusicDin
Copy link
Member

@MusicDin MusicDin commented Oct 18, 2023

If --storage flag is used along --target-project flag in move command, ensure the instance is migrated to another project as well.

In addition, when migrating to another project, determine root disk device in the following order:

  1. Check if local devices already contain root disk device
  2. Check if any of the profiles that will be applied to the instance in the target project contain configuration for root disk device
  3. Lastly, set the current root disk device to the instance's configuration (found from expended devices).

When using server-side move, flags --profile, --no-profiles, --device are not respected. Therefore, step 2. only works for profiles that are already applied to the instance in source project and exist in the target project.

A very simple solution would be to skip server-side project move if any of the above flags is set. This way, LXD would copy the instance (ensuring profiles and devices are applied).

If we are willing to support server-side move of the instance while respecting other flags, then we would most likely need to expand moveInstanceProject().


There is also a small inconsistency between "server-side" move and "move using copy". When moving an instance whose root disk device is configured in the profile, a root disk device configuration is moved to the instance's config. On the other side, copy ("move using copy") will not do that.

@tomponline
Copy link
Member

@MusicDin as discussed lets move this logic to the server side so we can sequentially action both a storage pool and project move in a single request.

@MusicDin MusicDin force-pushed the fix/move-instance-between-projects branch 2 times, most recently from 65e79e3 to 80fac0d Compare October 25, 2023 09:04
@tomponline
Copy link
Member

needs a rebase

@MusicDin MusicDin force-pushed the fix/move-instance-between-projects branch 3 times, most recently from 2bfaef5 to cc123f7 Compare October 25, 2023 14:03
@MusicDin
Copy link
Member Author

MusicDin commented Oct 25, 2023

Structure api.InstancesPost (which copy uses when calling instancesPost()) embeds api.InstancePut which contains Profiles, Devices and Config fields.

Structure api.InstancePost (which move uses when calling instancePost()) does not have such fields.

Would embedding api.InstancePut into api.InstancePost make sense, considering that it does not fit there and it would only be required for one specific use case - server-side move? Maybe we could even handle the move within instancesPost() (similar to copy)?

As a quickfix, I just added a condition that ensures server-side move is only used if custom profiles, devices or configurations are not provided. If they are, move is done using copy, which is still better compared to the current situation where those flags are completely ignored.

@MusicDin MusicDin force-pushed the fix/move-instance-between-projects branch 2 times, most recently from b6c7e92 to a3bb306 Compare October 26, 2023 07:47
@MusicDin MusicDin force-pushed the fix/move-instance-between-projects branch 3 times, most recently from bef45a5 to d5f298f Compare November 8, 2023 07:20
@MusicDin MusicDin force-pushed the fix/move-instance-between-projects branch from d5f298f to b9513d9 Compare November 22, 2023 15:55
@MusicDin
Copy link
Member Author

This is the first part that should be ported to 5.0 LTS and is done (if there is no comments and tests pass).

Just to give some context, flags like --profile, --no-profiles, --device were not respected during server-side move. With this PR in place, an error will be thrown instead of ignoring those flags.

Another PR is coming that will introduce a new API extension and will add support for the above flags.

@MusicDin MusicDin marked this pull request as ready for review November 22, 2023 16:24
@MusicDin MusicDin force-pushed the fix/move-instance-between-projects branch from b9513d9 to fd7fd6d Compare November 23, 2023 14:35
Signed-off-by: Din Music <din.music@canonical.com>
Signed-off-by: Din Music <din.music@canonical.com>
Signed-off-by: Din Music <din.music@canonical.com>
Signed-off-by: Din Music <din.music@canonical.com>
@MusicDin MusicDin force-pushed the fix/move-instance-between-projects branch from fd7fd6d to 94a87f6 Compare November 27, 2023 12:03
@MusicDin
Copy link
Member Author

@tomponline Follow up on our discussion this morning:

  1. I've renamed the error message.

  2. VMs are migrated as expected. Before this PR, the instance is migrated only to different storage pool or project. With this PR in place, instance is migrated to different project and storage pool with the same command.

$ lxc project create pmove
$ lxc storage create smove zfs

$ lxc launch ubuntu:22.04 v2 --vm -c migration.stateful=true

$ lxc move v2 --target-project pmove --storage smove
$ lxc config show v2 --project pmove
...
devices:
  root:
    path: /
    pool: smove
    size.state: 4GiB
    type: disk
  1. Provided configuration keys (using --config flag) are never respected, because they are not passed to the server in the first place.

@tomponline
Copy link
Member

$ lxc move v2 --target-project pmove --storage smove

What about with --target flag to a different host?

@MusicDin
Copy link
Member Author

Yep, just testing that out, will let you know

@MusicDin
Copy link
Member Author

What about with --target flag to a different host?

@tomponline moving instance to another project/storage and another target in the same time is not supported:

$ lxc move v2 --target-project pmove --storage smove --target cls-2
# Error: The --storage flag can't be used with --target

$ lxc move v2 --target-project pmove --target cls-2
# Error: The --target-project flag can't be used with --target

However, moving to another target, works normally:

$ lxc move v2 --target cls-2
$ lxc move v2 --target-project pmove --storage smove

$ lxc ls --project pmove
# +------+---------+-------------------------+-------------------------------------------------+-----------------+-----------+----------+
# | NAME |  STATE  |          IPV4           |                      IPV6                       |      TYPE       | SNAPSHOTS | LOCATION |
# +------+---------+-------------------------+-------------------------------------------------+-----------------+-----------+----------+
# | v2   | RUNNING | 10.195.159.139 (enp5s0) | fd42:5563:fe39:e0cc:216:3eff:fe44:40b5 (enp5s0) | VIRTUAL-MACHINE | 0         | cls-2    |
# +------+---------+-------------------------+-------------------------------------------------+-----------------+-----------+----------+

$ lxc config show v2 --project pmove
# ...
# devices:
#   root:
#     path: /
#     pool: smove
#     size.state: 4GiB
#     type: disk

@tomponline tomponline merged commit 19f27f9 into canonical:main Nov 27, 2023
26 checks passed
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.

2 participants