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.

Sign up for GitHub

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

Jump to bottom

lxd: Add support for starting instances on creation (from Incus) #13695

Merged
merged 7 commits into from
Jul 25, 2024
Merged

lxd: Add support for starting instances on creation (from Incus) #13695

merged 7 commits into from
Jul 25, 2024

Conversation

boltmark
Copy link
Contributor

@boltmark boltmark commented Jul 1, 2024 •
edited
Loading

This PR adds support for starting instances on creation.

Includes cherry-picks from lxc/incus#410.

Closes #13322

@boltmark boltmark added the API Changes to the REST API label Jul 1, 2024
@github-actions github-actions bot added the Documentation Documentation needs updating label Jul 1, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 1, 2024

Heads up @ru-fu - the "Documentation" label was applied to this issue.

@ru-fu
Copy link
Contributor

ru-fu commented Jul 2, 2024

This will also need a doc update to the API tab for creating an instance: https://documentation.ubuntu.com/lxd/en/latest/howto/instances_create/
The other tabs say that you have the choice between init & launch / create & create and start, while the API says you have to start the instance separately.

You can find a short intro to how the docs work here - get in touch if anything is unclear. :)

@tomponline
Copy link
Member

Hi @boltmark looks like not all of the commits have been cherry-picked correctly as the CLA test is failing.

Also please can you sign your commits, see https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification

Thanks!

@tomponline tomponline changed the title lxd: Add support for starting instances on creation -- from Incus lxd: Add support for starting instances on creation (from Incus) Jul 2, 2024
@boltmark boltmark force-pushed the instance-start-on-creation branch 2 times, most recently from 932518a to 4cbddb4 Compare July 2, 2024 19:54
@boltmark
Copy link
Contributor Author

boltmark commented Jul 2, 2024

Thanks for the comments.
@tomponline -- CLA test is now passing.
@ru-fu -- I updated the documentation to include information + an example on starting containers upon creation.

@boltmark boltmark marked this pull request as ready for review July 2, 2024 20:51
@boltmark boltmark requested a review from tomponline as a code owner July 2, 2024 20:51
ru-fu
ru-fu reviewed Jul 3, 2024
View reviewed changes
Copy link
Contributor

@ru-fu ru-fu left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good! Two small comments (but none are blocking).

doc/howto/instances_create.md Outdated Show resolved Hide resolved
doc/api-extensions.md Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 3, 2024
View reviewed changes
lxd/instances_post.go Outdated Show resolved Hide resolved
@tomponline tomponline mentioned this pull request Jul 3, 2024
Closed
@boltmark
Copy link
Contributor Author

@tomponline Ready for review. Looks like the documentation test is failing due to criu.org being down, but hopefully this is temporary.

tomponline
tomponline reviewed Jul 24, 2024
View reviewed changes
lxc/init.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 24, 2024
View reviewed changes
lxd/instances_post.go Outdated Show resolved Hide resolved
tomponline
tomponline requested changes Jul 24, 2024
View reviewed changes
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Couple of nits and question about the migration fix

@stgraber @boltmark
api: instance_create_start
090e6b7 
Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
Sponsored-by: Lanes & Planes GmbH (https://www.lanes-planes.com)
(cherry picked from commit a00b1aa4a713c7eb1460701ac6127c151fe72548)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
@stgraber @boltmark
shared/api: Add Start to InstancesPost
22b2b58 
Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
Sponsored-by: Lanes & Planes GmbH (https://www.lanes-planes.com)
(cherry picked from commit d85d6843ed5631660ca2375e797597174715c83f)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
@stgraber @boltmark
doc/rest-api: Refresh swagger YAML
7d42e41 
Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
Sponsored-by: Lanes & Planes GmbH (https://www.lanes-planes.com)
(cherry picked from commit b8c293da5a3399b8b16dcd2b5875dc5b4dd5323f)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
@stgraber @boltmark
lxd/instance: Add support for Start property
81f758b 
Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
Sponsored-by: Lanes & Planes GmbH (https://www.lanes-planes.com)
(cherry picked from commit 2405063803f5cc79198910014c48f4497b2ac091)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
@stgraber @boltmark
lxc/launch: Use the Start property
48dc9c4 
Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
Sponsored-by: Lanes & Planes GmbH (https://www.lanes-planes.com)
(cherry picked from commit aa15dd4d73dd3fee6d7abf19df307051d4ebfee7)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
@boltmark
doc/howto: Update documentation on start field
1428e4a 
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
@boltmark
i18n: Update translation templates
846fe00 
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
@boltmark
Copy link
Contributor Author

Couple of nits and question about the migration fix

Thanks again @tomponline! I've resolved the nits and provided an alternative fix for the migration issue. Let me know what you think.

tomponline
tomponline approved these changes Jul 25, 2024
View reviewed changes
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Thanks!

@tomponline tomponline merged commit 0021632 into canonical:main Jul 25, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ru-fu ru-fu ru-fu left review comments

@tomponline tomponline tomponline approved these changes

Assignees
No one assigned
Labels
API Changes to the REST API Documentation Documentation needs updating
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add create and start endpoint
4 participants
@boltmark @ru-fu @tomponline @stgraber