tests: Give more time for interface information to show up

[donaldsharp]

The test failed in upstream CI because the loopback did not have the address as of yet as part of a show interface. The show run showed that the address was applied, but the interface information in zebra and from ip ... commands showed that the data had not finished being sent to the kernel. Give this test more time to converge.

[greptile-apps]

Greptile Summary

This PR addresses a CI flakiness issue where the test_topology_setup test was failing because the loopback address on leaf2 had not yet propagated to zebra's interface table at the time of the show interface lo query. The fix wraps the single one-shot retrieval in a run_and_expect retry loop (up to 20 × 1 s = 20 seconds), matching the pattern already used for BGP and route-convergence checks elsewhere in the same test.

Key changes:

• check_leaf2_loopback_ipv4 is a new inner function that polls vtysh -c 'show interface lo' and stores the found IP in a closure dict (ipv4_state).
• topotest.run_and_expect drives the polling, consistent with the rest of the file.
• expected_loopback_ipv4 = "10.0.0.2" is introduced but only appears in the assertion failure message — it is never compared against the address actually returned by the regex. If the regex matches a different IP before 10.0.0.2 is configured, the retry loop exits early with the wrong nexthop and the subsequent sharp-route install silently uses a bad destination.

Confidence Score: 3/5

• The timing fix is correct in principle but the expected_loopback_ipv4 variable is never validated, which could cause the retry to succeed with a wrong nexthop.
• The retry-loop approach is sound and consistent with the rest of the test file. However, expected_loopback_ipv4 is defined and referenced in the error message but never compared against the retrieved IP, meaning the loop can exit successfully with an unintended address, leading to silent downstream failures.
• tests/topotests/two_layer_wucmp/test_two_layer_wuecmp.py — specifically the check_leaf2_loopback_ipv4 closure and the missing validation of the retrieved IP against expected_loopback_ipv4.

Important Files Changed

Filename	Overview
tests/topotests/two_layer_wucmp/test_two_layer_wuecmp.py	Wraps loopback-address retrieval in a run_and_expect retry loop to handle timing races; expected_loopback_ipv4 is defined but not actually validated against the retrieved address, which could allow the wrong nexthop to be silently used.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[test_topology_setup: Phase 2] --> B[Define expected_loopback_ipv4 = '10.0.0.2']
    B --> C[check_leaf2_loopback_ipv4\nclosure]
    C --> D{run_and_expect\ncount=20, wait=1s}
    D -->|retry| E[vtysh show interface lo]
    E --> F{inet addr\nfound?}
    F -->|No| G[log: still waiting] --> D
    F -->|Yes - any IP| H[ipv4_state nexthop = matched IP\nNOTE: not validated vs expected]
    H --> I[return True]
    D -->|success=True| J[assert success]
    D -->|timeout| K[assert failure\nmessage shows expected_loopback_ipv4]
    J --> L[ipv4_nexthop = ipv4_state nexthop]
    L --> M[sharp install routes\nusing ipv4_nexthop as nexthop]

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: tests/topotests/two_layer_wucmp/test_two_layer_wuecmp.py
Line: 377-402

Comment:
**`expected_loopback_ipv4` defined but never validated**

`expected_loopback_ipv4 = "10.0.0.2"` is only interpolated into the failure message — it is never compared against the IP address actually returned by the regex. The function `check_leaf2_loopback_ipv4` succeeds as soon as *any* `inet x.x.x.x/...` line appears in `show interface lo`. If another address is matched first (e.g. an unexpected loopback address configured before `10.0.0.2` arrives), `ipv4_nexthop` would silently be set to the wrong value and the subsequent sharp-route install would use a bad nexthop, causing downstream test failures that are hard to trace back to this point.

Consider validating the retrieved address against the expected value so the retry keeps waiting until the *correct* address is seen:

```suggestion
    def check_leaf2_loopback_ipv4():
        lo_output = net["leaf2"].cmd("vtysh -c 'show interface lo'")
        ipv4_match = re.search(r"inet (\d+\.\d+\.\d+\.\d+)/\d+", lo_output)

        if not ipv4_match:
            logger.info("Still waiting for IPv4 address on leaf2 loopback interface")
            return False

        found_ip = ipv4_match.group(1)
        if found_ip != expected_loopback_ipv4:
            logger.info(
                f"Found {found_ip} on leaf2 loopback, still waiting for {expected_loopback_ipv4}"
            )
            return False

        ipv4_state["nexthop"] = found_ip
        return True
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "tests: Give more tim..."}

[greptile-apps]

expected_loopback_ipv4 defined but never validated

expected_loopback_ipv4 = "10.0.0.2" is only interpolated into the failure message — it is never compared against the IP address actually returned by the regex. The function check_leaf2_loopback_ipv4 succeeds as soon as any inet x.x.x.x/... line appears in show interface lo. If another address is matched first (e.g. an unexpected loopback address configured before 10.0.0.2 arrives), ipv4_nexthop would silently be set to the wrong value and the subsequent sharp-route install would use a bad nexthop, causing downstream test failures that are hard to trace back to this point.

Consider validating the retrieved address against the expected value so the retry keeps waiting until the correct address is seen:

Suggested change

	expected_loopback_ipv4 = "10.0.0.2"
	ipv4_state = {"nexthop": None}
	if not ipv4_match:
	assert False, "Could not find IPv4 address on loopback interface"
	def check_leaf2_loopback_ipv4():
	lo_output = net["leaf2"].cmd("vtysh -c 'show interface lo'")
	ipv4_match = re.search(r"inet (\d+\.\d+\.\d+\.\d+)/\d+", lo_output)
	ipv4_nexthop = ipv4_match.group(1)
	if not ipv4_match:
	logger.info("Still waiting for IPv4 address on leaf2 loopback interface")
	return False
	ipv4_state["nexthop"] = ipv4_match.group(1)
	return True
	success, result = topotest.run_and_expect(
	check_leaf2_loopback_ipv4,
	True,
	count=20,
	wait=1,
	)
	assert (
	success
	), f"Could not find IPv4 address on loopback interface for node leaf2; expected {expected_loopback_ipv4}"
	ipv4_nexthop = ipv4_state["nexthop"]
	def check_leaf2_loopback_ipv4():
	lo_output = net["leaf2"].cmd("vtysh -c 'show interface lo'")
	ipv4_match = re.search(r"inet (\d+\.\d+\.\d+\.\d+)/\d+", lo_output)
	if not ipv4_match:
	logger.info("Still waiting for IPv4 address on leaf2 loopback interface")
	return False
	found_ip = ipv4_match.group(1)
	if found_ip != expected_loopback_ipv4:
	logger.info(
	f"Found {found_ip} on leaf2 loopback, still waiting for {expected_loopback_ipv4}"
	)
	return False
	ipv4_state["nexthop"] = found_ip
	return True

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/topotests/two_layer_wucmp/test_two_layer_wuecmp.py
Line: 377-402

Comment:
**`expected_loopback_ipv4` defined but never validated**

`expected_loopback_ipv4 = "10.0.0.2"` is only interpolated into the failure message — it is never compared against the IP address actually returned by the regex. The function `check_leaf2_loopback_ipv4` succeeds as soon as *any* `inet x.x.x.x/...` line appears in `show interface lo`. If another address is matched first (e.g. an unexpected loopback address configured before `10.0.0.2` arrives), `ipv4_nexthop` would silently be set to the wrong value and the subsequent sharp-route install would use a bad nexthop, causing downstream test failures that are hard to trace back to this point.

Consider validating the retrieved address against the expected value so the retry keeps waiting until the *correct* address is seen:

```suggestion
    def check_leaf2_loopback_ipv4():
        lo_output = net["leaf2"].cmd("vtysh -c 'show interface lo'")
        ipv4_match = re.search(r"inet (\d+\.\d+\.\d+\.\d+)/\d+", lo_output)

        if not ipv4_match:
            logger.info("Still waiting for IPv4 address on leaf2 loopback interface")
            return False

        found_ip = ipv4_match.group(1)
        if found_ip != expected_loopback_ipv4:
            logger.info(
                f"Found {found_ip} on leaf2 loopback, still waiting for {expected_loopback_ipv4}"
            )
            return False

        ipv4_state["nexthop"] = found_ip
        return True
```

How can I resolve this? If you propose a fix, please make it concise.

[donaldsharp]

@greptile Why do we need to make a function generic to handle any input when this is specific code to handle a specific test scenario where there will never be other addresses on lo

[greptile-apps]

You're right. In a controlled test environment where the loopback will only have the configured address, checking for the presence of any IPv4 address is sufficient. The original comment was over-engineering the solution for a test with a known, fixed topology.

The current implementation appropriately waits for the interface information to appear in zebra without adding unnecessary validation logic. Since expected_loopback_ipv4 is only used for the error message, that's fine—it makes the failure message more informative if something does go wrong, but you don't need to validate against it during the check.

[Jafaral]

@Mergifyio backport stable/10.6

[mergify]

backport stable/10.6

✅ Backports have been created

Details

• #21292 tests: Give more time for interface information to show up (backport #21278) has been created for branch stable/10.6