Skip to content

Add containerlab best practices skills - #3256

Draft
FloSch62 wants to merge 4 commits into
mainfrom
improve_apply
Draft

Add containerlab best practices skills#3256
FloSch62 wants to merge 4 commits into
mainfrom
improve_apply

Conversation

@FloSch62

Copy link
Copy Markdown
Member

No description provided.

@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 59.57447% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.80%. Comparing base (05c2006) to head (d618147).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
links/link_vxlan.go 25.00% 7 Missing and 2 partials ⚠️
core/clab.go 63.63% 2 Missing and 2 partials ⚠️
core/deploy.go 25.00% 1 Missing and 2 partials ⚠️
links/apply.go 60.00% 1 Missing and 1 partial ⚠️
cmd/tools_vxlan.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3256      +/-   ##
==========================================
+ Coverage   56.67%   56.80%   +0.13%     
==========================================
  Files         244      244              
  Lines       25517    25535      +18     
==========================================
+ Hits        14461    14505      +44     
+ Misses       9390     9360      -30     
- Partials     1666     1670       +4     
Files with missing lines Coverage Δ
core/apply.go 46.40% <100.00%> (+2.95%) ⬆️
links/link_macvlan.go 50.38% <100.00%> (+0.78%) ⬆️
links/link_vxlan_stitched.go 67.39% <100.00%> (+2.27%) ⬆️
cmd/tools_vxlan.go 72.02% <50.00%> (ø)
links/apply.go 69.23% <60.00%> (+19.23%) ⬆️
core/deploy.go 57.37% <25.00%> (ø)
core/clab.go 78.20% <63.63%> (-0.50%) ⬇️
links/link_vxlan.go 58.28% <25.00%> (-3.41%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread docs/cmd/apply.md Outdated
Comment thread core/deploy.go Outdated
Comment thread links/apply.go Outdated
return nil
}

if link, ok := l.(runtimeEndpointProvider); ok {

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.

this is bad. try to avoid type casts... add the GetRuntimeEndpoints to the Link interface and use it straight away.
remove the runtimeEndpointProvider.

type Link interface {
    // ...
    GetEndpoints() []Endpoint
    // GetRuntimeEndpoints returns endpoints apply/deploy may create, discover, or remove.
    // Parent-only and remote metadata endpoints are excluded where applicable.
    GetRuntimeEndpoints() []Endpoint
}

@steiler

steiler commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

The branch mixes:

  • apply endpoint architecture (links/apply.go, link methods)
  • apply state persistence (core/apply.go)
  • resolve-links idempotency (resetResolvedLinks)
  • agent skills/docs

better pull at least some of it into distinct PRs.

Comment thread links/link_vxlan.go
return nil, nil, fmt.Errorf("failed to resolve host veth component for vxlan-stitch")
}

// host endpoint is always the 2nd element in the Endpoints slice

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.

eps[1] assumes LinkHostRaw.Resolve always returns [node, host]. That's true today but implicit. We already know hostIface here — selecting by ep.GetIfaceName() == hostIface (or a small helper) would make the stitch endpoint choice explicit and order-independent.

@FloSch62 FloSch62 changed the title Add best practices skills and improve apply Add containerlab best practices skills Jun 30, 2026
@FloSch62

Copy link
Copy Markdown
Member Author

This PR is now only about agent skills/rules
Apply changes are now here: #3260

hellt added a commit that referenced this pull request Jul 10, 2026
## Summary
- move apply runtime endpoint selection onto the `links.Link` contract
with `GetRuntimeEndpoints()`
- keep stitched-link capability in the `links` package and update
apply/deploy callers
- persist apply state and reset resolved links for idempotent apply
paths
- remove short kind aliases from the apply docs table
- select the VXLAN stitch endpoint by interface name instead of endpoint
slice order

## Validation
- `go test ./links ./core ./cmd`

Split out from #3256 so that PR can stay focused on agent skills only.

---------

Co-authored-by: hellt <dodin.roman@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants