Improve interface detection logic in cloudinit script #1863

arbianshkodra · 2025-08-06T13:32:39Z

Fix network interface detection for various naming patterns

Problem

The network interface rename script fails to detect interfaces with certain naming patterns (e.g., enp7s0), causing k3s services to fail with "unable to find interface eth1" errors. This prevents nodes from joining the cluster properly.

Solution

Optimized the interface detection logic based on review feedback:

Removed redundant check: The pattern /enp[0-9]+s[0-9]:/ was a subset of /enp[0-9]+s[0-9]+:/, making the second check unreachable
Improved efficiency: Store ip link show output once in a variable to avoid multiple command executions
Simplified logic: Now uses only two checks - first tries the specific enp[0-9]+s[0-9]+ pattern (handles both enp1s0 and enp7s0), then falls back to
original detection logic if no match

Performance Impact

Reduces system calls from 3 potential ip link show executions to just 1, improving script efficiency.

Testing

Tested on Hetzner Cloud servers with both enp7s0 and enp1s0 interface naming patterns. All nodes successfully renamed their interfaces to eth1 and joined
the cluster.

Fixes: Network interface detection failure on nodes with enp*s0 naming pattern
Improves: Script efficiency by eliminating redundant checks and reducing system calls

gemini-code-assist

Summary of Changes

Hello @arbianshkodra, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the network interface detection within a cloud-init script to make it more robust and adaptable to modern Linux naming conventions. The changes aim to improve the reliability of identifying the correct network interface for configuration, while maintaining backward compatibility through a layered detection approach.

Highlights

Enhanced Network Interface Detection: The cloud-init script's logic for identifying the primary network interface has been significantly improved. It now prioritizes detecting interfaces following the enp naming convention (e.g., enp7s0, enp1s0), which are common in modern Linux distributions using predictable network interface naming.
Robust Fallback Mechanism: A multi-stage detection process has been implemented. If an enp interface is not found using the primary patterns, the script now includes a fallback to the original, more generic ip link show parsing logic, ensuring broader compatibility across various system configurations.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request aims to improve the network interface detection logic within a cloud-init script, making it more robust by searching for predictable interface names like enp*s* before reverting to the previous, more brittle index-based method.

My review identifies a logical issue in the new implementation where a redundant check creates unreachable code. Additionally, I've pointed out an opportunity to improve efficiency by avoiding multiple executions of the same command pipeline. I've provided a refactored code snippet that addresses both of these points, resulting in a cleaner, more correct, and more performant script.

gemini-code-assist · 2025-08-06T13:34:57Z

locals.tf

+      INTERFACE=$(ip link show | grep -v 'flannel' | awk '/enp[0-9]+s[0-9]+:/{print $2; exit}' | sed 's/://g')
+      # If no enp*s* interface found, try simpler enp* pattern (like enp7s0)
+      if [ -z "$INTERFACE" ]; then
+        INTERFACE=$(ip link show | grep -v 'flannel' | awk '/enp[0-9]+s[0-9]:/{print $2; exit}' | sed 's/://g')
+      fi
+      # If still no enp* interface found, use original logic as fallback
+      if [ -z "$INTERFACE" ]; then
+        INTERFACE=$(ip link show | grep -v 'flannel' | awk 'BEGIN{l3=""}; /^3:/{l3=$2}; /^2:/{l2=$2}; END{if(l3!="") print l3; else print l2}' | sed 's/://g')
+      fi


The logic for finding the interface can be simplified and made more efficient.

Redundant Check: The if block on lines 985-987 is redundant. The awk pattern /enp[0-9]+s[0-9]:/ is a subset of the pattern /enp[0-9]+s[0-9]+:/ used on line 983. If the first check fails to find a match and $INTERFACE is empty, the second one is guaranteed to fail as well, making this block unreachable code.

Efficiency: The command ip link show | grep -v 'flannel' is executed up to three times. This can be optimized by running it once and storing the output in a variable for reuse.

I suggest refactoring this block to address these points. This will make the script cleaner and more efficient.

# Find the private network interface by name, falling back to original logic. # The output of 'ip link show' is stored to avoid multiple calls. IP_LINK_NO_FLANNEL=$(ip link show | grep -v 'flannel') # Try to find an interface with a predictable name, e.g., enp1s0 INTERFACE=$(echo "$IP_LINK_NO_FLANNEL" | awk '/enp[0-9]+s[0-9]+:/{print $2; exit}' | sed 's/://g') # If no predictable name is found, use original logic as fallback if [ -z "$INTERFACE" ]; then INTERFACE=$(echo "$IP_LINK_NO_FLANNEL" | awk 'BEGIN{l3=""}; /^3:/{l3=$2}; /^2:/{l2=$2}; END{if(l3!="") print l3; else print l2}' | sed 's/://g') fi

mysticaltech · 2025-08-06T14:17:22Z

@arbianshkodra Thank you for this, super important. Please do address the review feedback and review again please with /(no-space-here)gemini review

…larity and efficiency

gemini-code-assist

Code Review

This pull request improves the network interface detection logic in the cloudinit script to handle more naming patterns. The change is good, but there's a discrepancy between the implementation and the pull request description regarding the detection logic. I've suggested a more robust and simplified implementation for the interface detection command that also addresses this discrepancy.

gemini-code-assist · 2025-08-06T14:29:13Z

locals.tf

+      # Try to find an interface with a predictable name, e.g., enp1s0
+      INTERFACE=$(echo "$IP_LINK_NO_FLANNEL" | awk '/enp[0-9]+s[0-9]+:/{print $2; exit}' | sed 's/://g')
+
+      # If no predictable name is found, use original logic as fallback
+      if [ -z "$INTERFACE" ]; then
+        INTERFACE=$(echo "$IP_LINK_NO_FLANNEL" | awk 'BEGIN{l3=""}; /^3:/{l3=$2}; /^2:/{l2=$2}; END{if(l3!="") print l3; else print l2}' | sed 's/://g')
+      fi


The implementation doesn't fully match the logic described in the pull request description. The description mentions trying two different patterns sequentially, but the code only checks for one (enp[0-9]+s[0-9]+).

Additionally, the order of checks in the description is logically flawed, as enp[0-9]+s[0-9] is a subset of enp[0-9]+s[0-9]+. Checking for the broader pattern first would make the second check redundant.

To make the logic more robust and align with the likely intent of supporting various enp*s* patterns, you could simplify the command. This also makes the code more efficient by removing the sed command.

Here is a suggested improvement:

# Try to find an interface with a predictable name, e.g., enp1s0 or enp7s0 INTERFACE=$(echo "$IP_LINK_NO_FLANNEL" | awk '/enp[0-9]+s[0-9]+:/ {sub(/:$/, "", $2); print $2; exit}') # If no predictable name is found, use original logic as fallback if [ -z "$INTERFACE" ]; then INTERFACE=$(echo "$IP_LINK_NO_FLANNEL" | awk 'BEGIN{l3=""}; /^3:/{l3=$2}; /^2:/{l2=$2}; END{if(l3!="") print l3; else print l2}' | sed 's/://g') fi

Forgot to update the description for this. I think the implementation is properly now!

arbianshkodra · 2025-08-06T14:31:58Z

/gemini review

gemini-code-assist

Code Review

This pull request improves the network interface detection logic in the cloudinit script. The changes are a good improvement, making the script more robust and efficient by caching the ip link show output and adding a detection method for enp*s* style interface names. I've added a few suggestions to further improve shell script efficiency and add a check for when no interface can be found to prevent the script from failing with an unclear error.

gemini-code-assist · 2025-08-06T14:33:39Z

locals.tf

+      fi
      MAC=$(cat /sys/class/net/$INTERFACE/address)


If both interface detection methods fail, $INTERFACE could be empty. This would cause cat /sys/class/net/$INTERFACE/address to fail with a cryptic error because of set -e. It's better to explicitly check if an interface was found and exit with a clear error message if not.

fi if [ -z "$INTERFACE" ]; then echo "Error: Could not determine private network interface." >&2 exit 1 fi MAC=$(cat /sys/class/net/$INTERFACE/address)

gemini-code-assist · 2025-08-06T14:33:39Z

locals.tf

+      IP_LINK_NO_FLANNEL=$(ip link show | grep -v 'flannel')
+
+      # Try to find an interface with a predictable name, e.g., enp1s0
+      INTERFACE=$(echo "$IP_LINK_NO_FLANNEL" | awk '/enp[0-9]+s[0-9]+:/{print $2; exit}' | sed 's/://g')


For efficiency and conciseness, you can combine the awk and sed commands into a single awk command by using sub(). This avoids an extra pipe and process. Also, using a here-string (<<<) is more efficient than piping from echo as it avoids a subshell.

INTERFACE=$(awk '/enp[0-9]+s[0-9]+:/{sub(/:/,"",$2); print $2; exit}' <<< "$IP_LINK_NO_FLANNEL")

gemini-code-assist · 2025-08-06T14:33:39Z

locals.tf

+
+      # If no predictable name is found, use original logic as fallback
+      if [ -z "$INTERFACE" ]; then
+        INTERFACE=$(echo "$IP_LINK_NO_FLANNEL" | awk 'BEGIN{l3=""}; /^3:/{l3=$2}; /^2:/{l2=$2}; END{if(l3!="") print l3; else print l2}' | sed 's/://g')


For better performance, you can use a here-string (<<<) instead of piping from echo, which avoids creating an unnecessary subshell. Additionally, the g flag in sed 's/://g' is not needed here as you only expect to remove one colon from the interface name.

INTERFACE=$(awk 'BEGIN{l3=""}; /^3:/{l3=$2}; /^2:/{l2=$2}; END{if(l3!="") print l3; else print l2}' <<< "$IP_LINK_NO_FLANNEL" | sed 's/://')

…proved error handling and fallback mechanism

arbianshkodra · 2025-08-06T14:37:07Z

/gemini review

gemini-code-assist

Code Review

This pull request improves the network interface detection logic in the cloud-init script, making it more robust for different interface naming patterns like enpXsY. The new logic first attempts to find a predictably named interface and falls back to the original detection method, which is a solid approach. Storing the ip link show output in a variable is a good efficiency improvement. The addition of error handling for cases where no interface is found is also a valuable enhancement. I've added a couple of suggestions to further improve efficiency and remove some potentially redundant code.

gemini-code-assist · 2025-08-06T14:40:04Z

locals.tf

+      if [ -z "$INTERFACE" ]; then
+        echo "ERROR: Failed to detect network interface for renaming to eth1" >&2
+        echo "Available interfaces:" >&2
+        ip link show | grep -E '^[0-9]+:' >&2


This makes an additional ip link show call in the error path. To improve efficiency and be consistent with the logic that filters out the flannel interface, you could reuse the IP_LINK_NO_FLANNEL variable here.

echo "$IP_LINK_NO_FLANNEL" >&2

gemini-code-assist · 2025-08-06T14:40:05Z

locals.tf

      if [ "$INTERFACE" = "eth1" ]; then
        echo "Interface $INTERFACE already points to $MAC, skipping..."
        exit 0
      fi


This check seems redundant. The outer if ! ip link show eth1; then block on line 981 is only entered if the eth1 interface does not exist. Therefore, the INTERFACE variable discovered within this block should never be eth1, making this condition unreachable. This block can likely be removed.

…rove error messaging and streamline interface handling

arbianshkodra · 2025-08-06T14:43:12Z

/gemini review

gemini-code-assist

Code Review

This pull request improves the network interface detection logic in the cloudinit script. The changes are a good step forward, making the script more efficient by caching command output and more robust with better error handling. I've identified a potential high-severity issue where the script could fail if no non-flannel interfaces are found, and I've provided a suggestion to fix it. Additionally, I've suggested a medium-severity improvement to make an awk pattern more specific and prevent potential mismatches. The change in variables.tf is a minor formatting adjustment.

gemini-code-assist · 2025-08-06T14:45:12Z

locals.tf

-      MAC=$(cat /sys/class/net/$INTERFACE/address)
+      # Find the private network interface by name, falling back to original logic.
+      # The output of 'ip link show' is stored to avoid multiple calls.
+      IP_LINK_NO_FLANNEL=$(ip link show | grep -v 'flannel')


The script uses set -euo pipefail. If grep -v 'flannel' doesn't find any matching lines (i.e., all interfaces have 'flannel' in their description line), it will exit with status 1. Due to pipefail and set -e, this will cause the entire script to terminate prematurely. To prevent this, you should ensure that a non-zero exit code from grep (when no lines match) doesn't cause the script to fail.

IP_LINK_NO_FLANNEL=$(ip link show | (grep -v 'flannel' || [ $? -eq 1 ]))

gemini-code-assist · 2025-08-06T14:45:13Z

locals.tf

+      IP_LINK_NO_FLANNEL=$(ip link show | grep -v 'flannel')
+
+      # Try to find an interface with a predictable name, e.g., enp1s0
+      INTERFACE=$(awk '/enp[0-9]+s[0-9]+:/{sub(/:/,"",$2); print $2; exit}' <<< "$IP_LINK_NO_FLANNEL")


The awk pattern /enp[0-9]+s[0-9]+:/ could potentially match incorrectly if the interface name pattern appears elsewhere on the line. To make the detection more robust, it's better to anchor the pattern to the second field of the ip link show output.

INTERFACE=$(awk '$2 ~ /^enp[0-9]+s[0-9]+:$/{sub(/:/,"",$2); print $2; exit}' <<< "$IP_LINK_NO_FLANNEL")

…hanced error handling and pattern matching

arbianshkodra · 2025-08-06T14:48:28Z

/gemini review

gemini-code-assist

Code Review

This pull request significantly improves the network interface detection logic within the cloud-init script, making it more robust and efficient. The changes successfully address the issue with varied interface naming patterns, reduce redundant system calls by caching command output, and introduce better error handling. The resulting code is clearer and more maintainable. I have a couple of suggestions to further enhance the script's robustness by adhering to shell scripting best practices.

gemini-code-assist · 2025-08-06T14:49:48Z

locals.tf

@@ -979,18 +979,29 @@ cloudinit_write_files_common = <<EOT
    # interface may already be correctly names, and this
    # block should be skipped.
    if ! ip link show eth1; then


The ip link show eth1 command will print interface details to standard output if the command is successful. Since we are only interested in the exit code to check for the interface's existence, it's best practice to redirect both stdout and stderr to /dev/null. This prevents cluttering the execution logs and simplifies debugging.

if ! ip link show eth1 >/dev/null 2>&1; then

gemini-code-assist · 2025-08-06T14:49:48Z

locals.tf

-      # Take row beginning with 3 if exists, 2 otherwise (if only a private ip)
-      # WV REMOVE > INTERFACE=$(ip link show | awk 'BEGIN{l3=""}; /^3:/{l3=$2}; /^2:/{l2=$2}; END{if(l3!="") print l3; else print l2}' | sed 's/://g')
-      # WV REMOVE > MAC=$(cat /sys/class/net/$INTERFACE/address)
+      MAC=$(cat /sys/class/net/$INTERFACE/address)


It is a good shell scripting practice to quote variables when they are used in commands, especially as part of a file path. This prevents unexpected behavior from word splitting or glob expansion if the variable were to contain spaces or special characters. While interface names are unlikely to contain such characters, quoting $INTERFACE adds robustness to the script.

MAC=$(cat "/sys/class/net/$INTERFACE/address")

…output and ensuring proper path handling for MAC address retrieval

arbianshkodra · 2025-08-06T14:56:13Z

/gemini review

gemini-code-assist

Code Review

This pull request significantly improves the network interface detection logic in the cloud-init script. The changes make the script more robust by handling different interface naming patterns (like enp7s0), more efficient by caching the output of ip link show, and easier to debug by adding clear error handling for cases where an interface cannot be detected. The refactoring is well-executed and a clear improvement over the previous implementation.

gemini-code-assist · 2025-08-06T14:57:52Z

locals.tf

+
+      # If no predictable name is found, use original logic as fallback
+      if [ -z "$INTERFACE" ]; then
+        INTERFACE=$(awk 'BEGIN{l3=""}; /^3:/{l3=$2; sub(/:/,"",l3)}; /^2:/{l2=$2; sub(/:/,"",l2)}; END{if(l3!="") print l3; else print l2}' <<< "$IP_LINK_NO_FLANNEL")


This awk command for the fallback interface detection can be made more concise and efficient. Instead of initializing variables in a BEGIN block and performing substitutions on every potential match, you can assign matches to variables and perform a single substitution in the END block. This improves readability and is slightly more performant as sub() is called only once.

INTERFACE=$(awk '/^3:/{p=$2} /^2:/{s=$2} END{iface=p?p:s; sub(/:/,"",iface); print iface}' <<< "$IP_LINK_NO_FLANNEL")

…eadability and efficiency

arbianshkodra · 2025-08-06T15:00:50Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the network interface detection logic in a cloud-init script to correctly handle more interface naming patterns and improve efficiency. The changes are well-structured, adding better error handling and reducing system calls. My review includes suggestions to improve the script's portability by replacing bash-specific syntax with POSIX-compliant alternatives, ensuring it runs reliably across different Linux distributions.

gemini-code-assist · 2025-08-06T15:02:30Z

locals.tf

+
+      # Try to find an interface with a predictable name, e.g., enp1s0
+      # Anchor pattern to second field to avoid false matches
+      INTERFACE=$(awk '$2 ~ /^enp[0-9]+s[0-9]+:$/{sub(/:/,"",$2); print $2; exit}' <<< "$IP_LINK_NO_FLANNEL")


The here-string (<<<) is a bash-specific feature. While it may work if this script is explicitly run with bash, it can fail on systems where the default shell (/bin/sh) is not bash (e.g., dash on Debian/Ubuntu). To improve portability and ensure the script runs correctly across different environments, it's recommended to use a more POSIX-compliant method like a pipe with printf. This change makes the script more robust.

INTERFACE=$(printf '%s' "$IP_LINK_NO_FLANNEL" | awk '$2 ~ /^enp[0-9]+s[0-9]+:$/{sub(/:/,"",$2); print $2; exit}')

gemini-code-assist · 2025-08-06T15:02:31Z

locals.tf

+
+      # If no predictable name is found, use original logic as fallback
+      if [ -z "$INTERFACE" ]; then
+        INTERFACE=$(awk '/^3:/{p=$2} /^2:/{s=$2} END{iface=p?p:s; sub(/:/,"",iface); print iface}' <<< "$IP_LINK_NO_FLANNEL")


As mentioned for line 989, the here-string (<<<) is a bashism and can cause issues on systems where /bin/sh is not bash. Using a pipe with printf is a more portable and robust solution for passing the variable to awk's standard input.

INTERFACE=$(printf '%s' "$IP_LINK_NO_FLANNEL" | awk '/^3:/{p=$2} /^2:/{s=$2} END{iface=p?p:s; sub(/:/,"",iface); print iface}')

arbianshkodra · 2025-08-06T15:03:07Z

@mysticaltech, is this enough refactoring?

mysticaltech · 2025-08-06T15:06:23Z

@arbianshkodra Awesome, thanks

ulfgebhardt · 2025-08-20T15:19:33Z

@mysticaltech @arbianshkodra Whats the status on this. I just broke my cluster with an update and wonder if this could fix it again - see #1877

arbianshkodra · 2025-08-20T15:22:55Z

Waiting on this to be merged, if @mysticaltech had the chance to test it, or someone else as well.

mysticaltech · 2025-08-20T15:40:43Z

Thanks @arbianshkodra , will test it later today.

elkh510 · 2025-09-03T19:14:02Z

Hi @mysticaltech
Could you pls merge this?

Improve interface detection logic in cloudinit script

1604ddc