Skip to content

Fix phc2sys source failover test to handle multiple log matches #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gtannous-spec wants to merge 1 commit into from
+14 −11
Closed

Fix phc2sys source failover test to handle multiple log matches #231

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions test/conformance/serial/ptp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1031,10 +1031,12 @@ var _ = Describe("["+strings.ToLower(DesiredMode.String())+"-serial]", Serial, f
fullConfig.DiscoveredClockUnderTestPod.Name, pkg.PtpContainerName,
phc2sysLogPattern, false, pkg.TimeoutIn1Minute)
Expect(err).NotTo(HaveOccurred())
Expect(len(logMatches)).To(BeNumerically("==", 1), "Could not identify which interface phc2sys is using")
Expect(len(logMatches)).To(BeNumerically(">=", 1), "Could not identify which interface phc2sys is using")

@nocturnalastro nocturnalastro May 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should we see more than 1?

Last time it was more than 1 we had a bug that was duplicating lines.

The other reason would be it is switching master then switching again which would also be odd.

Do you have an example of this happening?

@gtannous-spec gtannous-spec May 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is causing this #223 PR to fail on this test (after enabling PTP authentication), I added there a CI negative test to verify that slaves don't sync to master in case we have keys mismatch.

while we change SPP from 1 to 0 on the GM config and then from 0 to 1 each change causes the daemon to restart the ptp4l/phc2sys processes inside the same pod.
phc2sys re-selects its interface each time and logs "selecting". By the time the DualNICBCHA test ("DualNICBCHA phc2sys switches to secondary ptp4l when primary interface fails") runs, the pod has 3 accumulated entries: 1 from initial startup + 1 from spp change + 1 from spp restore.

I was not tracking the bug you talked about following the fact that "was duplicating lines", if this could lead to another issue then I can try a different approach to "get rid" of the 3 log entries.

@gtannous-spec gtannous-spec May 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed that PR, now all tests pass. I guess problem was replacing the pod before it was in LOCKED state.
which leads the operator to reconcile causing another process to restart so pod ends up selecting multiple entries.

Will close this one.


logrus.Infof("phc2sys log matching line: %v", logMatches[0][0])
selectedInterface = logMatches[0][1]
lastMatch := logMatches[len(logMatches)-1]
logrus.Infof("phc2sys log matching line: %v", lastMatch[0])
selectedInterface = lastMatch[1]
logrus.Infof("selected interface: %s", selectedInterface)
Comment on lines +1036 to +1039

@gemini-code-assist gemini-code-assist Bot May 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for extracting the interface from the phc2sys logs is duplicated in this file. Consider refactoring this into a helper function to improve maintainability and reduce code duplication.

@gtannous-spec gtannous-spec May 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: Will modify that after we validate all ci-tests pass.


// Save it as primary interface
primaryInterface := selectedInterface
Expand Down Expand Up @@ -1069,11 +1071,12 @@ var _ = Describe("["+strings.ToLower(DesiredMode.String())+"-serial]", Serial, f
fullConfig.DiscoveredClockUnderTestPod.Name, pkg.PtpContainerName,
phc2sysLogPattern, false, pkg.TimeoutIn1Minute, ifDownTime)
Expect(err).NotTo(HaveOccurred())
Expect(len(logMatches)).To(BeNumerically("==", 1), "Could not identify which interface phc2sys is using")
Expect(len(logMatches)).To(BeNumerically(">=", 1), "Could not identify which interface phc2sys is using")
// Get the most recent log entry
logrus.Infof("phc2sys log matching line: %v", logMatches[0][0])
newSelectedInterface = logMatches[0][1]

lastMatch = logMatches[len(logMatches)-1]
logrus.Infof("phc2sys log matching line: %v", lastMatch[0])
newSelectedInterface = lastMatch[1]
logrus.Infof("new selected interface: %s", newSelectedInterface)
// Verify that phc2sys switched to a different interface
Expect(newSelectedInterface).ToNot(Equal(selectedInterface), "phc2sys should have switched to a different interface")

Expand Down Expand Up @@ -1102,10 +1105,10 @@ var _ = Describe("["+strings.ToLower(DesiredMode.String())+"-serial]", Serial, f
fullConfig.DiscoveredClockUnderTestPod.Name, pkg.PtpContainerName,
phc2sysLogPattern, false, pkg.TimeoutIn1Minute, ifUpTime)
Expect(err).NotTo(HaveOccurred())
Expect(len(logMatches)).To(BeNumerically("==", 1), "Could not identify which interface phc2sys is using")
// Get the most recent log entry
logrus.Infof("phc2sys log matching line: %v", logMatches[0][0])
selectedInterface = logMatches[0][1]
Expect(len(logMatches)).To(BeNumerically(">=", 1), "Could not identify which interface phc2sys is using after port restore")
lastMatch = logMatches[len(logMatches)-1]
logrus.Infof("phc2sys log matching line: %v", lastMatch[0])
selectedInterface = lastMatch[1]

By("Verifying the selected interface " + selectedInterface + " is the original primary BC's slave interface " + primaryInterface)
if selectedInterface != primaryInterface {
Expand Down
Loading