Sync from upstream (22-Jul-2026) - #718
Conversation
DualFollower and other multi-port NIC tests fail looking up openshift_ptp_clock_state by port name (ens2f0np0) while the daemon emits ens2fx. Align getMetric aliasing with linuxptp-daemon GetAliasValue. Generated-by: Cursor
…e-alias-np-suffix fix: resolve PHC iface aliases for np-suffixed NICs in metrics checks
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nocturnalastro The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
WalkthroughThe metrics test helper now derives PHC interface aliases using compiled regex rules and updates metric lookup to try both the original interface and a distinct derived alias. ChangesPHC metric aliasing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/pkg/metrics/metrics.go`:
- Around line 235-236: Escape each candidate alias with regexp.QuoteMeta before
interpolating it into regex patterns in all three branches of the alias-matching
logic around getIfaceAlias. Preserve the existing matching behavior while
preventing metacharacters in raw aliases from changing the regex, and add a
regression test covering a VLAN alias such as ens1fx.100.
- Around line 201-203: Restrict ifaceAliasPattern to matching only the
documented PHC interface forms, including ens2f0, ens2f0np0, and ens1f0.100,
while preserving the optional representor and VLAN suffix handling needed by
getMetric. Update the associated tests with negative cases for ordinary NIC,
bridge, and VLAN names so these names are never aliased.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 88fcb3d1-00c8-4750-8536-329487d0ed81
📒 Files selected for processing (1)
test/pkg/metrics/metrics.go
| var ( | ||
| alreadyAliasedPattern = regexp.MustCompile(`^(.+?)x(\..+)?$`) | ||
| ifaceAliasPattern = regexp.MustCompile(`^(.+?)(\d+)(?:np\d+)?(\..+)?$`) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
git ls-files test/pkg/metrics/metrics.go
echo '--- outline ---'
ast-grep outline test/pkg/metrics/metrics.go --view expanded || true
echo '--- relevant lines ---'
sed -n '160,260p' test/pkg/metrics/metrics.go | cat -n
echo '--- searches ---'
rg -n "getIfaceAlias|ifaceAliasPattern|alreadyAliasedPattern|getMetric|PHC|alias" test/pkg/metrics/metrics.go test/pkg/metrics -SRepository: openshift/ptp-operator
Length of output: 11623
🏁 Script executed:
set -euo pipefail
# Find any tests or docs describing the alias contract.
rg -n "enox|bondx|ifaceAliasPattern|getIfaceAlias|PHC|linuxptp|alias" . -S --glob '!**/vendor/**' --glob '!**/node_modules/**'Repository: openshift/ptp-operator
Length of output: 50379
🏁 Script executed:
set -euo pipefail
# Check whether the candidate aliases are actually used by current tests.
sed -n '1,220p' test/pkg/metrics/metrics_test.go | cat -nRepository: openshift/ptp-operator
Length of output: 236
🏁 Script executed:
set -euo pipefail
# If there is a README or fixture documenting metric interface naming, inspect it.
rg -n "interface.*metric|PHC|linuxptp|alias" test pkg README.md . -S --glob '!**/vendor/**' --glob '!**/node_modules/**'Repository: openshift/ptp-operator
Length of output: 50379
🏁 Script executed:
set -euo pipefail
sed -n '180,250p' test/pkg/metrics/metrics.go | cat -nRepository: openshift/ptp-operator
Length of output: 3220
🌐 Web query:
linuxptp-daemon cloud-event-proxy GetAlias interface alias ens2f0 ens2fx source code
💡 Result:
The term GetAlias as referenced in the context of linuxptp-daemon and cloud-event-proxy is not a standard standalone API function, but rather refers to an internal mechanism used for mapping PTP network interfaces to their respective PHC (PTP Hardware Clock) identifiers [1]. In the linuxptp-daemon implementation, the alias store is used to match hardware interfaces (like ens2f0) to their PHC IDs [1]. This is critical in environments with multi-port NICs where different physical interfaces may share the same PHC, or where naming prefixes differ (e.g., comparing ens2f0 vs ens2fx) [1]. The daemon registers these interfaces in an alias store using the phcID and the network interface name, ensuring that the convergeConfig logic correctly associates PTP processes with the appropriate hardware [1]. Recent updates to the cloud-event-proxy (such as commit ceae33f) have explicitly improved the handling of these aliases by fetching them before the proxy begins listening to the event socket [2]. This ensures that alias mappings are correctly initialized if the cloud-event-proxy restarts while the linuxptp-daemon is already active, preventing defaults that might otherwise fall back to incorrect interface naming [2]. The GetAlias name itself is common in various software SDKs (such as AWS Lambda or other cloud management APIs) [3][4], but within the specific Red Hat/OpenShift PTP stack, it typically relates to the internal helper methods or storage management logic used by the ptp_operator_plugin and linuxptp-daemon to maintain the consistency of interface-to-PHC mapping [2][1].
Citations:
- 1: https://github.com/openshift/linuxptp-daemon/blob/master/pkg/daemon/daemon.go
- 2: redhat-cne/cloud-event-proxy@ceae33f
- 3: https://floci.io/floci/services/lambda/
- 4: https://docs.aws.amazon.com/lambda/latest/api/API_GetAlias.html
🌐 Web query:
site:github.com/openshift/linuxptp-daemon GetAlias ens2f0 ens2fx code
💡 Result:
The term "GetAlias" in the context of ens2f0 and ens2fx appears to be related to a specific internal mechanism within the openshift/linuxptp-daemon project, which manages PTP (Precision Time Protocol) configurations on Kubernetes clusters [1][2][3]. The linuxptp-daemon uses an alias management system to handle interface naming differences, particularly when multiple physical interfaces share the same PHC (PTP Hardware Clock) [1]. In the source code for this daemon, alias.AddInterface(phc, ifname) and alias.CalculateAliases() are used to register and map network interface names to their corresponding PHC IDs [1]. This ensures that PTP processes (like ptp4l) correctly associate network devices—such as ens2f0—even if their hardware identifiers or naming conventions differ across nodes [1]. There is no standard Linux kernel command or general-purpose utility called GetAlias for network interface renaming. The pattern you provided likely refers to a function or method within a specialized application (like the OpenShift PTP operator's components) designed to abstract hardware interface naming for configuration consistency [1][3]. If you are encountering this in a codebase, it is part of the custom logic used to normalize interface names for PTP hardware clock synchronization [1]. If you are looking for how to handle or rename network interfaces in standard Linux, you should use tools like ip link or systemd's predictable network interface naming policies instead.
Citations:
- 1: https://github.com/openshift/linuxptp-daemon/blob/master/pkg/daemon/daemon.go
- 2: https://github.com/openshift/linuxptp-daemon/blob/main/README.md
- 3: https://github.com/openshift/linuxptp-daemon
🏁 Script executed:
set -euo pipefail
echo '--- package files ---'
git ls-files 'test/pkg/metrics/*' 'test/pkg/*' | sed -n '1,120p'
echo '--- interface-name mentions in metrics package ---'
rg -n "ens[0-9]|bond[0-9]|eth[0-9]|enp|np[0-9]|vlan|alias|GetAlias|getIfaceAlias|ifaceAliasPattern" test/pkg/metrics -S
echo '--- surrounding callers ---'
sed -n '100,380p' test/pkg/metrics/metrics.go | cat -nRepository: openshift/ptp-operator
Length of output: 14751
🏁 Script executed:
set -euo pipefail
# Show just the call sites and helper behavior in a compact way.
python3 - <<'PY'
from pathlib import Path
p = Path('test/pkg/metrics/metrics.go')
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if 179 <= i <= 250 or 300 <= i <= 380:
print(f"{i:4d}: {line}")
PYRepository: openshift/ptp-operator
Length of output: 7318
Restrict PHC aliasing to the documented interface forms.
ifaceAliasPattern rewrites any numeric-suffixed name (eno1 → enox, bond0 → bondx, eth1.100 → ethx.100), so getMetric can fall back to the wrong interface when the aliased metric exists. Narrow this to the PHC naming contract (ens2f0/ens2f0np0/ens1f0.100) and add negative cases for ordinary NIC, bridge, and VLAN names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/pkg/metrics/metrics.go` around lines 201 - 203, Restrict
ifaceAliasPattern to matching only the documented PHC interface forms, including
ens2f0, ens2f0np0, and ens1f0.100, while preserving the optional representor and
VLAN suffix handling needed by getMetric. Update the associated tests with
negative cases for ordinary NIC, bridge, and VLAN names so these names are never
aliased.
Source: Path instructions
| aliasedIf := getIfaceAlias(aIf) | ||
| if aliasedIf != "" && aliasedIf != aIf { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -e
printf 'File size and relevant slices:\n'
wc -l test/pkg/metrics/metrics.go
sed -n '210,290p' test/pkg/metrics/metrics.go
printf '\nSearch for candidate/QuoteMeta/regexp usage in file:\n'
rg -n 'getIfaceAlias|QuoteMeta|MustCompile|candidate|ifCandidates' test/pkg/metrics/metrics.goRepository: openshift/ptp-operator
Length of output: 4234
Escape candidate before building the regex. test/pkg/metrics/metrics.go:240-249 Raw aliases like ens1fx.100 include regex metacharacters, so the unescaped interpolation can match the wrong metric. Use regexp.QuoteMeta(candidate) in all three branches and add a VLAN-alias regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/pkg/metrics/metrics.go` around lines 235 - 236, Escape each candidate
alias with regexp.QuoteMeta before interpolating it into regex patterns in all
three branches of the alias-matching logic around getIfaceAlias. Preserve the
existing matching behavior while preventing metacharacters in raw aliases from
changing the regex, and add a regression test covering a VLAN alias such as
ens1fx.100.
Source: Path instructions
|
@nocturnalastro: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
nocturnalastro
changed the title
2 participants
Upstream PRs included