Skip to content

Cover breakout port generation - #2567

Merged
berendt merged 1 commit into
mainfrom
sonic-e2e-v2-breakout
Sep 16, 2026
Merged

berendt merged 1 commit into
mainfrom
sonic-e2e-v2-breakout

Conversation

@ideaship

@ideaship ideaship commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

Part of the series tracked in #2562, which explains the ordering and what each PR covers. Based on the preceding PR in the stack, so review only the top commits here.

First scenario overlay: devices that exercise breakout port generation, covering
BREAKOUT_CFG and BREAKOUT_PORTS, which the base fixtures leave empty.

Two devices differ only in how their sub-port speed is established — one derived
from the interface type, one with an explicit NetBox speed — because the
kbps-to-Mbps conversion and the type-derivation fallback are separate code paths
that both feed breakout mode selection.

@ideaship ideaship changed the title sonic e2e v2 breakout Cover breakout port generation Aug 5, 2026
@berendt
berendt force-pushed the sonic-e2e-v2-breakout branch from 0acfa0a to f7628a6 Compare August 5, 2026 15:10
@ideaship
ideaship force-pushed the sonic-e2e-v2-breakout branch from f7628a6 to dbf7e9d Compare August 5, 2026 19:51
@berendt
berendt force-pushed the sonic-e2e-v2-breakout branch from dbf7e9d to e9567cb Compare August 6, 2026 10:56
@ideaship
ideaship force-pushed the sonic-e2e-v2-breakout branch from e9567cb to 9112fb9 Compare August 6, 2026 12:10
@ideaship
ideaship force-pushed the sonic-e2e-v2-breakout branch from 9112fb9 to c3f1efc Compare August 6, 2026 12:19
@berendt
berendt force-pushed the sonic-e2e-v2-breakout branch from c3f1efc to 90d8da1 Compare August 7, 2026 05:35
@ideaship
ideaship force-pushed the sonic-e2e-v2-breakout branch from 90d8da1 to 1db7834 Compare August 7, 2026 08:07
@berendt
berendt force-pushed the sonic-e2e-v2-breakout branch from 1db7834 to a1c92f5 Compare August 7, 2026 08:59
@ideaship
ideaship force-pushed the sonic-e2e-v2-breakout branch from a1c92f5 to 7dca70a Compare August 25, 2026 13:55
@ideaship
ideaship force-pushed the sonic-e2e-v2-breakout branch from 7dca70a to 58beccc Compare September 16, 2026 14:18
@ideaship
ideaship removed this pull request from stack #2572 September 16, 2026 14:27
@ideaship
ideaship force-pushed the sonic-e2e-v2-breakout branch from 58beccc to c5ae99c Compare September 16, 2026 15:02
@ideaship
ideaship force-pushed the sonic-e2e-v2-breakout branch from c5ae99c to 6c95c59 Compare September 16, 2026 15:11
@ideaship
ideaship added this pull request to stack #2704 September 16, 2026 15:50
@berendt
berendt force-pushed the sonic-e2e-v2-breakout branch from 6c95c59 to 4b964da Compare September 16, 2026 16:02
Base automatically changed from sonic-e2e-v2-fixtures to main September 16, 2026 18:05
The bundled netbox-manager example models no breakout ports and
sets no explicit interface speeds, so the generator's breakout
paths and its kbps->Mbps speed normalisation were never exercised
by the SONiC E2E golden test.

Add two standalone leaf devices on the shared E2E rack (positions
6 and 7, taking no cabling and needing none):

- e2e-breakout-derived: Eth1/1/1..4 use the device type's
  100gbase-x-qsfp28 interface type with no explicit speed, so the
  sub-port speed is derived from the interface type. Also carries
  a tagged VLAN on the plain Eth1/5 port, covering the VLAN /
  tagged-VLAN-to-port paths.
- e2e-breakout-explicit: the same four sub-ports instead carry an
  explicit NetBox speed of 100000000 kbps, exercising the other
  unit the collection step must normalise.

Both must yield sub-port speed 100000 in the generated config;
confirmed via the regenerated goldens (BREAKOUT_CFG and
BREAKOUT_PORTS populated on both, no bare speed "100" present).
This brings config_db table coverage from 30 to 32 of 38.

The two devices reuse the site, location, tenant, roles, tags and
custom fields already seeded by 100-base.yml, and the
edgecore-9726-32d-e2e device type is ported from ab8da03a.

Related-Bug: #2478
Related-Bug: #2246
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Roger Luethi <luethi@osism.tech>
@berendt
berendt force-pushed the sonic-e2e-v2-breakout branch from 4b964da to c615ca1 Compare September 16, 2026 18:05
@ideaship
ideaship marked this pull request as ready for review September 16, 2026 18:21
@ideaship
ideaship requested a review from berendt September 16, 2026 18:21

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/e2e/scenario/resources/500-breakout.yml" line_range="12-21" />
<code_context>
+# fields created by 100-base.yml, and take the next free positions in the
+# shared E2E rack so they never collide with the base devices.
+#
+# Both devices use the edgecore-9726-32d-e2e device type (hwsku
+# Accton-AS9726-32D) and break Eth1/1 into a 4x100G group. They differ
+# only in how the sub-port speed reaches NetBox:
+#
+#   e2e-breakout-derived   speed derived from the interface type
+#                          (100gbase-x-qsfp28 -> 100000 Mbps); no explicit
+#                          speed. This is how real deployments model
+#                          breakouts.
+#   e2e-breakout-explicit  the same sub-ports with an explicit NetBox speed
+#                          set in kbps (100000000), the other unit the
+#                          collection step must normalise.
+#
</code_context>
<issue_to_address>
**nitpick:** The scenario comment states that the two devices differ only in how sub-port speed reaches NetBox, but `e2e-breakout-derived` also has a tagged VLAN on `Eth1/5` while `e2e-breakout-explicit` has no corresponding VLAN fixture. The comment is misleading when comparing the resulting coverage and makes the fixture differences harder to understand.

**Suggested fix:** Change the comment to state that the devices differ in speed representation and that only the derived device additionally carries the tagged-VLAN coverage fixture.

```suggestion
# Accton-AS9726-32D) and break Eth1/1 into a 4x100G group. They differ in
# how the sub-port speed is represented in NetBox, and only the derived device
# additionally carries the tagged-VLAN coverage fixture:
#
#   e2e-breakout-derived   speed derived from the interface type
#                          (100gbase-x-qsfp28 -> 100000 Mbps); no explicit
#                          speed. This is how real deployments model
#                          breakouts.
#   e2e-breakout-explicit  the same sub-ports with an explicit NetBox speed
#                          set in kbps (100000000), the other unit the
#                          collection step must normalise.
```
</issue_to_address>

Sourcery assessment

Approved.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment on lines +12 to +21
# Accton-AS9726-32D) and break Eth1/1 into a 4x100G group. They differ
# only in how the sub-port speed reaches NetBox:
#
# e2e-breakout-derived speed derived from the interface type
# (100gbase-x-qsfp28 -> 100000 Mbps); no explicit
# speed. This is how real deployments model
# breakouts.
# e2e-breakout-explicit the same sub-ports with an explicit NetBox speed
# set in kbps (100000000), the other unit the
# collection step must normalise.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nitpick: The scenario comment states that the two devices differ only in how sub-port speed reaches NetBox, but e2e-breakout-derived also has a tagged VLAN on Eth1/5 while e2e-breakout-explicit has no corresponding VLAN fixture. The comment is misleading when comparing the resulting coverage and makes the fixture differences harder to understand.

Suggested fix: Change the comment to state that the devices differ in speed representation and that only the derived device additionally carries the tagged-VLAN coverage fixture.

Suggested change
# Accton-AS9726-32D) and break Eth1/1 into a 4x100G group. They differ
# only in how the sub-port speed reaches NetBox:
#
# e2e-breakout-derived speed derived from the interface type
# (100gbase-x-qsfp28 -> 100000 Mbps); no explicit
# speed. This is how real deployments model
# breakouts.
# e2e-breakout-explicit the same sub-ports with an explicit NetBox speed
# set in kbps (100000000), the other unit the
# collection step must normalise.
# Accton-AS9726-32D) and break Eth1/1 into a 4x100G group. They differ in
# how the sub-port speed is represented in NetBox, and only the derived device
# additionally carries the tagged-VLAN coverage fixture:
#
# e2e-breakout-derived speed derived from the interface type
# (100gbase-x-qsfp28 -> 100000 Mbps); no explicit
# speed. This is how real deployments model
# breakouts.
# e2e-breakout-explicit the same sub-ports with an explicit NetBox speed
# set in kbps (100000000), the other unit the
# collection step must normalise.

@berendt
berendt merged commit 8de4aee into main Sep 16, 2026
3 checks passed
@berendt
berendt deleted the sonic-e2e-v2-breakout branch September 16, 2026 19:28
@github-project-automation github-project-automation Bot moved this from New to Done in Human Board Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants