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

Snapshot Rollbacks & Deletion Not Working #13

Closed
AnushK-Fro opened this issue Nov 7, 2021 · 8 comments
Closed

Snapshot Rollbacks & Deletion Not Working #13

AnushK-Fro opened this issue Nov 7, 2021 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@AnushK-Fro
Copy link

AnushK-Fro commented Nov 7, 2021

Describe the bug
Trying to use the snapname class does not work (which results in rollback + deletion not working).

To Reproduce

  1. Create a Proxmox instance
  2. Attempt to use snapname

Desktop (please complete the following information):

  • OS: Windows

Additional context
Here's what we've tried:
$this->proxmox($server, $cluster)->qemu()->vmid($server->vmid)->snapshot()->snapname($snapname)->postRollback();
This appears to not work, as we get Class \"proxmox\\Api\\nodes\\qemu\\snapshot\\snapname\" not found.

When moving the postRollback function to the snapshot class itself (snapshot.php) and adding parameters for the name, it appears to work fine.

$this->proxmox($server, $cluster)->qemu()->vmid($server->vmid)->snapshot()->postRollback($snapname);

We added this function to snapshot.php:

public function postRollback($name){
        return connection::processHttpResponse(connection::postAPI($this->httpClient,$this->apiURL.$name.'/rollback/',$this->cookie));
    }

Here's the full code to the project that we're working on:
https://github.com/StratumPanel/Stratum-Panel
Code is in app/Services/Servers/SnapshotService.php

@MrKampf
Copy link
Owner

MrKampf commented Nov 7, 2021

Hi,

what version do you use?
The currently 3.7?
In the new version I have added new architecture and better performance.

Then you have to change something:
$pve->nodes()->node('example')->qemu()->vmId(1)->snapshot()->snapname('example')->rollback()->post();

@ericwang401
Copy link
Contributor

Hi,

what version do you use? The currently 3.7? In the new version I have added new architecture and better performance.

Then you have to change something: $pve->nodes()->node('example')->qemu()->vmId(1)->snapshot()->snapname('example')->rollback()->post();

Hey, this isn't related to this issue, but it seems kind of redundant/verbose to add rollback() and then post()

It might be better to just do return (new Rollback($this->getPve(), $this->getPathAdditional())->post() for the rollback method in snapname.php so a user would just have to call $pve->nodes()->node('example')->qemu()->vmId(1)->snapshot()->snapname('example')->rollback() without the extra ->post()

@MrKampf
Copy link
Owner

MrKampf commented Nov 8, 2021

Hey @ericwang401 ,

thanks a lot for your feedback. I will consider/integrate this in the next update, for single functions such as post.
Currently I have built this so to keep it consistent and so for all equally understandable. But of course I can also add a default return to something like this which makes it easier to use.
Thanks a lot!

@MrKampf MrKampf pinned this issue Nov 8, 2021
@MrKampf MrKampf added bug Something isn't working enhancement New feature or request labels Nov 8, 2021
@MrKampf MrKampf self-assigned this Nov 8, 2021
@MrKampf MrKampf removed the bug Something isn't working label Nov 8, 2021
@AnushK-Fro
Copy link
Author

AnushK-Fro commented Nov 16, 2021

Hi,

what version do you use? The currently 3.7? In the new version I have added new architecture and better performance.

Then you have to change something: $pve->nodes()->node('example')->qemu()->vmId(1)->snapshot()->snapname('example')->rollback()->post();

We just updated the API to the newest version and now trying to do something like: $this->proxmox($server, $cluster)->qemu()->vmid($server->vmid)->status()->start()->post(); doesn't seem to work correctly as it seems to do nothing. I also don't get any errors or output :(

I was able to make it fetch the status though, with this:
return $this->proxmox($server, $cluster)->qemu()->vmid($server->vmid)->status()->current()->get();

Here's our full project (Stratum-Next branch):
https://github.com/StratumPanel/Stratum-Panel/tree/Stratum-Next

Start function is in the PowerService service under app/Services/Servers.

By the way, great job on the API so far!

@MrKampf
Copy link
Owner

MrKampf commented Nov 16, 2021

Thanks for report.

I have fixed the issue. The problem was i have sent a Content-Type in Header and no content was sent, and this wasn't accepted proxmox in api.

When you want debug, the error you can add behind the authType, true and then you get debug report.

$proxmox = new PVE('example','dev', 'password',8006, 'pve', true);

The last true activate the debug mode.

Thank you for your praise

Thank you for helping me to improve this library and find errors that I can't always find right away.

@MrKampf
Copy link
Owner

MrKampf commented Nov 16, 2021

The new version is 4.0

@AnushK-Fro
Copy link
Author

Hey, when I turn on debug mode I get this issue:
{message: "curl_setopt_array(): Cannot represent a stream of type Output as a STDIO FILE*",…}
exception: "ErrorException"
file: "C:\xampp\htdocs\Stratum-Next\vendor\guzzlehttp\guzzle\src\Handler\CurlFactory.php"
line: 70
message: "curl_setopt_array(): Cannot represent a stream of type Output as a STDIO FILE*"
trace: [{function: "handleError", class: "Illuminate\Foundation\Bootstrap\HandleExceptions", type: "->"},…]

@MrKampf
Copy link
Owner

MrKampf commented Nov 26, 2021

Hi,

the error seems to be caused by the library Guzzehttp. I suspect the Guzzlehttp debug / or that CURL can not debug this, so the type "STDIO FILE". This is what I get from the error message. Otherwise write a thread at https://github.com/guzzle/guzzle they might be able to help you there.

KingMotro added a commit to KingMotro/proxmoxVE that referenced this issue Dec 29, 2021
According to the issue 13 specifically on the most recent comment MrKampf#13 (comment)
This seems to solve the issue ''cannot represent a stream of type Output as a STDIO FILE*'' and as well fixing the CSRF token occuring ''must be of type string, null given''. 
When testing I stumbled upon an issue according how the params are sent, it was already patched in the delete function I added these as well to post and put.

About the debug setting,
guzzle/guzzle#1413
@KingMotro KingMotro mentioned this issue Dec 29, 2021
@MrKampf MrKampf closed this as completed Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants