all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Alexandre Derumier <aderumier@odiso.com>
Subject: Re: [pve-devel] [PATCH-SERIES pve-http-server/pve-manager] fix#4689: rewrite_uri: autofind nodename for qemu/lxc
Date: Wed, 31 May 2023 15:19:12 +0200	[thread overview]
Message-ID: <ef020f53-4225-0a2a-221a-941567ead319@proxmox.com> (raw)
In-Reply-To: <20230531111952.568734-1-aderumier@odiso.com>

On 5/31/23 13:19, Alexandre Derumier wrote:
> Hi,
> 
> Currently, to manage qemu && lxc vms, we always need to specify nodename in uri.
> 
> This is a problem with automation tools like terraform, where is node is registered
> in the state of terraform.
> (That mean, than if we move the vm on another node, terraform don't known it, and try to create the vm
> again or can't delete the vm,...)
> https://github.com/Telmate/terraform-provider-proxmox/issues/168
> 
> This can also be a potential problem with race, if we need to query /cluster/ressources to find the node, then another
> query on the vm.
> 
> I have some discussion with fabian about it:
> https://bugzilla.proxmox.com/show_bug.cgi?id=4689
> 
> 
> This patch series, simply find the nodename if not specified and rewrite url.
> 
> /nodes/(qemu|lxc)/<vmid> ---> /nodes/<nodename>/<qemu|lxc>/<vmid>
> 
> 
> 
> Alexandre Derumier (1):
>    anyevent: add rewrite_uri
> 
>   src/PVE/APIServer/AnyEvent.pm | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> Alexandre Derumier (1):
>    httpserver: add rewrite uri for /nodes/(qemu/lxc)/<vmid>
> 
>   PVE/HTTPServer.pm | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 

hi, while this can work, it introduces a very specific rewriting into the
http server, which might not be obvious and AFAICT won't show up in the
auto-generated api docs?

wouldn't it be better to have some api calls defined for real
(the ones used most often for example) that make use of the
proxyto_callback method instead?

that way they would at least show up in the api documentation,
even though it's probably a bit of code duplication
(altough we could maybe just insert the api at a different place
and do some perl magic refactoring to have it once
with node parameter and once without?)

or does that seem like a bad idea?




  parent reply	other threads:[~2023-05-31 13:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-31 11:19 Alexandre Derumier
2023-05-31 11:19 ` [pve-devel] [PATCH pve-http-server 1/1] anyevent: add rewrite_uri Alexandre Derumier
2023-05-31 11:19 ` [pve-devel] [PATCH pve-manager 1/1] httpserver: add rewrite uri for /nodes/(qemu/lxc)/<vmid> Alexandre Derumier
2023-05-31 11:49 ` [pve-devel] [PATCH-SERIES pve-http-server/pve-manager] fix#4689: rewrite_uri: autofind nodename for qemu/lxc Wolfgang Bumiller
2023-05-31 14:12   ` DERUMIER, Alexandre
2023-05-31 13:19 ` Dominik Csapak [this message]
2023-05-31 14:05   ` DERUMIER, Alexandre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ef020f53-4225-0a2a-221a-941567ead319@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=aderumier@odiso.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal