Re: [pve-devel] [RFC manager 1/2] fix #5244 pveceph: install: add new repository for offline installation

[Aaron Lauterer <a.lauterer@proxmox.com>]

On  2025-05-02  13:00, Christoph Heiss wrote:
> Tested both patches on a fresh PVE 8.4 installation, setting up Ceph by
> manually providing a repo in /etc/apt/sources.list.d/ and then
> installing it
> - through the web interface and
> - with `pveceph -repository offline -version squid`
> 
> Tested-by: Christoph Heiss <c.heiss@proxmox.com>
> 
> On Wed Apr 23, 2025 at 3:28 PM CEST, Aaron Lauterer wrote:
>> by adding a 4th repository option called 'offline'. If set, the ceph
>> installation step will not touch the repository configuration.
>>
>> We add a simple version check to make sure that the latest version
>> available (and to be installed) does match the selected major Ceph
>> version.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>> I am not sure if we should call the new repo option "offline" or
>> something like "dont-set". Maybe someone else has another idea.
> 
> Maybe just call it 'manual'?
> 
> In patch #2, the (user-facing) description is "Offline (manual setup)" -
> IMHO "manual setup" for the repos fits pretty good for what the option
> does.

yeah sounds good, just using "manual" as parameter name. Will do so in a v2.

> 
>>
>> I am sending this as an RFC to get feedback if the approach is in the
>> right direction.
>>
>> One TODO that is missing, is to figure out which repo would be used and
>> print some metadata about it, so that the admins can verify that the
>> right manually configured repo is the source.
>>
>>   PVE/CLI/pveceph.pm | 46 +++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 33 insertions(+), 13 deletions(-)
>>
>> diff --git a/PVE/CLI/pveceph.pm b/PVE/CLI/pveceph.pm
>> index 488aea04..f5fde026 100755
>> --- a/PVE/CLI/pveceph.pm
>> +++ b/PVE/CLI/pveceph.pm
>> @@ -3,6 +3,7 @@ package PVE::CLI::pveceph;
>>   use strict;
>>   use warnings;
>>
>> +use AptPkg::Cache;
>>   use Data::Dumper;
>>   use Fcntl ':flock';
>>   use File::Path;
>> @@ -135,9 +136,12 @@ __PACKAGE__->register_method ({
>>   	    },
>>   	    repository => {
>>   		type => 'string',
>> -		enum => ['enterprise', 'no-subscription', 'test'],
>> +		enum => ['enterprise', 'no-subscription', 'test', 'offline'],
>>   		default => 'enterprise',
>> -		description => "Ceph repository to use.",
>> +		description => "Ceph repository to use. The 'offline' repository will not configure"
>> +		    ." any repositories. Use it if the host cannot access the public repositories,"
>> +		    ." for example if Proxmox Offline Mirror is used. A repository that contains"
>> +		    ." the Ceph packages for the version needs to be manually configured beforehand!",
>>   		optional => 1,
> 
> Since this parameter is optional anyway, what about only configuring
> repositories when it is set? Instead of introducing another enum value
> and defaulting to 'enterprise'.
> 
> Would seem a bit cleaner, but API breaking - so if, could only go in
> with 9.0. Do we rely on that behaviour somewhere else?

Hmm, AFAIK we only use it on the CLI directly and via the web UI console 
when installing it via the GUI.
But that would make quite the difference when installing it directly via 
the CLI pveceph install command. Therefore I would rather not make it 
optional at all so that the parameter needs to be defined, or leave it 
as is as in most situations that we anticipate, the enterprise repo 
should be used.

> 
>>   	    },
>>   	    'allow-experimental' => {
>> @@ -166,6 +170,9 @@ __PACKAGE__->register_method ({
>>   	} elsif ($repo eq 'no-subscription') {
>>   	    warn "\nHINT: The no-subscription repository is not the best choice for production setups.\n"
>>   	        ."Proxmox recommends using the enterprise repository with a valid subscription.\n";
>> +	} elsif ($repo eq 'offline') {
>> +	    warn "\nHINT: The offline repository option expects that the Ceph repository is already correctly configured."
>> +		." For example, when used in combination with Proxmox Offline Mirror.\n";
>>   	} else {
>>   	    warn "\nWARN: The test repository should only be used for test setups or after consulting"
>>   		." the official Proxmox support!\n\n"
>> @@ -186,19 +193,21 @@ __PACKAGE__->register_method ({
>>   	    die "Aborting installation as requested\n" if !$continue;
>>   	}
>>
>> -	PVE::Tools::file_set_contents("/etc/apt/sources.list.d/ceph.list", $repolist);
>> +	if ($repo ne "offline") {
>> +	    PVE::Tools::file_set_contents("/etc/apt/sources.list.d/ceph.list", $repolist);
>>
>> -	if ($available_ceph_releases->{$cephver}->{unsupported}) {
>> -	    if ($param->{'allow-experimental'}) {
>> -		warn "NOTE: installing experimental/tech-preview Ceph release ${rendered_release}!\n";
>> -	    } elsif (-t STDOUT) {
>> -		print "Ceph ${rendered_release} is currently considered a technology preview for Proxmox VE - continue (y/N)? ";
>> -		my $answer = <STDIN>;
>> -		my $continue = defined($answer) && $answer =~ m/^\s*y(?:es)?\s*$/i;
>> +	    if ($available_ceph_releases->{$cephver}->{unsupported}) {
>> +		if ($param->{'allow-experimental'}) {
>> +		    warn "NOTE: installing experimental/tech-preview Ceph release ${rendered_release}!\n";
>> +		} elsif (-t STDOUT) {
>> +		    print "Ceph ${rendered_release} is currently considered a technology preview for Proxmox VE - continue (y/N)? ";
>> +		    my $answer = <STDIN>;
>> +		    my $continue = defined($answer) && $answer =~ m/^\s*y(?:es)?\s*$/i;
>>
>> -		die "Aborting installation as requested\n" if !$continue;
>> -	    } else {
>> -		die "refusing to install tech-preview Ceph release ${rendered_release} without 'allow-experimental' parameter!\n";
>> +		    die "Aborting installation as requested\n" if !$continue;
>> +		} else {
>> +		    die "refusing to install tech-preview Ceph release ${rendered_release} without 'allow-experimental' parameter!\n";
> 
> tiny nit: All other messages here are started capitalized, so this one
> should as well for consistency.

I'll address that in a v2.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel