[dcaas] Implement Bundle Interfaces by sven-rosenzweig · Pull Request #145 · ironcore-dev/network-operator

sven-rosenzweig · 2025-12-18T13:04:16Z

No description provided.

felix-kaestner · 2026-02-13T14:05:16Z

config/crd/bases/networking.metal.ironcore.dev_interfaces.yaml

Can you please invoke make charts to also regenerate the Helm Chart incl. this CRD.

felix-kaestner · 2026-02-13T14:06:20Z

internal/provider/cisco/gnmiext/v2/empty.go

-	if string(b) != "[null]" {
+
+	// Due to some Cisco IOSX ouptut we also match [ \n null \n]
+	nullTypeRe := regexp.MustCompile(`^\[\s*null\s*]$`)


nit: can you move this variable outside of the function, so that we don't recreated this regexp instance on every function call?

felix-kaestner · 2026-02-13T14:06:35Z

internal/provider/cisco/iosxr/intf.go

+	EtherBundle IFaceSpeed = "etherbundle"
+)
+
+type BunldePortActivity string


felix-kaestner · 2026-02-13T14:11:14Z

internal/provider/cisco/iosxr/intf.go

+	StateShutDown  PhysIfStateType = "im-state-shutdown"
+)
+
+// Represent physical and bundle interfaces as part of the same struct as they share a lot of common configuration


Suggested change

// Represent physical and bundle interfaces as part of the same struct as they share a lot of common configuration

// Iface represents physical and bundle interfaces as part of the same struct as they share a lot of common configuration

Type comments should start with the type name.

felix-kaestner · 2026-02-13T14:11:48Z

internal/provider/cisco/gnmiext/v2/empty.go

 	}
