Re: [pve-devel] [PATCH container] lxc: fall back to 'unmanaged' on unknown ostype
On 5/18/20 9:31 AM, Arnout Engelen wrote: > On Mon, May 18, 2020 at 9:10 AM Thomas Lamprecht >> For what specific use case do you want this? What would be running in the >> CTs? >> Maybe it's easier to add support for that distro? > > My CT is not really directly based on a distro, the /sbin/init just executes > the > program I'm interested in running. I had tested this CT on another, > non-proxmox > machine with lxc/lxd where it ran fine. Adding an /etc/os-release would not > be a huge deal, but the experience would be much nicer when it'd work without > additional changes - so I'm trying to help make that happen ;). > OK, then let's go for the "unmanaged" fallback if there's no `/etc/os-release` and no other distro/version detection check hits at all. ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] lxc: fall back to 'unmanaged' on unknown ostype
On Mon, May 18, 2020 at 9:10 AM Thomas Lamprecht wrote: > On 5/17/20 11:03 PM, Arnout Engelen wrote: > > AFAICT the 'create CT' web UI does not currently allow setting the ostype. > > This of course could be changed/fixed, but the auto-detection that is > > already > > there seems nice. I haven't used the API yet, and wanted to avoid using the > > CLI (which requires root) where possible (though it looks like I'll have to > > compromise there to get access to some features anyway ;) ). > > From our POV it's a feature, we support a set of distributions which we can > all > detect in a sane way. All unsupported cannot really get made to work easily > over > the web-interface anyway. Yes, I agree detecting 'traditional' distributions in a CT and 'actively' supporting them is definitely a useful feature. > For what specific use case do you want this? What would be running in the CTs? > Maybe it's easier to add support for that distro? My CT is not really directly based on a distro, the /sbin/init just executes the program I'm interested in running. I had tested this CT on another, non-proxmox machine with lxc/lxd where it ran fine. Adding an /etc/os-release would not be a huge deal, but the experience would be much nicer when it'd work without additional changes - so I'm trying to help make that happen ;). Kind regards, Arnout ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] lxc: fall back to 'unmanaged' on unknown ostype
On 5/17/20 11:03 PM, Arnout Engelen wrote: > On Sun, May 17, 2020 at 10:44 PM Thomas Lamprecht > wrote: >> Am 5/17/20 um 10:10 AM schrieb Arnout Engelen: >>> It would make the scenario of starting an unmanaged image without explicit >>> parameters work. >> >> I'd rather add recognizing an explicit "unmanaged" type from the CTs >> `/etc/os-release` or if really only return this on setup if there's no >> os-release >> and no other releasefile, which then at least ensures that modern >> distros (which >> most ship /etc/os-release) are wrongly mapped to "unmanaged". > > Requiring an explicit "unmanaged" in `/etc/os-release` means proxmox will only > auto-detect CTs that were explicitly written for proxmox... only falling back > to > 'unmanaged' when /etc/os-release does not exist sounds fine to me, though. I mean, if you use such CTs already then they got created for a specific use case and thus adding some extra file, hint or whatever.. I mean, I do not like that approach 100%, but it would make it explicit which seems much better. > > I'll create an updated version of this patch that implements these 2 paths. > >> Fallback in update_pct_config for when no ostype is set in the config >> for existing >> CT is not OK, it must be recognizable on setup. > > Right, I was less sure about that part. > >>> When using the 'create CT' button in the web UI, PVE/LXC/Setup.pm will >>> auto-detect the ostype. >>> >>> (...) >>> >>> While I agree failing early is generally good practice, here running the >>> image 'unmanaged' when no OS was detected seems like a more >>> optimistic choice, as it fixes the 'raw unmanaged image' scenario. >> >> This is something most user won't ever need.. Why is it a problem to set the >> unmanaged OS type through CLI/API? > > AFAICT the 'create CT' web UI does not currently allow setting the ostype. > This of course could be changed/fixed, but the auto-detection that is already > there seems nice. I haven't used the API yet, and wanted to avoid using the > CLI (which requires root) where possible (though it looks like I'll have to > compromise there to get access to some features anyway ;) ). > >From our POV it's a feature, we support a set of distributions which we can all detect in a sane way. All unsupported cannot really get made to work easily over the web-interface anyway. For what specific use case do you want this? What would be running in the CTs? Maybe it's easier to add support for that distro? cheers, Thomas ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] lxc: fall back to 'unmanaged' on unknown ostype
On Sun, May 17, 2020 at 10:44 PM Thomas Lamprecht wrote: > Am 5/17/20 um 10:10 AM schrieb Arnout Engelen: > > It would make the scenario of starting an unmanaged image without explicit > > parameters work. > > I'd rather add recognizing an explicit "unmanaged" type from the CTs > `/etc/os-release` or if really only return this on setup if there's no > os-release > and no other releasefile, which then at least ensures that modern > distros (which > most ship /etc/os-release) are wrongly mapped to "unmanaged". Requiring an explicit "unmanaged" in `/etc/os-release` means proxmox will only auto-detect CTs that were explicitly written for proxmox... only falling back to 'unmanaged' when /etc/os-release does not exist sounds fine to me, though. I'll create an updated version of this patch that implements these 2 paths. > Fallback in update_pct_config for when no ostype is set in the config > for existing > CT is not OK, it must be recognizable on setup. Right, I was less sure about that part. > > When using the 'create CT' button in the web UI, PVE/LXC/Setup.pm will > > auto-detect the ostype. > > > > (...) > > > > While I agree failing early is generally good practice, here running the > > image 'unmanaged' when no OS was detected seems like a more > > optimistic choice, as it fixes the 'raw unmanaged image' scenario. > > This is something most user won't ever need.. Why is it a problem to set the > unmanaged OS type through CLI/API? AFAICT the 'create CT' web UI does not currently allow setting the ostype. This of course could be changed/fixed, but the auto-detection that is already there seems nice. I haven't used the API yet, and wanted to avoid using the CLI (which requires root) where possible (though it looks like I'll have to compromise there to get access to some features anyway ;) ). Kind regards, Arnout ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] lxc: fall back to 'unmanaged' on unknown ostype
Am 5/17/20 um 10:10 AM schrieb Arnout Engelen: On Sun, May 17, 2020 at 7:16 AM Dietmar Maurer wrote: Rather than failing, it would be nice to fall back to 'unmanaged' when the ostype cannot be determined/found. Why exactly would that be nice? FWICT it would start the container with wrong and unexpected setup. It would make the scenario of starting an unmanaged image without explicit parameters work. When using the 'create CT' button in the web UI, PVE/LXC/Setup.pm will auto-detect the ostype. When it fails to detect the ostype, like will be the case for a 'raw' image that should be run unmanaged, it dies. When instead of dying it would assume 'unmanaged', this would work for such 'raw' images. Indeed if it was an image that needs to be run 'managed' this change would make it fail later rather than earlier. First, thanks for sending out a contribution. I'd rather add recognizing an explicit "unmanaged" type from the CTs `/etc/os-release` or if really only return this on setup if there's no os-release and no other releasefile, which then at least ensures that modern distros (which most ship /etc/os-release)are wrongly mapped to "unmanaged". Fallback in update_pct_config for when no ostype is set in the config for existing CT is not OK, it must be recognizable on setup.. While I agree failing early is generally good practice, here running the image 'unmanaged' when no OS was detected seems like a more optimistic choice, as it fixes the 'raw unmanaged image' scenario. This is something most user won't ever need.. Why is it a problem to set the unmanaged OS type through CLI/API? cheers, Thomas ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] lxc: fall back to 'unmanaged' on unknown ostype
On Sun, May 17, 2020 at 7:16 AM Dietmar Maurer wrote: > > Rather than failing, it would be nice to fall back to 'unmanaged' when > > the ostype cannot be determined/found. > > Why exactly would that be nice? FWICT it would start the container > with wrong and unexpected setup. It would make the scenario of starting an unmanaged image without explicit parameters work. When using the 'create CT' button in the web UI, PVE/LXC/Setup.pm will auto-detect the ostype. When it fails to detect the ostype, like will be the case for a 'raw' image that should be run unmanaged, it dies. When instead of dying it would assume 'unmanaged', this would work for such 'raw' images. Indeed if it was an image that needs to be run 'managed' this change would make it fail later rather than earlier. While I agree failing early is generally good practice, here running the image 'unmanaged' when no OS was detected seems like a more optimistic choice, as it fixes the 'raw unmanaged image' scenario. Arnout ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] lxc: fall back to 'unmanaged' on unknown ostype
> Rather than failing, it would be nice to fall back to 'unmanaged' when > the ostype cannot be determined/found. Why exactly would that be nice? FWICT it would start the container with wrong and unexpected setup. ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH container] lxc: fall back to 'unmanaged' on unknown ostype
Rather than failing, it would be nice to fall back to 'unmanaged' when
the ostype cannot be determined/found.
Signed-off-by: Arnout Engelen
---
src/PVE/LXC.pm | 2 +-
src/PVE/LXC/Setup.pm | 6 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index f3aca7a..b32d70c 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -596,7 +596,7 @@ sub update_lxc_config {
my $custom_idmap = PVE::LXC::Config->has_lxc_entry($conf, 'lxc.idmap');
my $unprivileged = $conf->{unprivileged} || $custom_idmap;
-my $ostype = $conf->{ostype} || die "missing 'ostype' - internal error";
+my $ostype = $conf->{ostype} || 'unmanaged';
my $cfgpath = '/usr/share/lxc/config';
my $inc = "$cfgpath/$ostype.common.conf";
diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
index c738e64..010c9b7 100644
--- a/src/PVE/LXC/Setup.pm
+++ b/src/PVE/LXC/Setup.pm
@@ -71,7 +71,7 @@ my $autodetect_type = sub {
} elsif (-f "$rootdir/etc/gentoo-release") {
return "gentoo";
}
-die "unable to detect OS distribution\n";
+return "unmanaged";
};
sub new {
@@ -94,6 +94,10 @@ sub new {
if $type ne $expected_type;
}
+if ($type eq 'unmanaged') {
+return $self;
+}
+
my $plugin_class = $plugins->{$type} ||
"no such OS type '$type'\n";
--
2.26.2
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
