[PATCH docs/manager 0/7] interface-pinning: add rename subcommand and man(1) page

[PATCH docs/manager 0/7] interface-pinning: add rename subcommand and man(1) page

[Christoph Heiss]

It came up a few times in the forum already to be able to rename
already-pinned interfaces, which is sensible.

While at it, also add a full man(1) page for
pve-network-interface-pinning. Currently, it's just a hardcoded stub
without any actual help text.

pve-docs:

Christoph Heiss (4):
  network: fix typo occurences -> occurrences
  pve-doc-generator: handle CLI tools/services with dashes in their name
  pve-network-interface-pinning: init man(1) page
  network: pinning: mention how to rename an interface

 ...-network-interface-pinning.1-synopsis.adoc | 50 ++++++++++++++++
 pve-admin-guide.adoc                          |  8 +++
 pve-doc-generator.mk.in                       | 12 ++--
 pve-network-interface-pinning.adoc            | 60 +++++++++++++++++++
 pve-network.adoc                              |  8 ++-
 5 files changed, 131 insertions(+), 7 deletions(-)
 create mode 100644 generated/pve-network-interface-pinning.1-synopsis.adoc
 create mode 100644 pve-network-interface-pinning.adoc

pve-manager:

Christoph Heiss (3):
  network-interface-pinning: avoid warning if `bond-primary` is unset
  network-interface-pinning: add method for renaming existing pins
  network-interface-pinning: generate proper manpage from docs

 PVE/CLI/pve_network_interface_pinning.pm | 156 +++++++++++++++++------
 bin/Makefile                             |  19 ---
 2 files changed, 119 insertions(+), 56 deletions(-)
-- 

[PATCH docs 1/7] network: fix typo occurences -> occurrences

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 pve-network.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pve-network.adoc b/pve-network.adoc
index 03524e4..2d5f9b1 100644
--- a/pve-network.adoc
+++ b/pve-network.adoc
@@ -163,7 +163,7 @@ Using the pve-network-interface-pinning Tool
 
 Proxmox VE provides a tool for automatically generating .link files for
 overriding the name of network devices. It also automatically replaces the
-occurences of the old interface name in the following files:
+occurrences of the old interface name in the following files:
 
 * `/etc/network/interfaces`
 * `/etc/pve/nodes/<nodename>/host.fw`
-- 
2.54.0

[PATCH docs 2/7] pve-doc-generator: handle CLI tools/services with dashes in their name

[Christoph Heiss]

.. by replacing dashes with underscores.

Exact same as the rules for *.{1,8}-synopsis.adoc files above.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 pve-doc-generator.mk.in | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/pve-doc-generator.mk.in b/pve-doc-generator.mk.in
index 64b6398..deb848b 100644
--- a/pve-doc-generator.mk.in
+++ b/pve-doc-generator.mk.in
@@ -56,27 +56,27 @@ ifneq (${DGDIR},.)
 endif
 
 %.service-bash-completion:
-	perl ${PERL_DOC_INC} -T -e "use PVE::Service::$*; PVE::Service::$*->generate_bash_completions();" >$@.tmp
+	perl ${PERL_DOC_INC} -T -e "use PVE::Service::$(subst -,_,$*); PVE::Service::$(subst -,_,$*)->generate_bash_completions();" >$@.tmp
 	mv $@.tmp $@
 
 %.bash-completion:
-	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::$*; PVE::CLI::$*->generate_bash_completions();" >$@.tmp
+	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::$(subst -,_,$*); PVE::CLI::$(subst -,_,$*)->generate_bash_completions();" >$@.tmp
 	mv $@.tmp $@
 
 %.service-zsh-completion:
-	perl ${PERL_DOC_INC} -T -e "use PVE::Service::$*; PVE::Service::$*->generate_zsh_completions();" >$@.tmp
+	perl ${PERL_DOC_INC} -T -e "use PVE::Service::$(subst -,_,$*); PVE::Service::$(subst -,_,$*)->generate_zsh_completions();" >$@.tmp
 	mv $@.tmp $@
 
 %.zsh-completion:
-	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::$*; PVE::CLI::$*->generate_zsh_completions();" >$@.tmp
+	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::$(subst -,_,$*); PVE::CLI::$(subst -,_,$*)->generate_zsh_completions();" >$@.tmp
 	mv $@.tmp $@
 
 %.service-api-verified:
-	perl ${PERL_DOC_INC} -T -e "use PVE::Service::$*; PVE::Service::$*->verify_api();"
+	perl ${PERL_DOC_INC} -T -e "use PVE::Service::$(subst -,_,$*); PVE::Service::$(subst -,_,$*)->verify_api();"
 	touch $@
 
 %.api-verified:
-	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::$*; PVE::CLI::$*->verify_api();"
+	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::$(subst -,_,$*); PVE::CLI::$(subst -,_,$*)->verify_api();"
 	touch $@
 
 %-plain.html: %.adoc ${PVE_COMMON_DOC_SOURCES}
-- 
2.54.0

[PATCH docs 3/7] pve-network-interface-pinning: init man(1) page

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
FWIW; this already includes the help text for the new `rename`
subcommand introduced in patch #6.

 ...-network-interface-pinning.1-synopsis.adoc | 50 ++++++++++++++++
 pve-admin-guide.adoc                          |  8 +++
 pve-network-interface-pinning.adoc            | 60 +++++++++++++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 generated/pve-network-interface-pinning.1-synopsis.adoc
 create mode 100644 pve-network-interface-pinning.adoc

diff --git a/generated/pve-network-interface-pinning.1-synopsis.adoc b/generated/pve-network-interface-pinning.1-synopsis.adoc
new file mode 100644
index 0000000..f8eb18d
--- /dev/null
+++ b/generated/pve-network-interface-pinning.1-synopsis.adoc
@@ -0,0 +1,50 @@
+[[cli_pve-network-interface-pinning]]
+*pve-network-interface-pinning* `<COMMAND> [ARGS] [OPTIONS]`
+
+[[cli_pve-network-interface-pinning_generate]]
+*pve-network-interface-pinning generate* `[OPTIONS]`
+
+Generate systemd.link files to pin the names of one or more network
+interfaces and update all network-related configuration files.
+
+`--interface` `<string>` ('default =' `<all>`)::
+
+Only pin a specific interface.
+
+`--prefix` `^[a-zA-Z][a-zA-Z0-9-_]{0,7}$` ('default =' `nic`)::
+
+Use a specific prefix for automatically choosing the pinned name.
+
+`--target-name` `<string>` ::
+
+Pin the interface to a specific name.
++
+NOTE: Requires option(s): `interface`
+
+[[cli_pve-network-interface-pinning_help]]
+*pve-network-interface-pinning help* `[OPTIONS]`
+
+Get help about specified command.
+
+`--extra-args` `<array>` ::
+
+Shows help for a specific command
+
+`--verbose` `<boolean>` ::
+
+Verbose output format.
+
+[[cli_pve-network-interface-pinning_rename]]
+*pve-network-interface-pinning rename* `<interface> <target-name>`
+
+Rename an existing pin for a network interface to a specific name.
+
+`<interface>`: `<string>` ::
+
+The interface to rename.
+
+`<target-name>`: `<string>` ::
+
+The new pinned interface name.
+
+
diff --git a/pve-admin-guide.adoc b/pve-admin-guide.adoc
index fd37b1e..c086377 100644
--- a/pve-admin-guide.adoc
+++ b/pve-admin-guide.adoc
@@ -226,6 +226,14 @@ include::generated/ha-manager.1-synopsis.adoc[]
 
 :leveloffset: 0
 
+*pve-network-interface-pinning* - {pve} Network Interface Pinning Tool
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+:leveloffset: 1
+include::generated/pve-network-interface-pinning.1-synopsis.adoc[]
+
+:leveloffset: 0
+
 
 [appendix]
 Service Daemons
diff --git a/pve-network-interface-pinning.adoc b/pve-network-interface-pinning.adoc
new file mode 100644
index 0000000..79d6aeb
--- /dev/null
+++ b/pve-network-interface-pinning.adoc
@@ -0,0 +1,60 @@
+ifdef::manvolnum[]
+pve-network-interface-pinning(1)
+================================
+:pve-toplevel:
+
+NAME
+----
+
+pve-network-interface-pinning - Proxmox VE Network Interface Pinning Tool
+
+SYNOPSIS
+--------
+
+include::generated/pve-network-interface-pinning.1-synopsis.adoc[]
+
+DESCRIPTION
+-----------
+endif::manvolnum[]
+
+ifndef::manvolnum[]
+Overriding Network Device Names
+===============================
+endif::manvolnum[]
+
+Command line tool to generate and rename pinned network interface names.
+
+ifdef::manvolnum[]
+
+In order to apply the changes made by `pve-network-interface-pinning`, the node
+needs to be rebooted.
+
+EXAMPLES
+--------
+
+To generate .link for all present physical interfaces:
+
+----
+pve-network-interface-pinning generate
+----
+
+As above, but with a custom prefix instead of `nic`:
+
+----
+pve-network-interface-pinning generate --prefix myprefix
+----
+
+Or just pin a specific network interface to a chosen name:
+
+----
+pve-network-interface-pinning generate --interface enp1s0 --target-name mynic
+----
+
+To rename an already pinned interface, e.g. `nic0` to `fiber10g0`:
+
+----
+pve-network-interface-pinning rename nic0 fiber10g0
+----
+
+include::pve-copyright.adoc[]
+endif::manvolnum[]
-- 
2.54.0

[PATCH docs 4/7] network: pinning: mention how to rename an interface

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 pve-network.adoc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/pve-network.adoc b/pve-network.adoc
index 2d5f9b1..c7bacc3 100644
--- a/pve-network.adoc
+++ b/pve-network.adoc
@@ -215,6 +215,12 @@ interface should be pinned to:
 pve-network-interface-pinning generate --interface enp1s0 --target-name if42
 ----
 
+You can also rename an already pinned interface, e.g. `nic0` to `fiber10g0`:
+
+----
+pve-network-interface-pinning rename nic0 fiber10g0
+----
+
 In order to apply the changes made by `pve-network-interface-pinning` to the
 network configuration, the node needs to be rebooted.
 
-- 
2.54.0

[PATCH manager 5/7] network-interface-pinning: avoid warning if `bond-primary` is unset

[Christoph Heiss]

Results in warnings such as

Use of uninitialized value $iface_name in pattern match (m//) at ..
Use of uninitialized value $iface_name in hash element at ..

`bond-primary` is an optional property and thus may be unset.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 PVE/CLI/pve_network_interface_pinning.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/CLI/pve_network_interface_pinning.pm b/PVE/CLI/pve_network_interface_pinning.pm
index 9dff181dc..19e358cd9 100644
--- a/PVE/CLI/pve_network_interface_pinning.pm
+++ b/PVE/CLI/pve_network_interface_pinning.pm
@@ -95,7 +95,8 @@ my sub update_etc_network_interfaces {
                 $iface_name = $mapping->name($iface_name);
                 $iface->{'vlan-raw-device'} = $mapping->name($iface->{'vlan-raw-device'});
             } elsif ($iface->{type} eq 'bond') {
-                $iface->{'bond-primary'} = $mapping->name($iface->{'bond-primary'});
+                $iface->{'bond-primary'} = $mapping->name($iface->{'bond-primary'})
+                    if defined($iface->{'bond-primary'});
                 $iface->{slaves} = $mapping->list($iface->{slaves});
             } elsif ($iface->{type} eq 'bridge') {
                 $iface->{bridge_ports} = $mapping->list($iface->{bridge_ports});
-- 
2.54.0

[PATCH manager 6/7] network-interface-pinning: add method for renaming existing pins

[Christoph Heiss]

Enables easily renaming of already pinned network interfaces, e.g. for
further customization later on.

It essentially runs the normal "generate" step, then removes the old
link file.

Can be invoked as e.g.:

  pve-network-interface-pinning rename nic1 port1

Came up in the forum in the past [0][1].

[0] https://forum.proxmox.com/threads/renaming-of-pinned-nic-names.184257
[1] https://forum.proxmox.com/threads/edit-pinned-network-interface-names.182305

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Shortly talked to Stefan about this; seemed like a small but worthwhile
improvement to our tooling.

Tangentially also addresses [2], since that also asks for
recreating/renaming.

[2] https://bugzilla.proxmox.com/show_bug.cgi?id=6928

 PVE/CLI/pve_network_interface_pinning.pm | 153 +++++++++++++++++------
 1 file changed, 117 insertions(+), 36 deletions(-)

diff --git a/PVE/CLI/pve_network_interface_pinning.pm b/PVE/CLI/pve_network_interface_pinning.pm
index 19e358cd9..72515b164 100644
--- a/PVE/CLI/pve_network_interface_pinning.pm
+++ b/PVE/CLI/pve_network_interface_pinning.pm
@@ -364,6 +364,47 @@ sub resolve_pinned {
     return $resolved;
 }
 
+sub setup_mappings {
+    my ($ip_links, $existing_pins, $mapping) = @_;
+
+    if (!$mapping->%*) {
+        print "Nothing to do, aborting.\n";
+        exit 0;
+    }
+
+    my $altnames = PVE::Network::altname_mapping($ip_links);
+
+    my @sorted_links = sort {
+        my $a_name = $altnames->{$a} // $a;
+        my $b_name = $altnames->{$b} // $b;
+
+        $ip_links->{$a_name}->{ifindex} <=> $ip_links->{$b_name}->{ifindex};
+        } grep {
+            $ip_links->{$_}
+        } keys $mapping->%*;
+
+    for my $old_name (@sorted_links) {
+        my $altname_string = '';
+
+        if (my $interface_altnames = $ip_links->{$old_name}->{altnames}) {
+            $altname_string = join(', ', $interface_altnames->@*);
+        }
+
+        print "Name for link '$old_name' ";
+        print "($altname_string) " if $altname_string;
+        print "will change to '$mapping->{$old_name}'\n";
+
+    }
+
+    generate_link_files($ip_links, $mapping);
+    print "Successfully generated .link files in '/usr/local/lib/systemd/network/'\n";
+
+    update_host_fw_config($mapping);
+    update_etc_network_interfaces($mapping, $existing_pins);
+    update_sdn_controllers($mapping);
+    update_sdn_fabrics($mapping);
+}
+
 __PACKAGE__->register_method({
     name => 'generate',
     path => 'generate',
@@ -453,42 +494,7 @@ __PACKAGE__->register_method({
                 );
             }
 
-            if (!$mapping->%*) {
-                print "Nothing to do, aborting.\n";
-                exit 0;
-            }
-
-            my $altnames = PVE::Network::altname_mapping($ip_links);
-
-            my @sorted_links = sort {
-                my $a_name = $altnames->{$a} // $a;
-                my $b_name = $altnames->{$b} // $b;
-
-                $ip_links->{$a_name}->{ifindex} <=> $ip_links->{$b_name}->{ifindex};
-                } grep {
-                    $ip_links->{$_}
-                } keys $mapping->%*;
-
-            for my $old_name (@sorted_links) {
-                my $altname_string = '';
-
-                if (my $interface_altnames = $ip_links->{$old_name}->{altnames}) {
-                    $altname_string = join(', ', $interface_altnames->@*);
-                }
-
-                print "Name for link '$old_name' ";
-                print "($altname_string) " if $altname_string;
-                print "will change to '$mapping->{$old_name}'\n";
-
-            }
-
-            generate_link_files($ip_links, $mapping);
-            print "Successfully generated .link files in '/usr/local/lib/systemd/network/'\n";
-
-            update_host_fw_config($mapping);
-            update_etc_network_interfaces($mapping, $existing_pins);
-            update_sdn_controllers($mapping);
-            update_sdn_fabrics($mapping);
+            setup_mappings($ip_links, $existing_pins, $mapping);
 
             print "Successfully updated Proxmox VE configuration files.\n";
             print "\nPlease reboot to apply the changes to your configuration\n\n";
@@ -501,8 +507,83 @@ __PACKAGE__->register_method({
     },
 });
 
+__PACKAGE__->register_method({
+    name => 'rename',
+    path => 'rename',
+    method => 'POST',
+    description => 'Rename an existing pin for a network interface to a specific name.',
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            interface => {
+                description => 'The interface to rename.',
+                type => 'string',
+                format => 'pve-iface',
+            },
+            'target-name' => {
+                description => 'The new pinned interface name.',
+                type => 'string',
+                format => 'pve-iface',
+            },
+        },
+    },
+    returns => {
+        type => 'null',
+    },
+    code => sub {
+        my ($params) = @_;
+
+        my $iface = $params->{interface};
+        my $target_name = $params->{'target-name'};
+
+        if (-t STDOUT) {
+            say
+                "This will rename the existing pin '$iface' to '$target_name' - continue (y/N)? ";
+
+            my $answer = <STDIN>;
+            my $continue = defined($answer) && $answer =~ m/^\s*y(?:es)?\s*$/i;
+
+            die "Aborting renaming as requested\n" if !$continue;
+        }
+
+        my $code = sub {
+            my $ip_links = get_ip_links();
+            my $pinned = get_pinned();
+            my $existing_pins = resolve_pinned($ip_links, $pinned);
+
+            die "Could not find existing link with name '$iface'!\n" if !$ip_links->{$iface};
+
+            if (!defined($existing_pins->{$iface})) {
+                die "No existing pin found for NIC '$iface' - aborting.\n"
+                    . "Use the 'generate' subcommand for first time setup.\n";
+            }
+
+            die "target-name '$target_name' already exists as link or pin!\n"
+                if $ip_links->{$target_name} || grep { $target_name eq $_ } values $pinned->%*;
+
+            my $mapping = PVE::CLI::pve_network_interface_pinning::InterfaceMapping->new({
+                $iface => $target_name,
+            });
+
+            setup_mappings($ip_links, $existing_pins, $mapping);
+
+            delete_link_files({ $target_name => $iface });
+            print "Removed old .link file for '$iface'.\n";
+
+            print "Successfully updated Proxmox VE configuration files.\n";
+            print "\nPlease reboot to apply the changes to your configuration.\n\n";
+        };
+
+        PVE::Tools::lock_file($PVEETH_LOCK, 10, $code);
+        die $@ if $@;
+
+        return;
+    },
+});
+
 our $cmddef = {
     generate => [__PACKAGE__, 'generate', [], {}],
+    rename => [__PACKAGE__, 'rename', ['interface', 'target-name'], {}],
 };
 
 1;
-- 
2.54.0

[PATCH manager 7/7] network-interface-pinning: generate proper manpage from docs

[Christoph Heiss]

Now that pve-docs has a dedicated man page set up for
pve-network-interface-pinning, let the build process generate it from
there.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Requires a bumped pve-doc-generator with

  pve-doc-generator: handle CLI tools/services with dashes in their name

applied, so that pve-doc-generator correctly import the CLI module.

 bin/Makefile | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/bin/Makefile b/bin/Makefile
index 5d225d179..d04e4fefa 100644
--- a/bin/Makefile
+++ b/bin/Makefile
@@ -69,25 +69,6 @@ all: $(SERVICE_MANS) $(CLI_MANS)
 	podselect $* > $@.tmp
 	mv $@.tmp $@
 
-pve-network-interface-pinning.1:
-	printf "pve-network-interface-pinning" > $@.tmp
-	printf ".TH PVE-NETWORK-INTERFACE-PINNING 1\n.SH NAME\npve-network-interface-pinning \- Assisted interface name pinning.\n" > $@.tmp
-	printf ".SH DESCRIPTION\nThis tool helps you to configure a network interface name pinning \
-	 and adapting all relevant Proxmox VE config files.\n" >> $@.tmp
-	mv $@.tmp $@
-
-pve-network-interface-pinning.api-verified:
-	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::pve_network_interface_pinning; PVE::CLI::pve_network_interface_pinning->verify_api();"
-	touch 'pve-network-interface-pinning.service-api-verified'
-
-pve-network-interface-pinning.zsh-completion:
-	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::pve_network_interface_pinning; PVE::CLI::pve_network_interface_pinning->generate_zsh_completions();" >$@.tmp
-	mv $@.tmp $@
-
-pve-network-interface-pinning.bash-completion:
-	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::pve_network_interface_pinning; PVE::CLI::pve_network_interface_pinning->generate_bash_completions();" >$@.tmp
-	mv $@.tmp $@
-
 pve8to9.1:
 	printf ".TH PVE8TO9 1\n.SH NAME\npve8to9 \- Proxmox VE upgrade checker script for 8.4+ to current 9.x\n" > $@.tmp
 	printf ".SH DESCRIPTION\nThis tool will help you to detect common pitfalls and misconfguration\
-- 
2.54.0