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

Updates lxd_container to support new LXD API #7980

Merged
merged 2 commits into from Feb 28, 2024

Conversation

anthonyra
Copy link
Contributor

@anthonyra anthonyra commented Feb 17, 2024

SUMMARY

Updates the lxd_container module to work with the new LXD API which deprecates /1.0/containers and /1.0/virtual-machines and adds /1.0/instances. It tries to keep backwards compatibility with a try and catch compared to detecting the API version then determining which API endpoint to use.

It also adds support for Incus

Fixes #7853

ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
  • New Module/Plugin Pull Request
  • Refactoring Pull Request
  • Test Pull Request
COMPONENT NAME

lxd_container

ADDITIONAL INFORMATION

@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added WIP Work in progress bug This issue/PR relates to a bug merge_commit This PR contains at least one merge commit. Please resolve! module module needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html new_contributor Help guide this first time contributor labels Feb 17, 2024
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Feb 17, 2024
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-7 Automatically create a backport for the stable-7 branch backport-8 Automatically create a backport for the stable-8 branch labels Feb 17, 2024
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

plugins/modules/lxd_container.py Outdated Show resolved Hide resolved
plugins/modules/lxd_container.py Outdated Show resolved Hide resolved
plugins/modules/lxd_container.py Outdated Show resolved Hide resolved
plugins/modules/lxd_container.py Outdated Show resolved Hide resolved
plugins/modules/lxd_container.py Outdated Show resolved Hide resolved
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Feb 20, 2024
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI and removed ci_verified Push fixes to PR branch to re-run CI labels Feb 20, 2024
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Feb 20, 2024
@ansibullbot ansibullbot added plugins plugin (any type) and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Feb 20, 2024
# LXD (3.19) Rest API provides instance endpoint, failback to containers and virtual-machines
self.api_endpoint = '/1.0/instances'
try:
self.client.do('GET', '{0}/{1}'.format(self.api_endpoint, ''), ok_error_codes=[404])
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to check the return value whether it's a 404 error, instead of trying to catch an exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When working out the best approach to do this, I found/figured out that this approach would return 404 for endpoints that didn't exist even. For example, /1.0/test would result in 404 like /1.0/instances. However, adding the ?project filter with a blank project would result in the API response I was looking for.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI and removed ci_verified Push fixes to PR branch to re-run CI labels Feb 21, 2024
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Feb 21, 2024
@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Feb 21, 2024
@felixfontein
Copy link
Collaborator

The code change looks good to me (as far as I can judge). Can you please add a changelog fragment? Thanks.

@sleepy-nols
Copy link

can confirm, runs for me without problems

@felixfontein
Copy link
Collaborator

(And mark the PR as ready when you think you're done :) )

@anthonyra anthonyra marked this pull request as ready for review February 27, 2024 20:21
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed WIP Work in progress labels Feb 27, 2024
Co-authored-by: Felix Fontein <felix@fontein.de>
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Feb 28, 2024
@felixfontein felixfontein merged commit 74c15c1 into ansible-collections:main Feb 28, 2024
121 checks passed
Copy link

patchback bot commented Feb 28, 2024

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/74c15c12418bf2f7e1dcac1a88657c8aefaeeb6d/pr-7980

Backported as #8045

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Feb 28, 2024
patchback bot pushed a commit that referenced this pull request Feb 28, 2024
* feat: ✨ adds support for /1.0/instances endpoint

* Update changelogs/fragments/lxd-instances-api-endpoint-added.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 74c15c1)
Copy link

patchback bot commented Feb 28, 2024

Backport to stable-8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-8/74c15c12418bf2f7e1dcac1a88657c8aefaeeb6d/pr-7980

Backported as #8046

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Feb 28, 2024
* feat: ✨ adds support for /1.0/instances endpoint

* Update changelogs/fragments/lxd-instances-api-endpoint-added.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 74c15c1)
@felixfontein
Copy link
Collaborator

@anthonyra thanks a lot for your contribution!
@sleepy-nols thanks for testing this!

felixfontein pushed a commit that referenced this pull request Feb 28, 2024
…port new LXD API (#8045)

Updates `lxd_container` to support new LXD API (#7980)

* feat: ✨ adds support for /1.0/instances endpoint

* Update changelogs/fragments/lxd-instances-api-endpoint-added.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 74c15c1)

Co-authored-by: Anthony Anderson <fnanderson93@gmail.com>
felixfontein pushed a commit that referenced this pull request Feb 28, 2024
…port new LXD API (#8046)

Updates `lxd_container` to support new LXD API (#7980)

* feat: ✨ adds support for /1.0/instances endpoint

* Update changelogs/fragments/lxd-instances-api-endpoint-added.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 74c15c1)

Co-authored-by: Anthony Anderson <fnanderson93@gmail.com>
felixfontein added a commit that referenced this pull request Mar 25, 2024
…` to support new LXD API (#8045)"

This reverts commit 8b2e658.

(Considering this again, this looks a lot more like a feature than a bugfix. I shouldn't have backported it.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-7 Automatically create a backport for the stable-7 branch backport-8 Automatically create a backport for the stable-8 branch bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Incus in lxd_container module
4 participants