Skip to content

[NXOS] fix: do attempt to delete icmp redirection config on swithport interfaces#229

Draft
nikatza wants to merge 1 commit intomainfrom
disable-icmp-redirect
Draft

[NXOS] fix: do attempt to delete icmp redirection config on swithport interfaces#229
nikatza wants to merge 1 commit intomainfrom
disable-icmp-redirect

Conversation

@nikatza
Copy link
Contributor

@nikatza nikatza commented Mar 12, 2026

No description provided.

@nikatza nikatza force-pushed the disable-icmp-redirect branch from 21f241a to fe4444c Compare March 12, 2026 17:22
Check if interface is a switchport before attempting to remove its icmp
redirect configuration.
@nikatza nikatza force-pushed the disable-icmp-redirect branch from fe4444c to d0d3361 Compare March 12, 2026 17:23
@github-actions
Copy link

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos 10.70% (-0.00%) 👎

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/provider.go 0.07% (-0.00%) 1538 (+1) 1 1537 (+1) 👎

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

@hardikdr hardikdr added the area/metal-automation Automation processes within the Metal project. label Mar 13, 2026
@hardikdr hardikdr added this to Roadmap Mar 13, 2026
@nikatza nikatza marked this pull request as ready for review March 13, 2026 08:09
@nikatza nikatza requested a review from a team as a code owner March 13, 2026 08:09
@nikatza nikatza changed the title [NXOS] fix: do not delete icmp redirection config [NXOS] fix: do attempt to delete icmp redirection config on swithport interfaces Mar 13, 2026
@nikatza nikatza marked this pull request as draft March 13, 2026 08:34
case v1alpha1.InterfaceTypePhysical:
if err := p.client.Delete(ctx, icmp); err != nil {
return err
if req.Interface.Spec.Switchport == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this creating an error? Because the delete call seems to be working just fine for switchport interfaces.

λ gnmic -a 127.0.0.1 -u admin -p admin --port 9339 --skip-verify get --path 'System/intf-items/phys-items/PhysIf-list[id=eth1/12]/layer' --values-only --type config
[
  "Layer2"
]

λ gnmic -a 127.0.0.1 -u admin -p admin --port 9339 --skip-verify set --delete 'System/icmpv4-items/inst-items/dom-items/Dom-list[name=default]/if-items/If-list[id=eth1/12]'
{
  "source": "127.0.0.1",
  "timestamp": 1773406679784925525,
  "time": "2026-03-13T13:57:59.784925525+01:00",
  "results": [
    {
      "operation": "DELETE",
      "path": "System/icmpv4-items/inst-items/dom-items/Dom-list[name=default]/if-items/If-list[id=eth1/12]"
    }
  ]
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ignore this commit for now, i am still investigating an issue that may have some relation to this. One thing that caught our attention is that we configure icmp settings on interfaces that have no addressing configured. I will probably change this and push into this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/metal-automation Automation processes within the Metal project.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants