* [pve-devel] [PATCH v3 container 0/2] fix #3443: unique machine-id for containers @ 2021-05-27 9:25 Oguz Bektas 2021-05-27 9:26 ` [pve-devel] [PATCH v3 container 1/2] setup: clear /etc/machine-id for newly created containers Oguz Bektas 2021-05-27 9:26 ` [pve-devel] [PATCH v3 container 2/2] clear machine-id also after container clone Oguz Bektas 0 siblings, 2 replies; 6+ messages in thread From: Oguz Bektas @ 2021-05-27 9:25 UTC (permalink / raw) To: pve-devel v2->v3: * clear machine-id at the end of clone task worker Oguz Bektas (2): setup: clear /etc/machine-id for newly created containers clear machine-id also after container clone src/PVE/API2/LXC.pm | 6 ++++++ src/PVE/LXC/Setup.pm | 10 ++++++++++ src/PVE/LXC/Setup/Base.pm | 25 +++++++++++++++++++++++++ 3 files changed, 41 insertions(+) -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] [PATCH v3 container 1/2] setup: clear /etc/machine-id for newly created containers 2021-05-27 9:25 [pve-devel] [PATCH v3 container 0/2] fix #3443: unique machine-id for containers Oguz Bektas @ 2021-05-27 9:26 ` Oguz Bektas 2021-06-10 11:47 ` Fabian Grünbichler 2021-05-27 9:26 ` [pve-devel] [PATCH v3 container 2/2] clear machine-id also after container clone Oguz Bektas 1 sibling, 1 reply; 6+ messages in thread From: Oguz Bektas @ 2021-05-27 9:26 UTC (permalink / raw) To: pve-devel this way when new containers are created they will have a unique /etc/machine-id Signed-off-by: Oguz Bektas <o.bektas@proxmox.com> --- v3: no changes src/PVE/LXC/Setup.pm | 10 ++++++++++ src/PVE/LXC/Setup/Base.pm | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm index 8b8fee9..c31a164 100644 --- a/src/PVE/LXC/Setup.pm +++ b/src/PVE/LXC/Setup.pm @@ -352,6 +352,16 @@ sub pre_start_hook { $self->protected_call($code); } +sub clear_machine_id { + my ($self, $conf, $clone) = @_; + + my $code = sub { + $self->{plugin}->clear_machine_id($self->{conf}, $clone); + }; + $self->protected_call($code); + +} + sub post_create_hook { my ($self, $root_password, $ssh_keys) = @_; diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm index d73335b..21074b7 100644 --- a/src/PVE/LXC/Setup/Base.pm +++ b/src/PVE/LXC/Setup/Base.pm @@ -476,6 +476,30 @@ sub set_timezone { } } +sub clear_machine_id { + my ($self, $conf, $clone) = @_; + + my $uses_systemd = $self->ct_is_executable("/lib/systemd/systemd") + || $self->ct_is_executable("/usr/lib/systemd/systemd"); + + my $dbus_machine_id_path = "/var/lib/dbus/machine-id"; + my $machine_id_path = "/etc/machine-id"; + if ( + $self->ct_file_exists($dbus_machine_id_path) + && !$self->ct_is_symlink($dbus_machine_id_path) + && $uses_systemd + ) { + $self->ct_unlink($dbus_machine_id_path); + } + + # don't remove file if container is being cloned + if ($clone) { + $self->ct_file_set_contents($machine_id_path, "\n"); + } else { + $self->ct_unlink($machine_id_path); + } +} + sub pre_start_hook { my ($self, $conf) = @_; @@ -491,6 +515,7 @@ sub pre_start_hook { sub post_create_hook { my ($self, $conf, $root_password, $ssh_keys) = @_; + $self->clear_machine_id($conf); $self->template_fixup($conf); &$randomize_crontab($self, $conf); -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH v3 container 1/2] setup: clear /etc/machine-id for newly created containers 2021-05-27 9:26 ` [pve-devel] [PATCH v3 container 1/2] setup: clear /etc/machine-id for newly created containers Oguz Bektas @ 2021-06-10 11:47 ` Fabian Grünbichler 2021-06-10 12:04 ` Oguz Bektas 0 siblings, 1 reply; 6+ messages in thread From: Fabian Grünbichler @ 2021-06-10 11:47 UTC (permalink / raw) To: Proxmox VE development discussion On May 27, 2021 11:26 am, Oguz Bektas wrote: > this way when new containers are created they will have a unique > /etc/machine-id > > Signed-off-by: Oguz Bektas <o.bektas@proxmox.com> > --- > v3: > no changes > > > src/PVE/LXC/Setup.pm | 10 ++++++++++ > src/PVE/LXC/Setup/Base.pm | 25 +++++++++++++++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm > index 8b8fee9..c31a164 100644 > --- a/src/PVE/LXC/Setup.pm > +++ b/src/PVE/LXC/Setup.pm > @@ -352,6 +352,16 @@ sub pre_start_hook { > $self->protected_call($code); > } > > +sub clear_machine_id { > + my ($self, $conf, $clone) = @_; > + > + my $code = sub { > + $self->{plugin}->clear_machine_id($self->{conf}, $clone); > + }; > + $self->protected_call($code); > + > +} maybe it would make more sense to call this "post_clone_hook", so it is re-usable for other, similar changes (like optionally regenerating SSH keys, or ...) in the future without polluting the entry-point namespace too much? > + > sub post_create_hook { > my ($self, $root_password, $ssh_keys) = @_; > > diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm > index d73335b..21074b7 100644 > --- a/src/PVE/LXC/Setup/Base.pm > +++ b/src/PVE/LXC/Setup/Base.pm > @@ -476,6 +476,30 @@ sub set_timezone { > } > } > > +sub clear_machine_id { > + my ($self, $conf, $clone) = @_; > + > + my $uses_systemd = $self->ct_is_executable("/lib/systemd/systemd") > + || $self->ct_is_executable("/usr/lib/systemd/systemd"); > + > + my $dbus_machine_id_path = "/var/lib/dbus/machine-id"; > + my $machine_id_path = "/etc/machine-id"; > + if ( > + $self->ct_file_exists($dbus_machine_id_path) > + && !$self->ct_is_symlink($dbus_machine_id_path) > + && $uses_systemd > + ) { > + $self->ct_unlink($dbus_machine_id_path); > + } > + > + # don't remove file if container is being cloned > + if ($clone) { > + $self->ct_file_set_contents($machine_id_path, "\n"); > + } else { > + $self->ct_unlink($machine_id_path); > + } > +} > + > sub pre_start_hook { > my ($self, $conf) = @_; > > @@ -491,6 +515,7 @@ sub pre_start_hook { > sub post_create_hook { > my ($self, $conf, $root_password, $ssh_keys) = @_; > > + $self->clear_machine_id($conf); > $self->template_fixup($conf); > > &$randomize_crontab($self, $conf); > -- > 2.20.1 > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH v3 container 1/2] setup: clear /etc/machine-id for newly created containers 2021-06-10 11:47 ` Fabian Grünbichler @ 2021-06-10 12:04 ` Oguz Bektas 0 siblings, 0 replies; 6+ messages in thread From: Oguz Bektas @ 2021-06-10 12:04 UTC (permalink / raw) To: Proxmox VE development discussion hi, > +sub clear_machine_id { > > + my ($self, $conf, $clone) = @_; > > + > > + my $code = sub { > > + $self->{plugin}->clear_machine_id($self->{conf}, $clone); > > + }; > > + $self->protected_call($code); > > + > > +} > > maybe it would make more sense to call this "post_clone_hook", so it is > re-usable for other, similar changes (like optionally regenerating SSH > keys, or ...) in the future without polluting the entry-point namespace > too much? yes i was also thinking why we don't have a post_clone_hook :D i'll change that accordingly :) > > > + > > sub post_create_hook { > > my ($self, $root_password, $ssh_keys) = @_; > > > > diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm > > index d73335b..21074b7 100644 > > --- a/src/PVE/LXC/Setup/Base.pm > > +++ b/src/PVE/LXC/Setup/Base.pm > > @@ -476,6 +476,30 @@ sub set_timezone { > > } > > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] [PATCH v3 container 2/2] clear machine-id also after container clone 2021-05-27 9:25 [pve-devel] [PATCH v3 container 0/2] fix #3443: unique machine-id for containers Oguz Bektas 2021-05-27 9:26 ` [pve-devel] [PATCH v3 container 1/2] setup: clear /etc/machine-id for newly created containers Oguz Bektas @ 2021-05-27 9:26 ` Oguz Bektas 2021-06-10 11:51 ` Fabian Grünbichler 1 sibling, 1 reply; 6+ messages in thread From: Oguz Bektas @ 2021-05-27 9:26 UTC (permalink / raw) To: pve-devel pass $clone=1 to avoid removing the file. instead we truncate it to an empty file Signed-off-by: Oguz Bektas <o.bektas@proxmox.com> --- v2->v3: * clear machine-id at the end of clone task worker src/PVE/API2/LXC.pm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm index a9ea3a6..d5c12dc 100644 --- a/src/PVE/API2/LXC.pm +++ b/src/PVE/API2/LXC.pm @@ -1590,6 +1590,12 @@ __PACKAGE__->register_method({ die "clone failed: $err"; } + my $lastconf = PVE::LXC::Config->load_config($newid); + my $rootdir = PVE::LXC::mount_all($newid, $storecfg, $lastconf, 1); + my $lxc_setup = PVE::LXC::Setup->new($lastconf, $rootdir); + $lxc_setup->clear_machine_id($lastconf, 1); + PVE::LXC::umount_all($newid, $storecfg, $lastconf, 1); + return; }; -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH v3 container 2/2] clear machine-id also after container clone 2021-05-27 9:26 ` [pve-devel] [PATCH v3 container 2/2] clear machine-id also after container clone Oguz Bektas @ 2021-06-10 11:51 ` Fabian Grünbichler 0 siblings, 0 replies; 6+ messages in thread From: Fabian Grünbichler @ 2021-06-10 11:51 UTC (permalink / raw) To: Proxmox VE development discussion On May 27, 2021 11:26 am, Oguz Bektas wrote: > pass $clone=1 to avoid removing the file. instead we truncate it to an > empty file > > Signed-off-by: Oguz Bektas <o.bektas@proxmox.com> > --- > v2->v3: > * clear machine-id at the end of clone task worker > > src/PVE/API2/LXC.pm | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm > index a9ea3a6..d5c12dc 100644 > --- a/src/PVE/API2/LXC.pm > +++ b/src/PVE/API2/LXC.pm > @@ -1590,6 +1590,12 @@ __PACKAGE__->register_method({ > die "clone failed: $err"; > } > > + my $lastconf = PVE::LXC::Config->load_config($newid); > + my $rootdir = PVE::LXC::mount_all($newid, $storecfg, $lastconf, 1); > + my $lxc_setup = PVE::LXC::Setup->new($lastconf, $rootdir); > + $lxc_setup->clear_machine_id($lastconf, 1); > + PVE::LXC::umount_all($newid, $storecfg, $lastconf, 1); this is dangerous - the config is not locked anymore at this point, and we don't hold any other lock either! another unrelated thing I just noticed - in case the clone fails, we don't remove a potentially cloned firewall config.. > + > return; > }; > > -- > 2.20.1 > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-10 12:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-27 9:25 [pve-devel] [PATCH v3 container 0/2] fix #3443: unique machine-id for containers Oguz Bektas 2021-05-27 9:26 ` [pve-devel] [PATCH v3 container 1/2] setup: clear /etc/machine-id for newly created containers Oguz Bektas 2021-06-10 11:47 ` Fabian Grünbichler 2021-06-10 12:04 ` Oguz Bektas 2021-05-27 9:26 ` [pve-devel] [PATCH v3 container 2/2] clear machine-id also after container clone Oguz Bektas 2021-06-10 11:51 ` Fabian Grünbichler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox