[pve-devel] pve-network : downgrading frr dependency to suggested

[pve-devel] pve-network : downgrading frr dependency to suggested

[Thomas Lamprecht]

Hi,

I'd downgrade frr-pythontools to a suggestion, as Debian has install-recommends
on by default, so an upgrade would pull FRR always in, if we add a dependency
for our SDN package to e.g. pve-manager, and FRR daemon is quite noisy IIRC.

Would that be fine for you? A short documentation how to install it, or dnsmasq
for getting the DHCP feature, could be added.

For the code it would be nice to add a helper that asserts that frr is installed
for all API endpoints that need it, that way user get informed upfront, similar
to how we do it for ceph (but no full web installer for now ^^)

Re: [pve-devel] pve-network : downgrading frr dependency to suggested

[DERUMIER, Alexandre]

-------- Message initial --------
De: Thomas Lamprecht <t.lamprecht@proxmox.com>
À: PVE development discussion <pve-devel@pve.proxmox.com>, Alexandre
Derumier <Alexandre.DERUMIER@groupe-cyllene.com>
Objet: pve-network : downgrading frr dependency to suggested
Date: 20/11/2023 17:55:40

>>Hi,

>>I'd downgrade frr-pythontools to a suggestion, as Debian has install-
>>recommends
>>on by default, so an upgrade would pull FRR always in, if we add a
>>dependency
>>for our SDN package to e.g. pve-manager, and FRR daemon is quite
noisy >>IIRC.

>>Would that be fine for you? A short documentation how to install it,
or >>dnsmasq
>>for getting the DHCP feature, could be added.


It's needed for transparent reload (usr/lib/frr/frr-reload.py).

so you can be 100% sure It'll break if it's not installed ^_^



>>For the code it would be nice to add a helper that asserts that frr
>>is installed
>>for all API endpoints that need it, that way user get informed
>>upfront, similar
>>to how we do it for ceph (but no full web installer for now ^^)

Yes good idea. 

I don't known if it's easy to check, as the frr is local to each node,
and the sdn api is global at datacenter level. ?

Re: [pve-devel] pve-network : downgrading frr dependency to suggested

[Thomas Lamprecht]

Am 20/11/2023 um 18:20 schrieb DERUMIER, Alexandre:
> It's needed for transparent reload (usr/lib/frr/frr-reload.py).
> 
> so you can be 100% sure It'll break if it's not installed ^_^

Ah, we already got a check in EVPN's reload_controller:

    my $bin_path = "/usr/lib/frr/frr-reload.py";

    if (!-e $bin_path) {
        warn "missing $bin_path. Please install frr-pythontools package";
        return;
    }


> 
>>> For the code it would be nice to add a helper that asserts that frr
>>> is installed
>>> for all API endpoints that need it, that way user get informed
>>> upfront, similar
>>> to how we do it for ceph (but no full web installer for now ^^)
> 
> Yes good idea. 
> 
> I don't known if it's easy to check, as the frr is local to each node,
> and the sdn api is global at datacenter level. ?

hmm true, one would only see if the current node one is connect
through supports it, a warning (or error) on apply could be better
then, should ideally happen as early as possible though, to not roll
out a partial state.

Re: [pve-devel] pve-network : downgrading frr dependency to suggested

[DERUMIER, Alexandre]

> 
> > > For the code it would be nice to add a helper that asserts that
> > > frr
> > > is installed
> > > for all API endpoints that need it, that way user get informed
> > > upfront, similar
> > > to how we do it for ceph (but no full web installer for now ^^)
> 
> Yes good idea. 
> 
> I don't known if it's easy to check, as the frr is local to each
> node,
> and the sdn api is global at datacenter level. ?

>>hmm true, one would only see if the current node one is connect
>>through supports it, a warning (or error) on apply could be better
>>then, should ideally happen as early as possible though, to not roll
>>out a partial state.

I think that we could broadcast the state of the service with pvestatd,

like for ceph services ?

Re: [pve-devel] pve-network : downgrading frr dependency to suggested

[Thomas Lamprecht]

Am 21/11/2023 um 09:54 schrieb DERUMIER, Alexandre:
>>
>>>> For the code it would be nice to add a helper that asserts that
>>>> frr
>>>> is installed
>>>> for all API endpoints that need it, that way user get informed
>>>> upfront, similar
>>>> to how we do it for ceph (but no full web installer for now ^^)
>>
>> Yes good idea. 
>>
>> I don't known if it's easy to check, as the frr is local to each
>> node,
>> and the sdn api is global at datacenter level. ?
> 
>>> hmm true, one would only see if the current node one is connect
>>> through supports it, a warning (or error) on apply could be better
>>> then, should ideally happen as early as possible though, to not roll
>>> out a partial state.
> 
> I think that we could broadcast the state of the service with pvestatd,
> 
> like for ceph services ?
> 

Yes, but IMO it'd be better to rework that into a more general mechanism
that allows to check if a feature is supported supported. We could also
hook into apt/dpkg there and only update this state on actual changes
to the installed packages.

But IMO something for the future, for now warning on apply is fine enough
IMO, if we switch those warnings to using PVE::RESTEnvironment::log_warn
the task will also be marked orange in the UI task list, so it would be
quite visible for the admin.