all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Elias Huhsovitz" <e.huhsovitz@proxmox.com>
To: "Jonas Theisen" <j.theisen@proxmox.com>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH pve-manager v3 2/2] fix #8031: write APT proxy config on http_proxy change
Date: Fri, 02 Oct 2026 12:32:13 +0200	[thread overview]
Message-ID: <DLUA89529SLR.2F7JD85BBHQSI@proxmox.com> (raw)
In-Reply-To: <20260915091758.85522-3-j.theisen@proxmox.com>

I tested this using a the baisc HTTP Proxy docker image ubuntu/squid:

https://hub.docker.com/r/ubuntu/squid

comments inlide.

On Tue Sep 15, 2026 at 11:16 AM CEST, Jonas Theisen wrote:
> Writes the necessary file for APT if the http_proxy variable
> on the datacenter level is changed.
>
> This reuses the same function from the APT API for a regular
> database update.
>
> Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>
> ---
>  PVE/API2/Cluster.pm | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> index 4e5efbfd..efda3906 100644
> --- a/PVE/API2/Cluster.pm
> +++ b/PVE/API2/Cluster.pm
> @@ -36,6 +36,7 @@ use PVE::API2::ClusterConfig;
>  use PVE::API2::Firewall::Cluster;
>  use PVE::API2::HAConfig;
>  use PVE::API2::ReplicationConfig;
> +use PVE::API2::APT;

nit: imports are ordered Lexicographically.

So the new import should be after ACMEPlugin, like this:

use PVE::API2::ACMEPlugin;
use PVE::API2::APT;

>  
>  my $have_sdn;
>  eval {
> @@ -842,8 +843,17 @@ __PACKAGE__->register_method({
>      code => sub {
>          my ($param) = @_;
>  
> +        my $http_proxy_change = 0;
> +
>          my $delete = extract_param($param, 'delete');
>  
> +        if (
> +            defined($param->{http_proxy})
> +            || (defined($delete) && $delete =~ m/\bhttp_proxy\b/)
> +        ) {
> +            $http_proxy_change = 1;
> +        }
> +
>          cfs_lock_file(
>              'datacenter.cfg',
>              undef,
> @@ -859,6 +869,10 @@ __PACKAGE__->register_method({
>          );
>          die $@ if $@;
>  
> +        if ($http_proxy_change) {
> +            PVE::API2::APT::update_apt_proxy_config();
> +        }


Potential Pitfall
-----------------
The subroutine `update_apt_proxy_config` only writes the config to
the local node, not the cluster.

So we make a HTTP PUT request to the `/cluster/options` endpoint
expecting a cluster wide change, but only the node that accepted the
request actually updates their `/etc/apt/apt.conf.d/76pveproxy` config
file. 

I verified this behaviour on a 2 node cluster.

My take
-------
>From the top of my head 2 solutions come to mind:

1. Create a `update_apt_proxy_config_cluster()` subroutine that applies
the changes cluster wide. Then simply call this function, instead of
`update_apt_proxy_config()`

2. Create some kind of sync mechanism based on changes to the
datacenter.cfg. (also would need some kind of 
`update_apt_proxy_config_cluster()` subroutine)

This could consist of 2 functions:
* synch on change: You check diff between the previous datacenter.cfg
  and the new datacenter.cfg. 
  You apply the changes to all nodes in the cluster.

  e.g. if the `http_proxy` parameter is changed: update `76pveproxy` for
  all nodes in the cluster

* sync on demand: You go through the datacenter.cfg. For each config
  that needs additional settings on the host (e.g. `http_proxy`), apply
  the changes.

  e.g. I manually edit datacenter.cfg to set 
  `http_proxy: http://192.168.29.70:3128`

  I run the new subroutine `sync_datacenter_cfg`. All nodes in the
  cluster now contain:

   76pveproxy contents:
   Acquire::http::Proxy "http://192.168.29.70:3128";

  This function has to be idempotent.

  (Perhaps something like this already exists, I am not 100% sure atm)
 
  Let me know if this makes sense, or if you come up with a better
  solution!

> +
>          return undef;
>      },
>  });
	




  reply	other threads:[~2026-10-02 10:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15  9:16 [PATCH pve-manager v3 0/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
2026-09-15  9:16 ` [PATCH pve-manager v3 1/2] fix #8031: factor out APT proxy write from update_database call Jonas Theisen
2026-09-15  9:16 ` [PATCH pve-manager v3 2/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
2026-10-02 10:32   ` Elias Huhsovitz [this message]
2026-09-16  9:32 ` [PATCH pve-manager v3 0/2] " Maximiliano Sandoval

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=DLUA89529SLR.2F7JD85BBHQSI@proxmox.com \
    --to=e.huhsovitz@proxmox.com \
    --cc=j.theisen@proxmox.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