-	if string(b) != "[null]" {
+
+	// Due to some Cisco IOSX ouptut we also match [ \n null \n]


Suggested change

// Due to some Cisco IOSX ouptut we also match [ \n null \n]

// Due to some Cisco IOS-XR output we also match [ \n null \n]

felix-kaestner · 2026-02-13T14:12:14Z

internal/provider/cisco/iosxr/provider_test.go

+func (m *MockClient) Create(ctx context.Context, conf ...gnmiext.Configurable) error {
+	return nil
+}
+


I think this was left by mistake and should be removed.

felix-kaestner · 2026-02-13T14:16:11Z

internal/util/ip.go

Please don't introduce a util package for reasons explained in the Google Go Styleguide ¹.

At the same time, as both functions are pretty simple and could be just oneline statements, I would just inline them in the only place they are currently used.

Footnotes

https://google.github.io/styleguide/go/decisions#naming:~:text=Avoid%20uninformative%20package%20names%20like%20util%2C%20utility%2C%20common%2C%20helper%2C%20model%2C%20testhelper%2C%20and%20so%20on%20that%20would%20tempt%20users%20of%20the%20package%20to%20rename%20it%20when%20importing%2E%20See. ↩

felix-kaestner · 2026-03-31T08:08:54Z

internal/provider/cisco/gnmiext/v2/client.go

+
 		err = c.GetConfig(ctx, got)
-		if err != nil && !errors.Is(err, ErrNil) {
-			return fmt.Errorf("gnmiext: failed to retrieve current config for %s: %w", cf.XPath(), err)
-		}
-		// If the current configuration is equal to the desired configuration, skip the update.
-		// This avoids unnecessary updates and potential disruptions.
-		if err == nil && reflect.DeepEqual(cf, got) {
-			c.logger.V(1).Info("Configuration is already up-to-date", "path", cf.XPath())
-			continue
+
+		// If the current configuration does not exist, continue to set the desired configuration.
+		if status.Code(err) != codes.NotFound {
+			if err != nil && !errors.Is(err, ErrNil) {
+				return fmt.Errorf("gnmiext: failed to retrieve current config for %s: %w", cf.XPath(), err)
+			}
+			// If the current configuration is equal to the desired configuration, skip the update.
+			// This avoids unnecessary updates and potential disruptions.
+			if err == nil && reflect.DeepEqual(cf, got) {
+				c.logger.V(1).Info("Configuration is already up-to-date", "path", cf.XPath())
+				continue
+			}
 		}
+


Suggested change

err = c.GetConfig(ctx, got)

if err != nil && !errors.Is(err, ErrNil) {

return fmt.Errorf("gnmiext: failed to retrieve current config for %s: %w", cf.XPath(), err)

}

// If the current configuration is equal to the desired configuration, skip the update.

// This avoids unnecessary updates and potential disruptions.

if err == nil && reflect.DeepEqual(cf, got) {

c.logger.V(1).Info("Configuration is already up-to-date", "path", cf.XPath())

continue

// If the current configuration does not exist, continue to set the desired configuration.

if status.Code(err) != codes.NotFound {

if err != nil && !errors.Is(err, ErrNil) {

return fmt.Errorf("gnmiext: failed to retrieve current config for %s: %w", cf.XPath(), err)

}

// If the current configuration is equal to the desired configuration, skip the update.

// This avoids unnecessary updates and potential disruptions.

if err == nil && reflect.DeepEqual(cf, got) {

c.logger.V(1).Info("Configuration is already up-to-date", "path", cf.XPath())

continue

}

}

err = c.GetConfig(ctx, got)

if err != nil && !errors.Is(err, ErrNil) && status.Code(err) != codes.NotFound {

return fmt.Errorf("gnmiext: failed to retrieve current config for %s: %w", cf.XPath(), err)

}

// If the current configuration is equal to the desired configuration, skip the update.

// This avoids unnecessary updates and potential disruptions.

if err == nil && reflect.DeepEqual(cf, got) {

c.logger.V(1).Info("Configuration is already up-to-date", "path", cf.XPath())

continue

}

why not just add status.Code(err) != codes.NotFound to the existing if case?

If i remember correctly, I wanted to skip the DeepEqual, but your suggestions improves code readability, which I like. So I am going with that.

internal/provider/cisco/gnmiext/v2/empty.go

In Cisco IOS XR the yang empty type is not implemented correctly for bundle-interfaces. Instead of returning "[null]" as defined in the RFC, "[\n null \n]" is. We simply work around of this.

For Bundle- and Bundlesubinterfaces creation fails, as the gnmi Update/Patch call, checks whether the object exists or not. We skip this check once if the gnmi call returns a NotFound error code.

github-actions · 2026-04-01T14:50:32Z

Merging this branch will increase overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2	90.62% (ø)
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr	49.12% (+1.02%)	👍

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2/client.go	88.61% (ø)	158	140	18
github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2/empty.go	100.00% (ø)	10	10	0
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/intf.go	63.33% (+17.18%)	60 (+47)	38 (+32)	22 (+15)	🎉
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/provider.go	41.44% (-7.04%)	111 (+45)	46 (+14)	65 (+31)	👎

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.

Changed unit test files

github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/intf_test.go

sven-rosenzweig requested a review from a team as a code owner December 18, 2025 13:04

hardikdr added the area/metal-automation Automation processes within the Metal project. label Jan 13, 2026

hardikdr added this to Roadmap Jan 13, 2026

felix-kaestner force-pushed the main branch 2 times, most recently from 64984b9 to f71555b Compare January 15, 2026 10:26

sven-rosenzweig force-pushed the feat/bundleiface branch from fbec789 to 3708c30 Compare January 21, 2026 11:40

felix-kaestner force-pushed the main branch 4 times, most recently from aed4596 to 8d57167 Compare January 26, 2026 13:42

sven-rosenzweig force-pushed the feat/bundleiface branch 3 times, most recently from ef97c0e to 6789276 Compare February 6, 2026 13:31

felix-kaestner reviewed Feb 13, 2026

View reviewed changes

hardikdr added area/switch-automation Automation processes for network switch management and operations. and removed area/metal-automation Automation processes within the Metal project. labels Mar 23, 2026

sven-rosenzweig force-pushed the feat/bundleiface branch 4 times, most recently from 3af44d9 to 6b3940f Compare March 27, 2026 09:10

sven-rosenzweig mentioned this pull request Mar 27, 2026

feat: Implement DeviceProvider for IOS-XR #263

Draft

sven-rosenzweig force-pushed the feat/bundleiface branch 2 times, most recently from f8c1dcc to cda5b26 Compare March 30, 2026 14:30

felix-kaestner reviewed Mar 31, 2026

View reviewed changes

sven-rosenzweig force-pushed the feat/bundleiface branch from 0986b0b to 597e61c Compare April 1, 2026 09:28

sven-rosenzweig added 2 commits April 1, 2026 11:32

(fix): Cisco IOS XR Yang Empty Data Type

8d4dfef

In Cisco IOS XR the yang empty type is not implemented correctly for bundle-interfaces. Instead of returning "[null]" as defined in the RFC, "[\n null \n]" is. We simply work around of this.

(fix): Handle none existing config in case of creation

edba772

For Bundle- and Bundlesubinterfaces creation fails, as the gnmi Update/Patch call, checks whether the object exists or not. We skip this check once if the gnmi call returns a NotFound error code.

sven-rosenzweig force-pushed the feat/bundleiface branch from 597e61c to 274259e Compare April 1, 2026 09:32

(feat): Cisco IOS XR implement bundle interface configuration

1f094cd

sven-rosenzweig force-pushed the feat/bundleiface branch from 274259e to 1f094cd Compare April 1, 2026 14:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dcaas] Implement Bundle Interfaces #145

[dcaas] Implement Bundle Interfaces #145
sven-rosenzweig wants to merge 3 commits intomainfrom
feat/bundleiface

sven-rosenzweig commented Dec 18, 2025

Uh oh!

felix-kaestner Feb 13, 2026

Uh oh!

felix-kaestner Feb 13, 2026

Uh oh!

felix-kaestner Feb 13, 2026

Uh oh!

felix-kaestner Feb 13, 2026

Uh oh!

felix-kaestner Feb 13, 2026

Uh oh!

felix-kaestner Feb 13, 2026

Uh oh!

felix-kaestner Feb 13, 2026

Uh oh!

felix-kaestner Mar 31, 2026

Uh oh!

sven-rosenzweig Apr 1, 2026

Uh oh!

Uh oh!

github-actions bot commented Apr 1, 2026

Changed files (no unit tests)

Changed unit test files

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	// Represent physical and bundle interfaces as part of the same struct as they share a lot of common configuration
	// Iface represents physical and bundle interfaces as part of the same struct as they share a lot of common configuration

	// Due to some Cisco IOSX ouptut we also match [ \n null \n]
	// Due to some Cisco IOS-XR output we also match [ \n null \n]

Conversation

sven-rosenzweig commented Dec 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Apr 1, 2026

Merging this branch will increase overall coverage

Changed files (no unit tests)

Changed unit test files

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants