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

[BUG] lxd module imcompatible with pylxd >= 2.2.11 #59514

Closed
goapunk opened this issue Feb 16, 2021 · 5 comments · Fixed by #63911
Closed

[BUG] lxd module imcompatible with pylxd >= 2.2.11 #59514

goapunk opened this issue Feb 16, 2021 · 5 comments · Fixed by #63911
Labels
Bug broken, incorrect, or confusing behavior severity-high 2nd top severity, seen by most users, causes major problems

Comments

@goapunk
Copy link
Contributor

goapunk commented Feb 16, 2021

Description
The lxd module uses a hack which overwrites the FileManager class of pylxd. In pylxd >= 2.2.11 this class changed and creating container in salt will fail.

Setup
saltstack + pylxd==2.2.11

Steps to Reproduce the behavior
Create a container through the lxd module

Expected behavior
Creating container should work with pylxd >= 2.2.11

Additional context
The following patch should make it work on 2.2.11 but I did not test it thoroughly. In general getting rid of the hack would be much better.

diff --git a/salt/modules/lxd.py b/salt/modules/lxd.py                          
index 7c8f9fb..d3528ae 100644                                                   
--- a/salt/modules/lxd.py                                                       
+++ b/salt/modules/lxd.py                                                       
@@ -3656,9 +3656,8 @@ if HAS_PYLXD:                                             
                 headers["X-LXD-uid"] = six.text_type(uid)                      
             if gid is not None:                                                
                 headers["X-LXD-gid"] = six.text_type(gid)                      
-            response = self._client.api.containers[self._container.name].files.post(
-                params={"path": filepath}, data=data, headers=headers          
-            )                                                                  
+            response = self._instance.client.api.containers[self._instance.name]
+                .files.post(params={'path': filepath}, data=data, headers=headers)
             return response.status_code == 200                                 
                                                                               
     Container.FilesManager = FilesManager

Related to / Duplicate of #57822

@goapunk goapunk added the Bug broken, incorrect, or confusing behavior label Feb 16, 2021
@sagetherage sagetherage added severity-high 2nd top severity, seen by most users, causes major problems Silicon v3004.0 Release code name and removed needs-triage labels Apr 30, 2021
@sagetherage sagetherage added this to the Silicon milestone Apr 30, 2021
@sagetherage
Copy link
Contributor

we will review a PR if you are willing to submit one! :)

@sagetherage sagetherage modified the milestones: Silicon, Approved Aug 9, 2021
@Ch3LL Ch3LL added the Sulfur v3006.0 release code name and version label Oct 25, 2021
@Ch3LL Ch3LL modified the milestones: Approved, Sulphur v3006.0 Oct 25, 2021
@Ch3LL Ch3LL removed the Silicon v3004.0 Release code name label Oct 25, 2021
@Ch3LL
Copy link
Contributor

Ch3LL commented Oct 25, 2021

@goapunk would you be willing to submit a PR? We would also need to account for older versions of the library. If there is an easy way to check for the version we could take that approach, or possibly catch the exception and use the new attribute.

@goapunk
Copy link
Contributor Author

goapunk commented Oct 27, 2021

@Ch3LL I currently can't really say when I will have the time to work on it, so I have to pass for now.

@Ch3LL
Copy link
Contributor

Ch3LL commented Nov 2, 2021

No worries, thanks for getting back to me :)

@waynew waynew removed the Sulfur v3006.0 release code name and version label Dec 16, 2022
@goapunk
Copy link
Contributor Author

goapunk commented Mar 20, 2023

@Ch3LL I've opened #63911 to address this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior severity-high 2nd top severity, seen by most users, causes major problems
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants