From 07506770fdb082ead325dd5a4989e2008e33b05f Mon Sep 17 00:00:00 2001 From: Pujol Date: Wed, 2 Sep 2026 16:49:22 +0200 Subject: [PATCH 1/2] Rework DHCP relay API as an interface-bound resource Introduce per-interface configuration for the DHCPRelay relay (previously all interfaces were configured equally with one object). With this change the DHCPRelay is keyed with a Device and an Interface reference. This brings more flexibility to high-level resources. As per code project agreement the previous API is mantained and marked as deprecated: added validation hooks to return a warning if used. CEL rules prevent mixed used of deprecated and new format. The migration path for existing DHCPRelay objects that use the deprecated InterfaceRefs field requires deleting those objects and creating new ones using the new fields. Objects using the deprecated field InterfaceRefs likely replace the entire DHCPRelay tree and thus would remove configuration for all interfaces, even for those that are not referenced. Replace the interfaces list with a singular interfaceRef. DHCP server and VRF settings now live on the DHCPRelay resource, allowing each resource to configure a single interface. Update the controller and NX-OS provider to reconcile and delete relay configuration per interface, allowing multiple DHCPRelay resources on a device while rejecting duplicates for the same interface. Remove the status update, update samples/CRDs, and cover both the preferred and deprecated APIs in controller and gNMI tests. Signed-off-by: Pujol --- PROJECT | 3 + Tiltfile | 2 +- api/core/v1alpha1/dhcprelay_types.go | 27 +- api/core/v1alpha1/zz_generated.deepcopy.go | 10 +- ...prelays.networking.metal.ironcore.dev.yaml | 47 +- .../validating-webhook-configuration.yaml | 20 + cmd/main.go | 5 + ...working.metal.ironcore.dev_dhcprelays.yaml | 47 +- config/samples/v1alpha1_dhcprelay.yaml | 109 +- config/webhook/manifests.yaml | 20 + docs/api-reference/index.md | 12 +- .../controller/core/dhcprelay_controller.go | 219 ++-- .../dhcprelay_controller_deprecated_test.go | 80 ++ .../core/dhcprelay_controller_test.go | 1005 ++++++++--------- internal/controller/core/suite_test.go | 77 +- internal/provider/cisco/nxos/dhcprelay.go | 11 +- internal/provider/cisco/nxos/provider.go | 81 +- internal/provider/provider.go | 10 +- .../core/v1alpha1/dhcprelay_webhook.go | 61 + .../core/v1alpha1/dhcprelay_webhook_test.go | 59 + .../core/v1alpha1/webhook_suite_test.go | 3 + .../testdata/cisco-nxos-gnmi/dhcprelay.txtar | 136 ++- .../dhcprelay_deprecated.txtar | 315 ++++++ 23 files changed, 1580 insertions(+), 779 deletions(-) create mode 100644 internal/controller/core/dhcprelay_controller_deprecated_test.go create mode 100644 internal/webhook/core/v1alpha1/dhcprelay_webhook.go create mode 100644 internal/webhook/core/v1alpha1/dhcprelay_webhook_test.go create mode 100644 test/gnmi/testdata/cisco-nxos-gnmi/dhcprelay_deprecated.txtar diff --git a/PROJECT b/PROJECT index be2d8eb2d..c5a6f881d 100644 --- a/PROJECT +++ b/PROJECT @@ -293,6 +293,9 @@ resources: kind: DHCPRelay path: github.com/ironcore-dev/network-operator/api/core/v1alpha1 version: v1alpha1 + webhooks: + validation: true + webhookVersion: v1 - api: crdVersion: v1 namespaced: true diff --git a/Tiltfile b/Tiltfile index 75a8f3a23..dd3c0d9eb 100644 --- a/Tiltfile +++ b/Tiltfile @@ -168,7 +168,7 @@ k8s_resource(new_name='lldp', objects=['leaf1-lldp:lldp'], trigger_mode=TRIGGER_ # k8s_resource(new_name='lldpconfig', objects=['leaf1-lldpconfig:lldpconfig'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) k8s_yaml('./config/samples/v1alpha1_dhcprelay.yaml') -k8s_resource(new_name='dhcprelay', objects=['dhcprelay:dhcprelay'], resource_deps=['eth1-1'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) +k8s_resource(new_name='dhcprelays', objects=['dhcp-vrf:vrf', 'vlan100:vlan', 'vlan200:vlan', 'svi100:interface', 'svi200:interface', 'dhcprelay100:dhcprelay', 'dhcprelay200:dhcprelay'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) k8s_yaml('./config/samples/v1alpha1_ethernetsegment.yaml') k8s_resource(new_name='ethernetsegment-sample', objects=['ethernetsegment-sample:ethernetsegment'], resource_deps=['po10'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples']) diff --git a/api/core/v1alpha1/dhcprelay_types.go b/api/core/v1alpha1/dhcprelay_types.go index a418efd8f..3fdc07176 100644 --- a/api/core/v1alpha1/dhcprelay_types.go +++ b/api/core/v1alpha1/dhcprelay_types.go @@ -11,8 +11,11 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) -// DHCPRelaySpec defines the desired state of DHCPRelay. -// Only a single DHCPRelay resource should be created per Device, the controller will reject additional resources of this type with the same DeviceRef. +// DHCPRelaySpec defines the desired state of the DHCPRelay configuration for a single interface. +// The migration path for existing DHCPRelay objects that use the deprecated InterfaceRefs field requires deleting those objects +// and creating new ones using the new fields. Objects using the deprecated field InterfaceRefs likely replace the entire DHCPRelay tree and thus +// would remove configuration for all interfaces, even for those that are not referenced. +// +kubebuilder:validation:XValidation:rule="has(self.interfaceRef) != has(self.interfaceRefs)",message="specify either interfaceRef or interfaceRefs, but not both" type DHCPRelaySpec struct { // DeviceRef is a reference to the Device this object belongs to. The Device object must exist in the same namespace. // Immutable. @@ -25,7 +28,14 @@ type DHCPRelaySpec struct { // +optional ProviderConfigRef *TypedLocalObjectReference `json:"providerConfigRef,omitempty"` - // VrfRef is an optional reference to the VRF to use when relaying DHCP messages in all referenced interfaces. + // InterfaceRef is a reference to an interface resource on which to enable DHCP relay. + // Immutable. + // To be made non-pointer object and required once we remove the deprecated fields. + // +optional + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="InterfaceRef is immutable" + InterfaceRef *LocalObjectReference `json:"interfaceRef,omitempty"` + + // VrfRef is an optional reference to the VRF to use when relaying DHCP messages in the referenced interface(s). // +optional VrfRef *LocalObjectReference `json:"vrfRef,omitempty"` @@ -37,8 +47,8 @@ type DHCPRelaySpec struct { // +kubebuilder:validation:MinItems=1 Servers []string `json:"servers"` - // InterfaceRefs is a list of interfaces - // +required + // Deprecated: Use field Interfaces instead. + // +optional // +listType=atomic // +kubebuilder:validation:MinItems=1 InterfaceRefs []LocalObjectReference `json:"interfaceRefs,omitempty"` @@ -62,11 +72,6 @@ type DHCPRelayStatus struct { // +listMapKey=type // +optional Conditions []metav1.Condition `json:"conditions,omitempty"` - - // ConfiguredInterfaces contains the names of Interface resources that have DHCP relay configured as known by the device. - // +optional - // +listType=atomic - ConfiguredInterfaces []string `json:"configuredInterfaces,omitempty"` } // +kubebuilder:object:root=true @@ -74,6 +79,8 @@ type DHCPRelayStatus struct { // +kubebuilder:resource:path=dhcprelays // +kubebuilder:resource:singular=dhcprelay // +kubebuilder:printcolumn:name="Device",type=string,JSONPath=`.spec.deviceRef.name` +// +kubebuilder:printcolumn:name="VRF",type=string,JSONPath=`.spec.vrfRef.name` +// +kubebuilder:printcolumn:name="Servers",type=string,JSONPath=`.spec.servers` // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" diff --git a/api/core/v1alpha1/zz_generated.deepcopy.go b/api/core/v1alpha1/zz_generated.deepcopy.go index 1ab6b9e18..d5ced15bd 100644 --- a/api/core/v1alpha1/zz_generated.deepcopy.go +++ b/api/core/v1alpha1/zz_generated.deepcopy.go @@ -1643,6 +1643,11 @@ func (in *DHCPRelaySpec) DeepCopyInto(out *DHCPRelaySpec) { *out = new(TypedLocalObjectReference) **out = **in } + if in.InterfaceRef != nil { + in, out := &in.InterfaceRef, &out.InterfaceRef + *out = new(LocalObjectReference) + **out = **in + } if in.VrfRef != nil { in, out := &in.VrfRef, &out.VrfRef *out = new(LocalObjectReference) @@ -1680,11 +1685,6 @@ func (in *DHCPRelayStatus) DeepCopyInto(out *DHCPRelayStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ConfiguredInterfaces != nil { - in, out := &in.ConfiguredInterfaces, &out.ConfiguredInterfaces - *out = make([]string, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DHCPRelayStatus. diff --git a/charts/network-operator/templates/crd/dhcprelays.networking.metal.ironcore.dev.yaml b/charts/network-operator/templates/crd/dhcprelays.networking.metal.ironcore.dev.yaml index e6b75612b..800d1c438 100644 --- a/charts/network-operator/templates/crd/dhcprelays.networking.metal.ironcore.dev.yaml +++ b/charts/network-operator/templates/crd/dhcprelays.networking.metal.ironcore.dev.yaml @@ -21,6 +21,12 @@ spec: - jsonPath: .spec.deviceRef.name name: Device type: string + - jsonPath: .spec.vrfRef.name + name: VRF + type: string + - jsonPath: .spec.servers + name: Servers + type: string - jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string @@ -51,8 +57,10 @@ spec: type: object spec: description: |- - DHCPRelaySpec defines the desired state of DHCPRelay. - Only a single DHCPRelay resource should be created per Device, the controller will reject additional resources of this type with the same DeviceRef. + DHCPRelaySpec defines the desired state of the DHCPRelay configuration for a single interface. + The migration path for existing DHCPRelay objects that use the deprecated InterfaceRefs field requires deleting those objects + and creating new ones using the new fields. Objects using the deprecated field InterfaceRefs likely replace the entire DHCPRelay tree and thus + would remove configuration for all interfaces, even for those that are not referenced. properties: deviceRef: description: |- @@ -73,8 +81,28 @@ spec: x-kubernetes-validations: - message: DeviceRef is immutable rule: self == oldSelf + interfaceRef: + description: |- + InterfaceRef is a reference to an interface resource on which to enable DHCP relay. + Immutable. + To be made non-pointer object and required once we remove the deprecated fields. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + maxLength: 63 + minLength: 1 + type: string + required: + - name + type: object + x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: InterfaceRef is immutable + rule: self == oldSelf interfaceRefs: - description: InterfaceRefs is a list of interfaces + description: 'Deprecated: Use field Interfaces instead.' items: description: |- LocalObjectReference contains enough information to locate a @@ -140,7 +168,7 @@ spec: x-kubernetes-list-type: atomic vrfRef: description: VrfRef is an optional reference to the VRF to use when - relaying DHCP messages in all referenced interfaces. + relaying DHCP messages in the referenced interface(s). properties: name: description: |- @@ -155,9 +183,11 @@ spec: x-kubernetes-map-type: atomic required: - deviceRef - - interfaceRefs - servers type: object + x-kubernetes-validations: + - message: specify either interfaceRef or interfaceRefs, but not both + rule: has(self.interfaceRef) != has(self.interfaceRefs) status: description: DHCPRelayStatus defines the observed state of DHCPRelay. properties: @@ -230,13 +260,6 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map - configuredInterfaces: - description: ConfiguredInterfaces contains the names of Interface - resources that have DHCP relay configured as known by the device. - items: - type: string - type: array - x-kubernetes-list-type: atomic type: object required: - metadata diff --git a/charts/network-operator/templates/webhook/validating-webhook-configuration.yaml b/charts/network-operator/templates/webhook/validating-webhook-configuration.yaml index bb0556186..f080ad35e 100644 --- a/charts/network-operator/templates/webhook/validating-webhook-configuration.yaml +++ b/charts/network-operator/templates/webhook/validating-webhook-configuration.yaml @@ -68,6 +68,26 @@ webhooks: resources: - bgppeers sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: {{ include "network-operator.resourceName" (dict "suffix" "webhook-service" "context" $) }} + namespace: {{ .Release.Namespace }} + path: /validate-networking-metal-ironcore-dev-v1alpha1-dhcprelay + failurePolicy: Fail + name: dhcprelay-v1alpha1.kb.io + rules: + - apiGroups: + - networking.metal.ironcore.dev + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - dhcprelays + sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/cmd/main.go b/cmd/main.go index 92b2af85c..b4dc8b9f5 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -774,6 +774,11 @@ func main() { //nolint:gocyclo os.Exit(1) } + if err := webhookv1alpha1.SetupDHCPRelayWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "Failed to create webhook", "webhook", "DHCPRelay") + os.Exit(1) + } + if err := webhooknxv1alpha1.SetupNetworkVirtualizationEdgeConfigWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "NetworkVirtualizationEdgeConfig") os.Exit(1) diff --git a/config/crd/bases/networking.metal.ironcore.dev_dhcprelays.yaml b/config/crd/bases/networking.metal.ironcore.dev_dhcprelays.yaml index 48cefc42c..bf6fad526 100644 --- a/config/crd/bases/networking.metal.ironcore.dev_dhcprelays.yaml +++ b/config/crd/bases/networking.metal.ironcore.dev_dhcprelays.yaml @@ -18,6 +18,12 @@ spec: - jsonPath: .spec.deviceRef.name name: Device type: string + - jsonPath: .spec.vrfRef.name + name: VRF + type: string + - jsonPath: .spec.servers + name: Servers + type: string - jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string @@ -48,8 +54,10 @@ spec: type: object spec: description: |- - DHCPRelaySpec defines the desired state of DHCPRelay. - Only a single DHCPRelay resource should be created per Device, the controller will reject additional resources of this type with the same DeviceRef. + DHCPRelaySpec defines the desired state of the DHCPRelay configuration for a single interface. + The migration path for existing DHCPRelay objects that use the deprecated InterfaceRefs field requires deleting those objects + and creating new ones using the new fields. Objects using the deprecated field InterfaceRefs likely replace the entire DHCPRelay tree and thus + would remove configuration for all interfaces, even for those that are not referenced. properties: deviceRef: description: |- @@ -70,8 +78,28 @@ spec: x-kubernetes-validations: - message: DeviceRef is immutable rule: self == oldSelf + interfaceRef: + description: |- + InterfaceRef is a reference to an interface resource on which to enable DHCP relay. + Immutable. + To be made non-pointer object and required once we remove the deprecated fields. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + maxLength: 63 + minLength: 1 + type: string + required: + - name + type: object + x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: InterfaceRef is immutable + rule: self == oldSelf interfaceRefs: - description: InterfaceRefs is a list of interfaces + description: 'Deprecated: Use field Interfaces instead.' items: description: |- LocalObjectReference contains enough information to locate a @@ -137,7 +165,7 @@ spec: x-kubernetes-list-type: atomic vrfRef: description: VrfRef is an optional reference to the VRF to use when - relaying DHCP messages in all referenced interfaces. + relaying DHCP messages in the referenced interface(s). properties: name: description: |- @@ -152,9 +180,11 @@ spec: x-kubernetes-map-type: atomic required: - deviceRef - - interfaceRefs - servers type: object + x-kubernetes-validations: + - message: specify either interfaceRef or interfaceRefs, but not both + rule: has(self.interfaceRef) != has(self.interfaceRefs) status: description: DHCPRelayStatus defines the observed state of DHCPRelay. properties: @@ -227,13 +257,6 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map - configuredInterfaces: - description: ConfiguredInterfaces contains the names of Interface - resources that have DHCP relay configured as known by the device. - items: - type: string - type: array - x-kubernetes-list-type: atomic type: object required: - metadata diff --git a/config/samples/v1alpha1_dhcprelay.yaml b/config/samples/v1alpha1_dhcprelay.yaml index 40b8b68cb..5a618106b 100644 --- a/config/samples/v1alpha1_dhcprelay.yaml +++ b/config/samples/v1alpha1_dhcprelay.yaml @@ -1,16 +1,115 @@ apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: VRF +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + networking.metal.ironcore.dev/device-name: leaf1 + name: dhcp-vrf +spec: + deviceRef: + name: leaf1 + name: DHCP-VRF-GREEN +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: VLAN +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + networking.metal.ironcore.dev/device-name: leaf1 + name: vlan100 +spec: + deviceRef: + name: leaf1 + id: 100 + name: DHCP-VLAN1 +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: VLAN +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + networking.metal.ironcore.dev/device-name: leaf1 + name: vlan200 +spec: + deviceRef: + name: leaf1 + id: 200 + name: DHCP-VLAN2 +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: Interface +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + networking.metal.ironcore.dev/device-name: leaf1 + name: svi100 +spec: + deviceRef: + name: leaf1 + name: Vlan100 + adminState: Up + type: RoutedVLAN + vlanRef: + name: vlan100 + ipv4: + addresses: + - 192.168.100.1/24 +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: Interface +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + networking.metal.ironcore.dev/device-name: leaf1 + name: svi200 +spec: + deviceRef: + name: leaf1 + name: Vlan200 + adminState: Up + type: RoutedVLAN + vlanRef: + name: vlan200 + ipv4: + addresses: + - 192.168.101.1/24 +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: DHCPRelay +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + networking.metal.ironcore.dev/device-name: leaf1 + name: dhcprelay100 +spec: + deviceRef: + name: leaf1 + interfaceRef: + name: svi100 + servers: + - "10.0.0.10" +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 kind: DHCPRelay metadata: labels: app.kubernetes.io/name: network-operator app.kubernetes.io/managed-by: kustomize networking.metal.ironcore.dev/device-name: leaf1 - name: dhcprelay + name: dhcprelay200 spec: deviceRef: name: leaf1 + interfaceRef: + name: svi200 + vrfRef: + name: dhcp-vrf servers: - - 192.168.1.3 - - 192.168.1.4 - interfaceRefs: - - name: eth1-1 + - "10.0.0.10" + - "10.0.1.10" diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 463cf758f..cf5ae4a31 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -64,6 +64,26 @@ webhooks: resources: - bgppeers sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-networking-metal-ironcore-dev-v1alpha1-dhcprelay + failurePolicy: Fail + name: dhcprelay-v1alpha1.kb.io + rules: + - apiGroups: + - networking.metal.ironcore.dev + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - dhcprelays + sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/docs/api-reference/index.md b/docs/api-reference/index.md index ed271ca0e..0d92da540 100644 --- a/docs/api-reference/index.md +++ b/docs/api-reference/index.md @@ -1664,8 +1664,10 @@ DHCPRelay is the Schema for the DHCPRelays API -DHCPRelaySpec defines the desired state of DHCPRelay. -Only a single DHCPRelay resource should be created per Device, the controller will reject additional resources of this type with the same DeviceRef. +DHCPRelaySpec defines the desired state of the DHCPRelay configuration for a single interface. +The migration path for existing DHCPRelay objects that use the deprecated InterfaceRefs field requires deleting those objects +and creating new ones using the new fields. Objects using the deprecated field InterfaceRefs likely replace the entire DHCPRelay tree and thus +would remove configuration for all interfaces, even for those that are not referenced. @@ -1676,9 +1678,10 @@ _Appears in:_ | --- | --- | --- | --- | | `deviceRef` _[LocalObjectReference](#localobjectreference)_ | DeviceRef is a reference to the Device this object belongs to. The Device object must exist in the same namespace.
Immutable. | | Required: \{\}
| | `providerConfigRef` _[TypedLocalObjectReference](#typedlocalobjectreference)_ | ProviderConfigRef is a reference to a resource holding the provider-specific configuration for this DHCPRelay.
If not specified the provider applies the target platform's default settings. | | Optional: \{\}
| -| `vrfRef` _[LocalObjectReference](#localobjectreference)_ | VrfRef is an optional reference to the VRF to use when relaying DHCP messages in all referenced interfaces. | | Optional: \{\}
| +| `interfaceRef` _[LocalObjectReference](#localobjectreference)_ | InterfaceRef is a reference to an interface resource on which to enable DHCP relay.
Immutable.
To be made non-pointer object and required once we remove the deprecated fields. | | Optional: \{\}
| +| `vrfRef` _[LocalObjectReference](#localobjectreference)_ | VrfRef is an optional reference to the VRF to use when relaying DHCP messages in the referenced interface(s). | | Optional: \{\}
| | `servers` _string array_ | Servers is a list of DHCP server addresses to which DHCP messages will be relayed.
Only IPv4 addresses are currently supported. | | MinItems: 1
items:Format: ipv4
Required: \{\}
| -| `interfaceRefs` _[LocalObjectReference](#localobjectreference) array_ | InterfaceRefs is a list of interfaces | | MinItems: 1
Required: \{\}
| +| `interfaceRefs` _[LocalObjectReference](#localobjectreference) array_ | Deprecated: Use field Interfaces instead. | | MinItems: 1
Optional: \{\}
| #### DHCPRelayStatus @@ -1695,7 +1698,6 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#condition-v1-meta) array_ | conditions represent the current state of the DHCPRelay resource.
Each condition has a unique type and reflects the status of a specific aspect of the resource.
Standard condition types include:
- "Available": the resource is fully functional
- "Progressing": the resource is being created or updated
- "Degraded": the resource failed to reach or maintain its desired state
The status of each condition is one of True, False, or Unknown. | | Optional: \{\}
| -| `configuredInterfaces` _string array_ | ConfiguredInterfaces contains the names of Interface resources that have DHCP relay configured as known by the device. | | Optional: \{\}
| #### DNS diff --git a/internal/controller/core/dhcprelay_controller.go b/internal/controller/core/dhcprelay_controller.go index ec88c2a47..0db884916 100644 --- a/internal/controller/core/dhcprelay_controller.go +++ b/internal/controller/core/dhcprelay_controller.go @@ -230,22 +230,41 @@ func (r *DHCPRelayReconciler) reconcile(ctx context.Context, s *dhcprelayScope) } } - if err := r.validateUniqueResourcePerDevice(ctx, s); err != nil { - return err - } + defer func() { + conditions.RecomputeReady(s.DHCPRelay) + }() if err := r.validateProviderConfigRef(ctx, s); err != nil { return err } - interfaces, err := r.reconcileInterfaceRefs(ctx, s) - if err != nil { - return err + req := provider.DHCPRelayRequest{ + DHCPRelay: s.DHCPRelay, + ProviderConfig: s.ProviderConfig, } - var vrf *v1alpha1.VRF + var err error if s.DHCPRelay.Spec.VrfRef != nil { - vrf, err = r.reconcileVRFRef(ctx, s) + if req.VRF, err = r.reconcileVRFRef(ctx, *s.DHCPRelay.Spec.VrfRef, s); err != nil { + return err + } + } + + if err := r.validateUniqueResource(ctx, s); err != nil { + return err + } + + // preferred path, TODO: remove guard after removing deprecated fields + if s.DHCPRelay.Spec.InterfaceRef != nil { + req.Interface, err = r.reconcileInterfaceRef(ctx, *s.DHCPRelay.Spec.InterfaceRef, s) + if err != nil { + return err + } + } + + // deprecated + if len(s.DHCPRelay.Spec.InterfaceRefs) > 0 { //nolint:staticcheck + req.Interfaces, err = r.reconcileInterfaceRefs(ctx, s) if err != nil { return err } @@ -262,35 +281,13 @@ func (r *DHCPRelayReconciler) reconcile(ctx context.Context, s *dhcprelayScope) }() // Ensure the DHCPRelay is realized on the remote device. - err = s.Provider.EnsureDHCPRelay(ctx, &provider.DHCPRelayRequest{ - DHCPRelay: s.DHCPRelay, - ProviderConfig: s.ProviderConfig, - Interfaces: interfaces, - VRF: vrf, - }) + err = s.Provider.EnsureDHCPRelay(ctx, &req) cond := conditions.FromError(err) - // As this resource is configuration only, we use the Configured condition as top-level Ready condition. - cond.Type = v1alpha1.ReadyCondition + cond.Type = v1alpha1.ConfiguredCondition conditions.Set(s.DHCPRelay, cond) - if err != nil { - return err - } - - // Retrieve and update the status from the device; this include the list of interfaces that are actually configured on the device. - status, err := s.Provider.GetDHCPRelayStatus(ctx, &provider.DHCPRelayRequest{ - DHCPRelay: s.DHCPRelay, - ProviderConfig: s.ProviderConfig, - Interfaces: interfaces, - }) - if err != nil { - return fmt.Errorf("failed to get DHCP relay status: %w", err) - } - - s.DHCPRelay.Status.ConfiguredInterfaces = status.ConfiguredInterfaces - - return nil + return err } // SetupWithManager sets up the controller with the Manager. @@ -377,8 +374,7 @@ func (r *DHCPRelayReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man UpdateFunc: func(e event.UpdateEvent) bool { oldVRF := e.ObjectOld.(*v1alpha1.VRF) newVRF := e.ObjectNew.(*v1alpha1.VRF) - // Only trigger when Configured condition changes (not operational status). - return conditions.IsConfigured(oldVRF) != conditions.IsConfigured(newVRF) + return conditions.IsReady(oldVRF) != conditions.IsReady(newVRF) }, GenericFunc: func(e event.GenericEvent) bool { return false @@ -406,7 +402,6 @@ func (r *DHCPRelayReconciler) validateProviderConfigRef(_ context.Context, s *dh } gvk := gv.WithKind(s.DHCPRelay.Spec.ProviderConfigRef.Kind) - if ok := slices.Contains(v1alpha1.DHCPRelayDependencies, gvk); !ok { conditions.Set(s.DHCPRelay, metav1.Condition{ Type: v1alpha1.ConfiguredCondition, @@ -420,19 +415,58 @@ func (r *DHCPRelayReconciler) validateProviderConfigRef(_ context.Context, s *dh return nil } +// validateUniqueResource checks that there is only one DHCPRelay resource per interface +// It also checks if another resource with the deprecated InterfaceRefs field exists on the same device +func (r *DHCPRelayReconciler) validateUniqueResource(ctx context.Context, s *dhcprelayScope) error { + var list v1alpha1.DHCPRelayList + if err := r.List( + ctx, &list, + client.InNamespace(s.DHCPRelay.Namespace), + client.MatchingFields{v1alpha1.DeviceRefIndexKey: s.Device.Name}, + ); err != nil { + return err + } + + for _, dhcprelay := range list.Items { + // refuse to reconcile if another DHCPRelay exists that uses deprecated paths exists ont he same device + if dhcprelay.Name != s.DHCPRelay.Name && + len(dhcprelay.Spec.InterfaceRefs) > 0 && s.DHCPRelay.Spec.DeviceRef.Name == dhcprelay.Spec.DeviceRef.Name { //nolint:staticcheck + conditions.Set(s.DHCPRelay, metav1.Condition{ + Type: v1alpha1.ConfiguredCondition, + Status: metav1.ConditionFalse, + Reason: v1alpha1.DuplicateResourceOnDevice, + Message: fmt.Sprintf("Another DHCPRelay (%s) using deprecated field .spec.InterfaceRefs already exists for interface %s, this migration path is not supported.", dhcprelay.Name, s.DHCPRelay.Spec.InterfaceRef.Name), + }) + return reconcile.TerminalError(fmt.Errorf("only one DHCPRelay resource allowed per interface (%s)", s.DHCPRelay.Spec.InterfaceRef.Name)) + } + if dhcprelay.Name != s.DHCPRelay.Name && s.DHCPRelay.Spec.InterfaceRef != nil && + dhcprelay.Spec.InterfaceRef.Name == s.DHCPRelay.Spec.InterfaceRef.Name { + conditions.Set(s.DHCPRelay, metav1.Condition{ + Type: v1alpha1.ConfiguredCondition, + Status: metav1.ConditionFalse, + Reason: v1alpha1.DuplicateResourceOnDevice, + Message: fmt.Sprintf("Another DHCPRelay (%s) already exists for interface %s", dhcprelay.Name, s.DHCPRelay.Spec.InterfaceRef.Name), + }) + return reconcile.TerminalError(fmt.Errorf("only one DHCPRelay resource allowed per interface (%s)", s.DHCPRelay.Spec.InterfaceRef.Name)) + } + } + return nil +} + // reconcileInterfaceRefs fetches all referenced interfaces and validates them -func (r *DHCPRelayReconciler) reconcileInterfaceRefs(ctx context.Context, s *dhcprelayScope) ([]*v1alpha1.Interface, error) { - if len(s.DHCPRelay.Spec.InterfaceRefs) == 0 { +// used for the deprecated InterfaceRefs field +func (r *DHCPRelayReconciler) reconcileInterfaceRefs(ctx context.Context, s *dhcprelayScope) ([]v1alpha1.Interface, error) { + if len(s.DHCPRelay.Spec.InterfaceRefs) == 0 { //nolint:staticcheck return nil, nil } - interfaces := make([]*v1alpha1.Interface, 0, len(s.DHCPRelay.Spec.InterfaceRefs)) - for _, ifRef := range s.DHCPRelay.Spec.InterfaceRefs { + interfaces := make([]v1alpha1.Interface, 0, len(s.DHCPRelay.Spec.InterfaceRefs)) //nolint:staticcheck + for _, ifRef := range s.DHCPRelay.Spec.InterfaceRefs { //nolint:staticcheck iface, err := r.reconcileInterfaceRef(ctx, ifRef, s) if err != nil { return nil, err } - interfaces = append(interfaces, iface) + interfaces = append(interfaces, *iface) } return interfaces, nil @@ -506,10 +540,10 @@ func (r *DHCPRelayReconciler) reconcileInterfaceRef(ctx context.Context, interfa return intf, nil } -func (r *DHCPRelayReconciler) reconcileVRFRef(ctx context.Context, s *dhcprelayScope) (*v1alpha1.VRF, error) { +func (r *DHCPRelayReconciler) reconcileVRFRef(ctx context.Context, vrfRef v1alpha1.LocalObjectReference, s *dhcprelayScope) (*v1alpha1.VRF, error) { vrf := new(v1alpha1.VRF) if err := r.Get(ctx, types.NamespacedName{ - Name: s.DHCPRelay.Spec.VrfRef.Name, + Name: vrfRef.Name, Namespace: s.DHCPRelay.Namespace, }, vrf); err != nil { if apierrors.IsNotFound(err) { @@ -517,11 +551,11 @@ func (r *DHCPRelayReconciler) reconcileVRFRef(ctx context.Context, s *dhcprelayS Type: v1alpha1.ConfiguredCondition, Status: metav1.ConditionFalse, Reason: v1alpha1.WaitingForDependenciesReason, - Message: fmt.Sprintf("VRF %s not found", s.DHCPRelay.Spec.VrfRef.Name), + Message: fmt.Sprintf("VRF %s not found", vrfRef.Name), }) - return nil, reconcile.TerminalError(fmt.Errorf("vrf %s not found", s.DHCPRelay.Spec.VrfRef.Name)) + return nil, reconcile.TerminalError(fmt.Errorf("vrf %s not found", vrfRef.Name)) } - return nil, fmt.Errorf("failed to get VRF %s: %w", s.DHCPRelay.Spec.VrfRef.Name, err) + return nil, fmt.Errorf("failed to get VRF %s: %w", vrfRef.Name, err) } // Verify the VRF belongs to the same device @@ -530,48 +564,25 @@ func (r *DHCPRelayReconciler) reconcileVRFRef(ctx context.Context, s *dhcprelayS Type: v1alpha1.ConfiguredCondition, Status: metav1.ConditionFalse, Reason: v1alpha1.CrossDeviceReferenceReason, - Message: fmt.Sprintf("VRF %s belongs to device %s, not %s", s.DHCPRelay.Spec.VrfRef.Name, vrf.Spec.DeviceRef.Name, s.Device.Name), + Message: fmt.Sprintf("VRF %s belongs to device %s, not %s", vrfRef.Name, vrf.Spec.DeviceRef.Name, s.Device.Name), }) - return nil, reconcile.TerminalError(fmt.Errorf("vrf %s belongs to different device", s.DHCPRelay.Spec.VrfRef.Name)) + return nil, reconcile.TerminalError(fmt.Errorf("vrf %s belongs to different device", vrfRef.Name)) } - // Verify the VRF is ready (configured) on the device + // Verify the VRF is configured on the device if !conditions.IsReady(vrf) { conditions.Set(s.DHCPRelay, metav1.Condition{ Type: v1alpha1.ConfiguredCondition, Status: metav1.ConditionFalse, Reason: v1alpha1.WaitingForDependenciesReason, - Message: fmt.Sprintf("VRF %s is not configured on the device", s.DHCPRelay.Spec.VrfRef.Name), + Message: fmt.Sprintf("VRF %s is not configured on the device", vrfRef.Name), }) - return nil, reconcile.TerminalError(fmt.Errorf("vrf %s is not configured", s.DHCPRelay.Spec.VrfRef.Name)) + return nil, reconcile.TerminalError(fmt.Errorf("vrf %s is not configured", vrfRef.Name)) } return vrf, nil } -func (r *DHCPRelayReconciler) validateUniqueResourcePerDevice(ctx context.Context, s *dhcprelayScope) error { - var list v1alpha1.DHCPRelayList - if err := r.List( - ctx, &list, - client.InNamespace(s.DHCPRelay.Namespace), - client.MatchingFields{v1alpha1.DeviceRefIndexKey: s.Device.Name}, - ); err != nil { - return err - } - for _, dhcprelay := range list.Items { - if dhcprelay.Name != s.DHCPRelay.Name { - conditions.Set(s.DHCPRelay, metav1.Condition{ - Type: v1alpha1.ConfiguredCondition, - Status: metav1.ConditionFalse, - Reason: v1alpha1.DuplicateResourceOnDevice, - Message: fmt.Sprintf("Another DHCPRelay (%s) already exists for device %s", dhcprelay.Name, s.DHCPRelay.Spec.DeviceRef.Name), - }) - return reconcile.TerminalError(fmt.Errorf("only one DHCPRelay resource allowed per device (%s)", s.DHCPRelay.Spec.DeviceRef.Name)) - } - } - return nil -} - func (r *DHCPRelayReconciler) mapProviderConfigToDHCPRelay(ctx context.Context, obj client.Object) []reconcile.Request { log := ctrl.LoggerFrom(ctx, "Object", klog.KObj(obj)) @@ -611,10 +622,52 @@ func (r *DHCPRelayReconciler) finalize(ctx context.Context, s *dhcprelayScope) ( } }() - return s.Provider.DeleteDHCPRelay(ctx, &provider.DHCPRelayRequest{ + req := provider.DHCPRelayRequest{ + DHCPRelay: s.DHCPRelay, + ProviderConfig: s.ProviderConfig, + } + + // deprecated path + if len(s.DHCPRelay.Spec.InterfaceRefs) > 0 { //nolint:staticcheck + return s.Provider.DeleteDHCPRelay(ctx, &req) + } + + // Skip finalization if another DHCPRelay exists for the same interface on the same device, e.g., + // a resource rejected by the controller uniqueness will still be finalized and would delete + // the configuration of the other resource. + var list v1alpha1.DHCPRelayList + if err := r.List( + ctx, &list, + client.InNamespace(s.DHCPRelay.Namespace), + client.MatchingFields{v1alpha1.DeviceRefIndexKey: s.Device.Name}, + ); err != nil { + return fmt.Errorf("failed to list DHCPRelays for device: %w", err) + } + for i := range list.Items { + other := &list.Items[i] + if other.Name != s.DHCPRelay.Name && other.DeletionTimestamp.IsZero() && + other.Spec.InterfaceRef != nil && other.Spec.InterfaceRef.Name == s.DHCPRelay.Spec.InterfaceRef.Name { + return nil + } + } + + intf := new(v1alpha1.Interface) + if err := r.Get(ctx, types.NamespacedName{ + Name: s.DHCPRelay.Spec.InterfaceRef.Name, + Namespace: s.DHCPRelay.Namespace, + }, intf); err != nil { + if apierrors.IsNotFound(err) { + // If the interface no longer exists, there is no device config to clean up + return nil + } + return fmt.Errorf("failed to get referenced interface: %w", err) + } + + return s.Provider.DeleteDHCPRelay(ctx, new(provider.DHCPRelayRequest{ DHCPRelay: s.DHCPRelay, ProviderConfig: s.ProviderConfig, - }) + Interface: intf, + })) } // deviceToDHCPRelays is a [handler.MapFunc] to be used to enqueue requests for reconciliation @@ -671,7 +724,19 @@ func (r *DHCPRelayReconciler) interfaceToDHCPRelays(ctx context.Context, obj cli var requests []ctrl.Request for _, dhcpRelay := range list.Items { - for _, ifRef := range dhcpRelay.Spec.InterfaceRefs { + if dhcpRelay.Spec.InterfaceRef != nil && dhcpRelay.Spec.InterfaceRef.Name == intf.Name { + log.V(2).Info("Enqueuing DHCPRelay for reconciliation", "DHCPRelay", klog.KObj(&dhcpRelay)) + requests = append(requests, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Name: dhcpRelay.Name, + Namespace: dhcpRelay.Namespace, + }, + }) + break + } + + // deprecated path + for _, ifRef := range dhcpRelay.Spec.InterfaceRefs { //nolint:staticcheck // deprecated field for backward compatibility if ifRef.Name == intf.Name { log.V(2).Info("Enqueuing DHCPRelay for reconciliation", "DHCPRelay", klog.KObj(&dhcpRelay)) requests = append(requests, ctrl.Request{ diff --git a/internal/controller/core/dhcprelay_controller_deprecated_test.go b/internal/controller/core/dhcprelay_controller_deprecated_test.go new file mode 100644 index 000000000..74868c612 --- /dev/null +++ b/internal/controller/core/dhcprelay_controller_deprecated_test.go @@ -0,0 +1,80 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package core + +import ( + "net/netip" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/ironcore-dev/network-operator/api/core/v1alpha1" +) + +var _ = Describe("DHCPRelay Controller with deprecated API fields", func() { + It("Should reconcile interfaceRefs", func() { + device := &v1alpha1.Device{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-deprecated-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.DeviceSpec{Endpoint: v1alpha1.Endpoint{Address: "192.168.20.50:9339"}}} + Expect(k8sClient.Create(ctx, device)).To(Succeed()) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) + }) + + vlan := &v1alpha1.VLAN{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-deprecated-vlan-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.VLANSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: device.Name}, ID: 70, Name: "vlan70"}} + Expect(k8sClient.Create(ctx, vlan)).To(Succeed()) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vlan))).To(Succeed()) + }) + + intf := &v1alpha1.Interface{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-deprecated-intf-", Namespace: metav1.NamespaceDefault}, + Spec: v1alpha1.InterfaceSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: device.Name}, Name: "vlan70", Type: v1alpha1.InterfaceTypeRoutedVLAN, AdminState: v1alpha1.AdminStateUp, + VlanRef: &v1alpha1.LocalObjectReference{Name: vlan.Name}, IPv4: &v1alpha1.InterfaceIPv4{Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.7.1/24")}}}, + }, + } + Expect(k8sClient.Create(ctx, intf)).To(Succeed()) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed()) + }) + + Eventually(func(g Gomega) { + configuredInterface := &v1alpha1.Interface{} + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(intf), configuredInterface)).To(Succeed()) + condition := meta.FindStatusCondition(configuredInterface.Status.Conditions, v1alpha1.ConfiguredCondition) + g.Expect(condition).NotTo(BeNil()) + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) + + relay := &v1alpha1.DHCPRelay{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-deprecated-", Namespace: metav1.NamespaceDefault}, + Spec: v1alpha1.DHCPRelaySpec{DeviceRef: v1alpha1.LocalObjectReference{Name: device.Name}, InterfaceRefs: []v1alpha1.LocalObjectReference{{Name: intf.Name}}, Servers: []string{"192.168.1.1"}}, + } + Expect(k8sClient.Create(ctx, relay)).To(Succeed()) + relayKey := client.ObjectKeyFromObject(relay) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, relay))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, relayKey, &v1alpha1.DHCPRelay{}))).To(BeTrue()) + g.Expect(testProvider.DHCPRelay).To(BeNil()) + }).Should(Succeed()) + }) + + Eventually(func(g Gomega) { + g.Expect(testProvider.DHCPRelay).ToNot(BeNil()) + g.Expect(testProvider.DHCPRelay.GetName()).To(Equal(relay.Name)) + }).Should(Succeed()) + + Eventually(func(g Gomega) { + configuredRelay := &v1alpha1.DHCPRelay{} + g.Expect(k8sClient.Get(ctx, relayKey, configuredRelay)).To(Succeed()) + condition := meta.FindStatusCondition(configuredRelay.Status.Conditions, v1alpha1.ReadyCondition) + g.Expect(condition).NotTo(BeNil()) + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) + }) +}) diff --git a/internal/controller/core/dhcprelay_controller_test.go b/internal/controller/core/dhcprelay_controller_test.go index e11406d5c..dd40c659d 100644 --- a/internal/controller/core/dhcprelay_controller_test.go +++ b/internal/controller/core/dhcprelay_controller_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -21,21 +22,24 @@ import ( var _ = Describe("DHCPRelay Controller", func() { Context("When reconciling a resource", func() { var ( - deviceName string - resourceName string - interfaceName string - vlanName string - resourceKey client.ObjectKey - deviceKey client.ObjectKey - interfaceKey client.ObjectKey - vlanKey client.ObjectKey - device *v1alpha1.Device - vlan *v1alpha1.VLAN - intf *v1alpha1.Interface - dhcprelay *v1alpha1.DHCPRelay + deviceName string + resourceName string + interfaceName string + vlanName string + resourceKey client.ObjectKey + deviceKey client.ObjectKey + interfaceKey client.ObjectKey + vlanKey client.ObjectKey + device *v1alpha1.Device + vlan *v1alpha1.VLAN + intf *v1alpha1.Interface + dhcprelay *v1alpha1.DHCPRelay + providerConfig *corev1.ConfigMap ) BeforeEach(func() { + providerConfig = nil + By("Creating the custom resource for the Kind Device") device = &v1alpha1.Device{ ObjectMeta: metav1.ObjectMeta{ @@ -112,6 +116,11 @@ var _ = Describe("DHCPRelay Controller", func() { g.Expect(errors.IsNotFound(err)).To(BeTrue()) }).Should(Succeed()) + if providerConfig != nil { + By("Cleaning up the provider configuration resource") + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, providerConfig))).To(Succeed()) + } + By("Cleaning up the Interface resource") intf = &v1alpha1.Interface{} intf.Name = interfaceKey.Name @@ -152,11 +161,9 @@ var _ = Describe("DHCPRelay Controller", func() { Namespace: metav1.NamespaceDefault, }, Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1", "192.168.1.2"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: interfaceName}, - }, + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: interfaceName}, + Servers: []string{"192.168.1.1", "192.168.1.2"}, }, } Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) @@ -196,13 +203,6 @@ var _ = Describe("DHCPRelay Controller", func() { g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) }).Should(Succeed()) - By("Verifying the status contains configured interface refs") - Eventually(func(g Gomega) { - dhcprelay = &v1alpha1.DHCPRelay{} - g.Expect(k8sClient.Get(ctx, resourceKey, dhcprelay)).To(Succeed()) - g.Expect(dhcprelay.Status.ConfiguredInterfaces).To(ContainElement(intf.Spec.Name)) - }).Should(Succeed()) - By("Ensuring the DHCPRelay is created in the provider") Eventually(func(g Gomega) { g.Expect(testProvider.DHCPRelay).ToNot(BeNil(), "Provider DHCPRelay should not be nil") @@ -212,8 +212,17 @@ var _ = Describe("DHCPRelay Controller", func() { }).Should(Succeed()) }) - It("Should reject duplicate DHCPRelay resources on the same device", func() { - By("Creating the first DHCPRelay resource") + It("Should reject an incompatible ProviderConfigRef", func() { + By("Creating an unsupported provider configuration resource") + providerConfig = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-dhcprelay-config-", + Namespace: metav1.NamespaceDefault, + }, + } + Expect(k8sClient.Create(ctx, providerConfig)).To(Succeed()) + + By("Creating a DHCPRelay that references the unsupported configuration") dhcprelay = &v1alpha1.DHCPRelay{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-dhcprelay-", @@ -221,10 +230,100 @@ var _ = Describe("DHCPRelay Controller", func() { }, Spec: v1alpha1.DHCPRelaySpec{ DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: interfaceName}, + ProviderConfigRef: &v1alpha1.TypedLocalObjectReference{ + APIVersion: "v1", + Kind: "ConfigMap", + Name: providerConfig.Name, }, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: interfaceName}, + Servers: []string{"192.168.1.1"}, + }, + } + Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) + resourceName = dhcprelay.Name + resourceKey = client.ObjectKey{Name: resourceName, Namespace: metav1.NamespaceDefault} + + By("Verifying the incompatible configuration is rejected") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, resourceKey, dhcprelay)).To(Succeed()) + cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ConfiguredCondition) + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(v1alpha1.IncompatibleProviderConfigRef)) + g.Expect(testProvider.DHCPRelay).To(BeNil()) + }).Should(Succeed()) + }) + + It("Should successfully reconcile using a top-level VRF", func() { + By("Creating a VRF resource") + vrf := &v1alpha1.VRF{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-dhcprelay-vrf-", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.VRFSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + Name: "VRF-TEST", + }, + } + Expect(k8sClient.Create(ctx, vrf)).To(Succeed()) + vrfKey := client.ObjectKey{Name: vrf.Name, Namespace: metav1.NamespaceDefault} + defer func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vrf))).To(Succeed()) + }() + + By("Waiting for VRF to be ready") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, vrfKey, vrf)).To(Succeed()) + cond := meta.FindStatusCondition(vrf.Status.Conditions, v1alpha1.ReadyCondition) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) + + By("Creating DHCPRelay using a top-level VRF") + dhcprelay = &v1alpha1.DHCPRelay{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-dhcprelay-", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.DHCPRelaySpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: interfaceName}, + VrfRef: &v1alpha1.LocalObjectReference{Name: vrf.Name}, + Servers: []string{"192.168.1.1"}, + }, + } + Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) + resourceName = dhcprelay.Name + resourceKey = client.ObjectKey{Name: resourceName, Namespace: metav1.NamespaceDefault} + + By("Verifying the controller sets ReadyCondition to True") + Eventually(func(g Gomega) { + dhcprelay = &v1alpha1.DHCPRelay{} + g.Expect(k8sClient.Get(ctx, resourceKey, dhcprelay)).To(Succeed()) + cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ReadyCondition) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) + + By("Ensuring the DHCPRelay is created in the provider") + Eventually(func(g Gomega) { + g.Expect(testProvider.DHCPRelay).ToNot(BeNil()) + g.Expect(testProvider.DHCPRelay.GetName()).To(Equal(resourceName)) + }).Should(Succeed()) + }) + + It("Should reject duplicate DHCPRelay resources on the same interface", func() { + By("Creating the first DHCPRelay resource") + dhcprelay = &v1alpha1.DHCPRelay{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-dhcprelay-", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.DHCPRelaySpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: interfaceName}, + Servers: []string{"192.168.1.1"}, }, } Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) @@ -240,18 +339,16 @@ var _ = Describe("DHCPRelay Controller", func() { g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) }).Should(Succeed()) - By("Creating a second DHCPRelay resource for the same device") + By("Creating a second DHCPRelay resource for the same interface") duplicateDHCPRelay := &v1alpha1.DHCPRelay{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-dhcprelay-dup-", Namespace: metav1.NamespaceDefault, }, Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: interfaceName}, - }, + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: interfaceName}, + Servers: []string{"192.168.1.1"}, }, } Expect(k8sClient.Create(ctx, duplicateDHCPRelay)).To(Succeed()) @@ -269,7 +366,112 @@ var _ = Describe("DHCPRelay Controller", func() { }).Should(Succeed()) By("Cleaning up the duplicate DHCPRelay resource") + testProvider.Lock() + deleteCalls := testProvider.DHCPRelayDeleteCalls + testProvider.Unlock() Expect(k8sClient.Delete(ctx, duplicateDHCPRelay)).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, duplicateKey, &v1alpha1.DHCPRelay{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + + By("Verifying deleting the duplicate did not remove the active provider configuration") + testProvider.Lock() + actualDeleteCalls := testProvider.DHCPRelayDeleteCalls + providerDHCPRelay := testProvider.DHCPRelay + testProvider.Unlock() + Expect(actualDeleteCalls).To(Equal(deleteCalls)) + Expect(providerDHCPRelay).ToNot(BeNil()) + Expect(providerDHCPRelay.Name).To(Equal(resourceName)) + }) + + It("Should allow DHCPRelay resources on different interfaces of the same device", func() { + By("Creating another VLAN and Interface resource") + otherVLAN := &v1alpha1.VLAN{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-dhcprelay-other-vlan-", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.VLANSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + ID: 11, + Name: "vlan11", + }, + } + Expect(k8sClient.Create(ctx, otherVLAN)).To(Succeed()) + defer func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherVLAN))).To(Succeed()) + }() + + otherInterface := &v1alpha1.Interface{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-dhcprelay-other-intf-", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.InterfaceSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + Name: "vlan11", + Type: v1alpha1.InterfaceTypeRoutedVLAN, + AdminState: v1alpha1.AdminStateUp, + VlanRef: &v1alpha1.LocalObjectReference{Name: otherVLAN.Name}, + IPv4: &v1alpha1.InterfaceIPv4{ + Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.1.1/24")}}, + }, + }, + } + Expect(k8sClient.Create(ctx, otherInterface)).To(Succeed()) + defer func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherInterface))).To(Succeed()) + }() + + By("Waiting for the second Interface to be configured") + otherInterfaceKey := client.ObjectKeyFromObject(otherInterface) + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, otherInterfaceKey, otherInterface)).To(Succeed()) + cond := meta.FindStatusCondition(otherInterface.Status.Conditions, v1alpha1.ConfiguredCondition) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) + + By("Creating DHCPRelay resources for both interfaces") + dhcprelay = &v1alpha1.DHCPRelay{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-", Namespace: metav1.NamespaceDefault}, + Spec: v1alpha1.DHCPRelaySpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: interfaceName}, + Servers: []string{"192.168.1.1"}, + }, + } + Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) + resourceKey = client.ObjectKeyFromObject(dhcprelay) + + otherDHCPRelay := &v1alpha1.DHCPRelay{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-other-", Namespace: metav1.NamespaceDefault}, + Spec: v1alpha1.DHCPRelaySpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: otherInterface.Name}, + Servers: []string{"192.168.1.1"}, + }, + } + Expect(k8sClient.Create(ctx, otherDHCPRelay)).To(Succeed()) + otherDHCPRelayKey := client.ObjectKeyFromObject(otherDHCPRelay) + defer func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherDHCPRelay))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, otherDHCPRelayKey, &v1alpha1.DHCPRelay{}))).To(BeTrue()) + }).Should(Succeed()) + }() + + By("Verifying both DHCPRelay resources become ready") + for _, key := range []client.ObjectKey{resourceKey, otherDHCPRelayKey} { + Eventually(func(g Gomega) { + relay := &v1alpha1.DHCPRelay{} + g.Expect(k8sClient.Get(ctx, key, relay)).To(Succeed()) + cond := meta.FindStatusCondition(relay.Status.Conditions, v1alpha1.ReadyCondition) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) + } }) It("Should properly handle deletion and cleanup", func() { @@ -280,11 +482,9 @@ var _ = Describe("DHCPRelay Controller", func() { Namespace: metav1.NamespaceDefault, }, Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: interfaceName}, - }, + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: interfaceName}, + Servers: []string{"192.168.1.1"}, }, } Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) @@ -343,11 +543,9 @@ var _ = Describe("DHCPRelay Controller", func() { Namespace: metav1.NamespaceDefault, }, Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: "non-existent-device"}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: "test-interface"}, - }, + DeviceRef: v1alpha1.LocalObjectReference{Name: "non-existent-device"}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: "test-interface"}, + Servers: []string{"192.168.1.1"}, }, } Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) @@ -363,31 +561,97 @@ var _ = Describe("DHCPRelay Controller", func() { }) }) - Context("When InterfaceRef references non-existent Interface", func() { + Context("When Interface has unnumbered IPv4 configuration", func() { var ( - deviceName string - resourceName string - resourceKey client.ObjectKey - deviceKey client.ObjectKey - device *v1alpha1.Device + deviceName string + resourceName string + loopbackIntfName string + unnumberedIntfName string + resourceKey client.ObjectKey + deviceKey client.ObjectKey + loopbackIntfKey client.ObjectKey + unnumberedIntfKey client.ObjectKey + device *v1alpha1.Device + loopbackIntf *v1alpha1.Interface + unnumberedIntf *v1alpha1.Interface ) BeforeEach(func() { By("Creating the Device resource") device = &v1alpha1.Device{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-noint-", + GenerateName: "test-dhcprelay-unnum-", Namespace: metav1.NamespaceDefault, }, Spec: v1alpha1.DeviceSpec{ Endpoint: v1alpha1.Endpoint{ - Address: "192.168.10.51:9339", + Address: "192.168.10.54:9339", }, }, } Expect(k8sClient.Create(ctx, device)).To(Succeed()) deviceName = device.Name deviceKey = client.ObjectKey{Name: deviceName, Namespace: metav1.NamespaceDefault} + + By("Creating a loopback Interface with an IP address") + loopbackIntf = &v1alpha1.Interface{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-dhcprelay-unnum-lo-", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.InterfaceSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + Name: "loopback0", + Type: v1alpha1.InterfaceTypeLoopback, + AdminState: v1alpha1.AdminStateUp, + IPv4: &v1alpha1.InterfaceIPv4{ + Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.255.255.1/32")}}, + }, + }, + } + Expect(k8sClient.Create(ctx, loopbackIntf)).To(Succeed()) + loopbackIntfName = loopbackIntf.Name + loopbackIntfKey = client.ObjectKey{Name: loopbackIntfName, Namespace: metav1.NamespaceDefault} + + By("Waiting for loopback Interface to be ready") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, loopbackIntfKey, loopbackIntf) + g.Expect(err).NotTo(HaveOccurred()) + cond := meta.FindStatusCondition(loopbackIntf.Status.Conditions, v1alpha1.ConfiguredCondition) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) + + By("Creating an unnumbered Interface referencing the loopback") + unnumberedIntf = &v1alpha1.Interface{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-dhcprelay-unnum-intf-", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.InterfaceSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + Name: "ethernet1/1", + Type: v1alpha1.InterfaceTypePhysical, + AdminState: v1alpha1.AdminStateUp, + IPv4: &v1alpha1.InterfaceIPv4{ + Unnumbered: &v1alpha1.InterfaceIPv4Unnumbered{ + InterfaceRef: v1alpha1.LocalObjectReference{Name: loopbackIntfName}, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, unnumberedIntf)).To(Succeed()) + unnumberedIntfName = unnumberedIntf.Name + unnumberedIntfKey = client.ObjectKey{Name: unnumberedIntfName, Namespace: metav1.NamespaceDefault} + + By("Waiting for unnumbered Interface to be configured") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, unnumberedIntfKey, unnumberedIntf) + g.Expect(err).NotTo(HaveOccurred()) + cond := meta.FindStatusCondition(unnumberedIntf.Status.Conditions, v1alpha1.ConfiguredCondition) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) }) AfterEach(func() { @@ -401,381 +665,194 @@ var _ = Describe("DHCPRelay Controller", func() { g.Expect(errors.IsNotFound(err)).To(BeTrue()) }).Should(Succeed()) + By("Cleaning up the unnumbered Interface resource") + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, unnumberedIntf))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, unnumberedIntfKey, &v1alpha1.Interface{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + + By("Cleaning up the loopback Interface resource") + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, loopbackIntf))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, loopbackIntfKey, &v1alpha1.Interface{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + + By("Verifying the provider has been cleaned up") + Eventually(func(g Gomega) { + g.Expect(testProvider.DHCPRelay).To(BeNil(), "Provider should have no DHCPRelay configured") + }).Should(Succeed()) + By("Cleaning up the Device resource") - device := &v1alpha1.Device{} + device = &v1alpha1.Device{} device.Name = deviceKey.Name device.Namespace = deviceKey.Namespace Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) }) - It("Should set ConfiguredCondition to False when Interface does not exist", func() { - By("Creating DHCPRelay referencing a non-existent Interface") + It("Should successfully reconcile with an unnumbered Interface", func() { + By("Creating DHCPRelay with an unnumbered Interface") dhcprelay := &v1alpha1.DHCPRelay{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-noint-", + GenerateName: "test-dhcprelay-unnum-", Namespace: metav1.NamespaceDefault, }, Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: "non-existent-interface"}, - }, + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: unnumberedIntfName}, + Servers: []string{"192.168.1.1"}, }, } Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) resourceName = dhcprelay.Name resourceKey = client.ObjectKey{Name: resourceName, Namespace: metav1.NamespaceDefault} - By("Verifying the controller sets ConfiguredCondition to False with WaitingForDependenciesReason") + By("Verifying the controller sets ReadyCondition to True") Eventually(func(g Gomega) { err := k8sClient.Get(ctx, resourceKey, dhcprelay) g.Expect(err).NotTo(HaveOccurred()) - cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ConfiguredCondition) + cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ReadyCondition) g.Expect(cond).ToNot(BeNil()) - g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - g.Expect(cond.Reason).To(Equal(v1alpha1.WaitingForDependenciesReason)) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) + + By("Ensuring the DHCPRelay is created in the provider") + Eventually(func(g Gomega) { + g.Expect(testProvider.DHCPRelay).ToNot(BeNil(), "Provider DHCPRelay should not be nil") }).Should(Succeed()) }) }) - Context("When InterfaceRef belongs to a different device", func() { + Context("When the DHCPRelay references are invalid", func() { var ( - deviceName string - otherDeviceName string - resourceName string - otherIntfName string - otherVlanName string - resourceKey client.ObjectKey - deviceKey client.ObjectKey - otherDeviceKey client.ObjectKey - otherIntfKey client.ObjectKey - otherVlanKey client.ObjectKey - device *v1alpha1.Device - otherDevice *v1alpha1.Device - otherVlan *v1alpha1.VLAN - otherIntf *v1alpha1.Interface + deviceName string + deviceKey client.ObjectKey ) + cleanupObject := func(object client.Object) { + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, object))).To(Succeed()) + }) + } + + cleanupDHCPRelay := func(dhcprelay *v1alpha1.DHCPRelay) { + resourceKey := client.ObjectKeyFromObject(dhcprelay) + DeferCleanup(func() { + By("Cleaning up the DHCPRelay resource") + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dhcprelay))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, resourceKey, &v1alpha1.DHCPRelay{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + }) + } + BeforeEach(func() { By("Creating the Device resource") - device = &v1alpha1.Device{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-crossdev-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.DeviceSpec{ - Endpoint: v1alpha1.Endpoint{ - Address: "192.168.10.52:9339", - }, - }, + device := &v1alpha1.Device{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-invalid-", Namespace: metav1.NamespaceDefault}, + Spec: v1alpha1.DeviceSpec{Endpoint: v1alpha1.Endpoint{Address: "192.168.10.51:9339"}}, } Expect(k8sClient.Create(ctx, device)).To(Succeed()) deviceName = device.Name - deviceKey = client.ObjectKey{Name: deviceName, Namespace: metav1.NamespaceDefault} - - By("Creating another Device resource") - otherDevice = &v1alpha1.Device{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-crossdev-other-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.DeviceSpec{ - Endpoint: v1alpha1.Endpoint{ - Address: "192.168.10.53:9339", - }, - }, - } - Expect(k8sClient.Create(ctx, otherDevice)).To(Succeed()) - otherDeviceName = otherDevice.Name - otherDeviceKey = client.ObjectKey{Name: otherDeviceName, Namespace: metav1.NamespaceDefault} - - By("Creating a VLAN on the other Device") - otherVlan = &v1alpha1.VLAN{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-crossdev-vlan-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.VLANSpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: otherDeviceName}, - ID: 20, - Name: "vlan20", - }, - } - Expect(k8sClient.Create(ctx, otherVlan)).To(Succeed()) - otherVlanName = otherVlan.Name - otherVlanKey = client.ObjectKey{Name: otherVlanName, Namespace: metav1.NamespaceDefault} + deviceKey = client.ObjectKeyFromObject(device) + DeferCleanup(func() { + By("Cleaning up the Device resource") + device := &v1alpha1.Device{ObjectMeta: metav1.ObjectMeta{Name: deviceKey.Name, Namespace: deviceKey.Namespace}} + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) + }) + }) - By("Creating an Interface on the other Device") - otherIntf = &v1alpha1.Interface{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-crossdev-intf-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.InterfaceSpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: otherDeviceName}, - Name: "vlan20", - Type: v1alpha1.InterfaceTypeRoutedVLAN, - VlanRef: &v1alpha1.LocalObjectReference{Name: otherVlanName}, - AdminState: v1alpha1.AdminStateUp, - IPv4: &v1alpha1.InterfaceIPv4{ - Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.1.1/24")}}, - }, - }, + It("Should set ConfiguredCondition to False when Interface does not exist", func() { + By("Creating DHCPRelay referencing a non-existent Interface") + dhcprelay := &v1alpha1.DHCPRelay{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-noint-new-", Namespace: metav1.NamespaceDefault}, + Spec: v1alpha1.DHCPRelaySpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, InterfaceRef: &v1alpha1.LocalObjectReference{Name: "non-existent-interface"}, Servers: []string{"192.168.1.1"}}, } - Expect(k8sClient.Create(ctx, otherIntf)).To(Succeed()) - otherIntfName = otherIntf.Name - otherIntfKey = client.ObjectKey{Name: otherIntfName, Namespace: metav1.NamespaceDefault} - }) + Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) + cleanupDHCPRelay(dhcprelay) - AfterEach(func() { - By("Cleaning up the DHCPRelay resource") - dhcprelay := &v1alpha1.DHCPRelay{} - dhcprelay.Name = resourceKey.Name - dhcprelay.Namespace = resourceKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dhcprelay))).To(Succeed()) + By("Verifying the controller sets ConfiguredCondition to False with WaitingForDependenciesReason") Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, resourceKey, &v1alpha1.DHCPRelay{}) - g.Expect(errors.IsNotFound(err)).To(BeTrue()) + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(dhcprelay), dhcprelay)).To(Succeed()) + cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ConfiguredCondition) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(v1alpha1.WaitingForDependenciesReason)) }).Should(Succeed()) + }) - By("Cleaning up the Interface resource") - otherIntf := &v1alpha1.Interface{} - otherIntf.Name = otherIntfKey.Name - otherIntf.Namespace = otherIntfKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherIntf))).To(Succeed()) + It("Should set ConfiguredCondition to False when Interface belongs to a different Device", func() { + By("Creating another Device resource") + otherDevice := &v1alpha1.Device{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-crossdev-other-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.DeviceSpec{Endpoint: v1alpha1.Endpoint{Address: "192.168.10.53:9339"}}} + Expect(k8sClient.Create(ctx, otherDevice)).To(Succeed()) + cleanupObject(otherDevice) - By("Cleaning up the VLAN resource") - otherVlan := &v1alpha1.VLAN{} - otherVlan.Name = otherVlanKey.Name - otherVlan.Namespace = otherVlanKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherVlan))).To(Succeed()) + By("Creating a VLAN on the other Device") + otherVLAN := &v1alpha1.VLAN{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-crossdev-vlan-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.VLANSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: otherDevice.Name}, ID: 20, Name: "vlan20"}} + Expect(k8sClient.Create(ctx, otherVLAN)).To(Succeed()) + cleanupObject(otherVLAN) - By("Cleaning up the Device resources") - device := &v1alpha1.Device{} - device.Name = deviceKey.Name - device.Namespace = deviceKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) - otherDevice := &v1alpha1.Device{} - otherDevice.Name = otherDeviceKey.Name - otherDevice.Namespace = otherDeviceKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherDevice))).To(Succeed()) - }) + By("Creating an Interface on the other Device") + otherInterface := &v1alpha1.Interface{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-crossdev-intf-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.InterfaceSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: otherDevice.Name}, Name: "vlan20", Type: v1alpha1.InterfaceTypeRoutedVLAN, VlanRef: &v1alpha1.LocalObjectReference{Name: otherVLAN.Name}, AdminState: v1alpha1.AdminStateUp, IPv4: &v1alpha1.InterfaceIPv4{Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.1.1/24")}}}}} + Expect(k8sClient.Create(ctx, otherInterface)).To(Succeed()) + cleanupObject(otherInterface) - It("Should set ConfiguredCondition to False with CrossDeviceReferenceReason", func() { By("Creating DHCPRelay referencing an Interface from a different device") - dhcprelay := &v1alpha1.DHCPRelay{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-crossdev-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: otherIntfName}, - }, - }, - } + dhcprelay := &v1alpha1.DHCPRelay{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-crossdev-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.DHCPRelaySpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, InterfaceRef: &v1alpha1.LocalObjectReference{Name: otherInterface.Name}, Servers: []string{"192.168.1.1"}}} Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) - resourceName = dhcprelay.Name - resourceKey = client.ObjectKey{Name: resourceName, Namespace: metav1.NamespaceDefault} + cleanupDHCPRelay(dhcprelay) By("Verifying the controller sets ConfiguredCondition to False with CrossDeviceReferenceReason") Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, resourceKey, dhcprelay) - g.Expect(err).NotTo(HaveOccurred()) - + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(dhcprelay), dhcprelay)).To(Succeed()) cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ConfiguredCondition) g.Expect(cond).ToNot(BeNil()) g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) g.Expect(cond.Reason).To(Equal(v1alpha1.CrossDeviceReferenceReason)) }).Should(Succeed()) }) - }) - - Context("When VrfRef belongs to a different device", func() { - var ( - deviceName string - otherDeviceName string - resourceName string - interfaceName string - vlanName string - otherVrfName string - resourceKey client.ObjectKey - deviceKey client.ObjectKey - otherDeviceKey client.ObjectKey - interfaceKey client.ObjectKey - vlanKey client.ObjectKey - otherVrfKey client.ObjectKey - device *v1alpha1.Device - otherDevice *v1alpha1.Device - vlan *v1alpha1.VLAN - intf *v1alpha1.Interface - otherVrf *v1alpha1.VRF - ) - - BeforeEach(func() { - By("Creating the Device resource") - device = &v1alpha1.Device{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-vrfcross-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.DeviceSpec{ - Endpoint: v1alpha1.Endpoint{ - Address: "192.168.10.57:9339", - }, - }, - } - Expect(k8sClient.Create(ctx, device)).To(Succeed()) - deviceName = device.Name - deviceKey = client.ObjectKey{Name: deviceName, Namespace: metav1.NamespaceDefault} + It("Should set ConfiguredCondition to False when VRF belongs to a different Device", func() { By("Creating another Device resource") - otherDevice = &v1alpha1.Device{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-vrfcross-other-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.DeviceSpec{ - Endpoint: v1alpha1.Endpoint{ - Address: "192.168.10.58:9339", - }, - }, - } + otherDevice := &v1alpha1.Device{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-vrfcross-other-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.DeviceSpec{Endpoint: v1alpha1.Endpoint{Address: "192.168.10.58:9339"}}} Expect(k8sClient.Create(ctx, otherDevice)).To(Succeed()) - otherDeviceName = otherDevice.Name - otherDeviceKey = client.ObjectKey{Name: otherDeviceName, Namespace: metav1.NamespaceDefault} + cleanupObject(otherDevice) By("Creating a VLAN on the main Device") - vlan = &v1alpha1.VLAN{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-vrfcross-vlan-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.VLANSpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - ID: 60, - Name: "vlan60", - }, - } + vlan := &v1alpha1.VLAN{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-vrfcross-vlan-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.VLANSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, ID: 60, Name: "vlan60"}} Expect(k8sClient.Create(ctx, vlan)).To(Succeed()) - vlanName = vlan.Name - vlanKey = client.ObjectKey{Name: vlanName, Namespace: metav1.NamespaceDefault} + cleanupObject(vlan) By("Creating an Interface on the main Device") - intf = &v1alpha1.Interface{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-vrfcross-intf-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.InterfaceSpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Name: "vlan60", - Type: v1alpha1.InterfaceTypeRoutedVLAN, - VlanRef: &v1alpha1.LocalObjectReference{Name: vlanName}, - AdminState: v1alpha1.AdminStateUp, - IPv4: &v1alpha1.InterfaceIPv4{ - Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.6.1/24")}}, - }, - }, - } + intf := &v1alpha1.Interface{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-vrfcross-intf-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.InterfaceSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, Name: "vlan60", Type: v1alpha1.InterfaceTypeRoutedVLAN, VlanRef: &v1alpha1.LocalObjectReference{Name: vlan.Name}, AdminState: v1alpha1.AdminStateUp, IPv4: &v1alpha1.InterfaceIPv4{Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.6.1/24")}}}}} Expect(k8sClient.Create(ctx, intf)).To(Succeed()) - interfaceName = intf.Name - interfaceKey = client.ObjectKey{Name: interfaceName, Namespace: metav1.NamespaceDefault} + cleanupObject(intf) By("Waiting for Interface to be configured") + interfaceKey := client.ObjectKeyFromObject(intf) Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, interfaceKey, intf) - g.Expect(err).NotTo(HaveOccurred()) + g.Expect(k8sClient.Get(ctx, interfaceKey, intf)).To(Succeed()) cond := meta.FindStatusCondition(intf.Status.Conditions, v1alpha1.ConfiguredCondition) g.Expect(cond).ToNot(BeNil()) g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) }).Should(Succeed()) By("Creating a VRF on the other Device") - otherVrf = &v1alpha1.VRF{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-vrfcross-vrf-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.VRFSpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: otherDeviceName}, - Name: "VRF-OTHER", - }, - } - Expect(k8sClient.Create(ctx, otherVrf)).To(Succeed()) - otherVrfName = otherVrf.Name - otherVrfKey = client.ObjectKey{Name: otherVrfName, Namespace: metav1.NamespaceDefault} - }) - - AfterEach(func() { - By("Cleaning up the DHCPRelay resource") - dhcprelay := &v1alpha1.DHCPRelay{} - dhcprelay.Name = resourceKey.Name - dhcprelay.Namespace = resourceKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dhcprelay))).To(Succeed()) - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, resourceKey, &v1alpha1.DHCPRelay{}) - g.Expect(errors.IsNotFound(err)).To(BeTrue()) - }).Should(Succeed()) - - By("Cleaning up the VRF resource") - otherVrf := &v1alpha1.VRF{} - otherVrf.Name = otherVrfKey.Name - otherVrf.Namespace = otherVrfKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherVrf))).To(Succeed()) - - By("Cleaning up the Interface resource") - intf := &v1alpha1.Interface{} - intf.Name = interfaceKey.Name - intf.Namespace = interfaceKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed()) - - By("Cleaning up the VLAN resource") - vlan := &v1alpha1.VLAN{} - vlan.Name = vlanKey.Name - vlan.Namespace = vlanKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vlan))).To(Succeed()) - - By("Cleaning up the Device resources") - device := &v1alpha1.Device{} - device.Name = deviceKey.Name - device.Namespace = deviceKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) - otherDevice := &v1alpha1.Device{} - otherDevice.Name = otherDeviceKey.Name - otherDevice.Namespace = otherDeviceKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherDevice))).To(Succeed()) - }) + otherVRF := &v1alpha1.VRF{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-vrfcross-vrf-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.VRFSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: otherDevice.Name}, Name: "VRF-OTHER"}} + Expect(k8sClient.Create(ctx, otherVRF)).To(Succeed()) + cleanupObject(otherVRF) - It("Should set ConfiguredCondition to False with CrossDeviceReferenceReason", func() { - By("Creating DHCPRelay referencing a VRF from a different device") - dhcprelay := &v1alpha1.DHCPRelay{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-vrfcross-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: interfaceName}, - }, - VrfRef: &v1alpha1.LocalObjectReference{Name: otherVrfName}, - }, - } + By("Creating DHCPRelay with a VRF from a different device") + dhcprelay := &v1alpha1.DHCPRelay{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-vrfcross-new-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.DHCPRelaySpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, InterfaceRef: &v1alpha1.LocalObjectReference{Name: intf.Name}, VrfRef: &v1alpha1.LocalObjectReference{Name: otherVRF.Name}, Servers: []string{"192.168.1.1"}}} Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) - resourceName = dhcprelay.Name - resourceKey = client.ObjectKey{Name: resourceName, Namespace: metav1.NamespaceDefault} + cleanupDHCPRelay(dhcprelay) By("Verifying the controller sets ConfiguredCondition to False with CrossDeviceReferenceReason") Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, resourceKey, dhcprelay) - g.Expect(err).NotTo(HaveOccurred()) - + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(dhcprelay), dhcprelay)).To(Succeed()) cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ConfiguredCondition) g.Expect(cond).ToNot(BeNil()) g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) @@ -783,182 +860,46 @@ var _ = Describe("DHCPRelay Controller", func() { g.Expect(cond.Message).To(ContainSubstring("VRF")) }).Should(Succeed()) }) - }) - Context("When Interface has unnumbered IPv4 configuration", func() { - var ( - deviceName string - resourceName string - loopbackIntfName string - unnumberedIntfName string - resourceKey client.ObjectKey - deviceKey client.ObjectKey - loopbackIntfKey client.ObjectKey - unnumberedIntfKey client.ObjectKey - device *v1alpha1.Device - loopbackIntf *v1alpha1.Interface - unnumberedIntf *v1alpha1.Interface - ) - - BeforeEach(func() { - By("Creating the Device resource") - device = &v1alpha1.Device{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-unnum-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.DeviceSpec{ - Endpoint: v1alpha1.Endpoint{ - Address: "192.168.10.54:9339", - }, - }, - } - Expect(k8sClient.Create(ctx, device)).To(Succeed()) - deviceName = device.Name - deviceKey = client.ObjectKey{Name: deviceName, Namespace: metav1.NamespaceDefault} - - By("Creating a loopback Interface with an IP address") - loopbackIntf = &v1alpha1.Interface{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-unnum-lo-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.InterfaceSpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Name: "loopback0", - Type: v1alpha1.InterfaceTypeLoopback, - AdminState: v1alpha1.AdminStateUp, - IPv4: &v1alpha1.InterfaceIPv4{ - Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.255.255.1/32")}}, - }, - }, - } - Expect(k8sClient.Create(ctx, loopbackIntf)).To(Succeed()) - loopbackIntfName = loopbackIntf.Name - loopbackIntfKey = client.ObjectKey{Name: loopbackIntfName, Namespace: metav1.NamespaceDefault} - - By("Waiting for loopback Interface to be ready") - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, loopbackIntfKey, loopbackIntf) - g.Expect(err).NotTo(HaveOccurred()) - cond := meta.FindStatusCondition(loopbackIntf.Status.Conditions, v1alpha1.ConfiguredCondition) - g.Expect(cond).ToNot(BeNil()) - g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - }).Should(Succeed()) + It("Should set ConfiguredCondition to False when Interface is not configured", func() { + const nonExistentVrfName = "testdhcprelay-intfnotready-nonexistent-vrf" + By("Creating the VLAN resource") + vlan := &v1alpha1.VLAN{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-intfnr-vlan-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.VLANSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, ID: 40, Name: "vlan40", AdminState: v1alpha1.AdminStateUp}} + Expect(k8sClient.Create(ctx, vlan)).To(Succeed()) + cleanupObject(vlan) - By("Creating an unnumbered Interface referencing the loopback") - unnumberedIntf = &v1alpha1.Interface{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-unnum-intf-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.InterfaceSpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Name: "ethernet1/1", - Type: v1alpha1.InterfaceTypePhysical, - AdminState: v1alpha1.AdminStateUp, - IPv4: &v1alpha1.InterfaceIPv4{ - Unnumbered: &v1alpha1.InterfaceIPv4Unnumbered{ - InterfaceRef: v1alpha1.LocalObjectReference{Name: loopbackIntfName}, - }, - }, - }, - } - Expect(k8sClient.Create(ctx, unnumberedIntf)).To(Succeed()) - unnumberedIntfName = unnumberedIntf.Name - unnumberedIntfKey = client.ObjectKey{Name: unnumberedIntfName, Namespace: metav1.NamespaceDefault} + By("Creating an Interface resource with a VRF reference to a non-existent VRF") + intf := &v1alpha1.Interface{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-intfnr-intf-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.InterfaceSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, Name: "vlan40", AdminState: v1alpha1.AdminStateUp, Type: v1alpha1.InterfaceTypeRoutedVLAN, VlanRef: &v1alpha1.LocalObjectReference{Name: vlan.Name}, VrfRef: &v1alpha1.LocalObjectReference{Name: nonExistentVrfName}, IPv4: &v1alpha1.InterfaceIPv4{Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.4.1/24")}}}}} + Expect(k8sClient.Create(ctx, intf)).To(Succeed()) + cleanupObject(intf) - By("Waiting for unnumbered Interface to be configured") + By("Verifying the Interface is NOT Ready") + interfaceKey := client.ObjectKeyFromObject(intf) Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, unnumberedIntfKey, unnumberedIntf) - g.Expect(err).NotTo(HaveOccurred()) - cond := meta.FindStatusCondition(unnumberedIntf.Status.Conditions, v1alpha1.ConfiguredCondition) + g.Expect(k8sClient.Get(ctx, interfaceKey, intf)).To(Succeed()) + cond := meta.FindStatusCondition(intf.Status.Conditions, v1alpha1.ReadyCondition) g.Expect(cond).ToNot(BeNil()) - g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - }).Should(Succeed()) - }) - - AfterEach(func() { - By("Cleaning up the DHCPRelay resource") - dhcprelay := &v1alpha1.DHCPRelay{} - dhcprelay.Name = resourceKey.Name - dhcprelay.Namespace = resourceKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dhcprelay))).To(Succeed()) - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, resourceKey, &v1alpha1.DHCPRelay{}) - g.Expect(errors.IsNotFound(err)).To(BeTrue()) - }).Should(Succeed()) - - By("Cleaning up the unnumbered Interface resource") - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, unnumberedIntf))).To(Succeed()) - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, unnumberedIntfKey, &v1alpha1.Interface{}) - g.Expect(errors.IsNotFound(err)).To(BeTrue()) - }).Should(Succeed()) - - By("Cleaning up the loopback Interface resource") - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, loopbackIntf))).To(Succeed()) - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, loopbackIntfKey, &v1alpha1.Interface{}) - g.Expect(errors.IsNotFound(err)).To(BeTrue()) - }).Should(Succeed()) - - By("Verifying the provider has been cleaned up") - Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).To(BeNil(), "Provider should have no DHCPRelay configured") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) }).Should(Succeed()) - By("Cleaning up the Device resource") - device = &v1alpha1.Device{} - device.Name = deviceKey.Name - device.Namespace = deviceKey.Namespace - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) - }) - - It("Should successfully reconcile with an unnumbered Interface", func() { - By("Creating DHCPRelay with an unnumbered Interface") - dhcprelay := &v1alpha1.DHCPRelay{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-unnum-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: unnumberedIntfName}, - }, - }, - } + By("Creating DHCPRelay referencing a non-configured Interface") + dhcprelay := &v1alpha1.DHCPRelay{ObjectMeta: metav1.ObjectMeta{GenerateName: "test-dhcprelay-intfnr-", Namespace: metav1.NamespaceDefault}, Spec: v1alpha1.DHCPRelaySpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, InterfaceRef: &v1alpha1.LocalObjectReference{Name: intf.Name}, Servers: []string{"192.168.1.1"}}} Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) - resourceName = dhcprelay.Name - resourceKey = client.ObjectKey{Name: resourceName, Namespace: metav1.NamespaceDefault} + cleanupDHCPRelay(dhcprelay) - By("Verifying the controller sets ReadyCondition to True") + By("Verifying the controller sets ConfiguredCondition to False with WaitingForDependenciesReason") Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, resourceKey, dhcprelay) - g.Expect(err).NotTo(HaveOccurred()) - - cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ReadyCondition) + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(dhcprelay), dhcprelay)).To(Succeed()) + cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ConfiguredCondition) g.Expect(cond).ToNot(BeNil()) - g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - }).Should(Succeed()) - - By("Verifying the status contains configured interface refs") - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, resourceKey, dhcprelay) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(dhcprelay.Status.ConfiguredInterfaces).To(ContainElement(unnumberedIntf.Spec.Name)) - }).Should(Succeed()) - - By("Ensuring the DHCPRelay is created in the provider") - Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).ToNot(BeNil(), "Provider DHCPRelay should not be nil") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(v1alpha1.WaitingForDependenciesReason)) + g.Expect(cond.Message).To(ContainSubstring("not configured")) }).Should(Succeed()) }) }) - Context("When Interface is not Ready", func() { + Context("When Interface becomes configured", func() { var ( deviceName string resourceName string @@ -1071,38 +1012,6 @@ var _ = Describe("DHCPRelay Controller", func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) }) - It("Should set ConfiguredCondition to False with WaitingForDependenciesReason when Interface is not configured", func() { - By("Creating DHCPRelay referencing a non-configured Interface") - dhcprelay := &v1alpha1.DHCPRelay{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-dhcprelay-intfnr-", - Namespace: metav1.NamespaceDefault, - }, - Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: interfaceName}, - }, - }, - } - Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) - resourceName = dhcprelay.Name - resourceKey = client.ObjectKey{Name: resourceName, Namespace: metav1.NamespaceDefault} - - By("Verifying the controller sets ConfiguredCondition to False with WaitingForDependenciesReason") - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, resourceKey, dhcprelay) - g.Expect(err).NotTo(HaveOccurred()) - - cond := meta.FindStatusCondition(dhcprelay.Status.Conditions, v1alpha1.ConfiguredCondition) - g.Expect(cond).ToNot(BeNil()) - g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - g.Expect(cond.Reason).To(Equal(v1alpha1.WaitingForDependenciesReason)) - g.Expect(cond.Message).To(ContainSubstring("not configured")) - }).Should(Succeed()) - }) - It("Should re-reconcile DHCPRelay when Interface becomes configured (watch trigger)", func() { By("Creating DHCPRelay referencing a non-configured Interface") dhcprelay := &v1alpha1.DHCPRelay{ @@ -1111,11 +1020,9 @@ var _ = Describe("DHCPRelay Controller", func() { Namespace: metav1.NamespaceDefault, }, Spec: v1alpha1.DHCPRelaySpec{ - DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, - Servers: []string{"192.168.1.1"}, - InterfaceRefs: []v1alpha1.LocalObjectReference{ - {Name: interfaceName}, - }, + DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, + InterfaceRef: &v1alpha1.LocalObjectReference{Name: interfaceName}, + Servers: []string{"192.168.1.1"}, }, } Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) diff --git a/internal/controller/core/suite_test.go b/internal/controller/core/suite_test.go index 6474735af..49def73c3 100644 --- a/internal/controller/core/suite_test.go +++ b/internal/controller/core/suite_test.go @@ -458,37 +458,38 @@ type Provider struct { ConnectError error // if non-nil, Connect returns this error LastRebootTime time.Time - Ports sets.Set[string] - User sets.Set[string] - PreLoginBanner *string - PostLoginBanner *string - DNS *v1alpha1.DNS - NTP *v1alpha1.NTP - ACLs sets.Set[string] - Certs sets.Set[string] - SNMP *v1alpha1.SNMP - Syslog *v1alpha1.Syslog - Access *v1alpha1.ManagementAccess - ISIS sets.Set[string] - VRF sets.Set[string] - PIM *v1alpha1.PIM - BGP *v1alpha1.BGP - BGPVRF *v1alpha1.VRF - BGPPeers sets.Set[string] - OSPF sets.Set[string] - VLANs sets.Set[int16] - EVIs sets.Set[int32] - PrefixSets sets.Set[string] - RoutingPolicies sets.Set[string] - NVE *v1alpha1.NetworkVirtualizationEdge - LLDP *v1alpha1.LLDP - LLDPOperStatus bool - LLDPNeighbors map[string]*provider.LLDPAdjacency - DHCPRelay *v1alpha1.DHCPRelay - EthernetSegments map[string]string - StartupConfig *v1alpha1.ConfigBackup - ConfigBackups []*provider.ConfigBackupFile - StorageTotal int64 + Ports sets.Set[string] + User sets.Set[string] + PreLoginBanner *string + PostLoginBanner *string + DNS *v1alpha1.DNS + NTP *v1alpha1.NTP + ACLs sets.Set[string] + Certs sets.Set[string] + SNMP *v1alpha1.SNMP + Syslog *v1alpha1.Syslog + Access *v1alpha1.ManagementAccess + ISIS sets.Set[string] + VRF sets.Set[string] + PIM *v1alpha1.PIM + BGP *v1alpha1.BGP + BGPVRF *v1alpha1.VRF + BGPPeers sets.Set[string] + OSPF sets.Set[string] + VLANs sets.Set[int16] + EVIs sets.Set[int32] + PrefixSets sets.Set[string] + RoutingPolicies sets.Set[string] + NVE *v1alpha1.NetworkVirtualizationEdge + LLDP *v1alpha1.LLDP + LLDPOperStatus bool + LLDPNeighbors map[string]*provider.LLDPAdjacency + DHCPRelay *v1alpha1.DHCPRelay + DHCPRelayDeleteCalls int + EthernetSegments map[string]string + StartupConfig *v1alpha1.ConfigBackup + ConfigBackups []*provider.ConfigBackupFile + StorageTotal int64 } func NewProvider() *Provider { @@ -1042,23 +1043,11 @@ func (p *Provider) EnsureDHCPRelay(_ context.Context, req *provider.DHCPRelayReq func (p *Provider) DeleteDHCPRelay(_ context.Context, req *provider.DHCPRelayRequest) error { p.Lock() defer p.Unlock() + p.DHCPRelayDeleteCalls++ p.DHCPRelay = nil return nil } -func (p *Provider) GetDHCPRelayStatus(_ context.Context, req *provider.DHCPRelayRequest) (provider.DHCPRelayStatus, error) { - p.Lock() - defer p.Unlock() - status := provider.DHCPRelayStatus{} - if p.DHCPRelay != nil { - // Return the interface names from the request (simulating what the device would return) - for _, intf := range req.Interfaces { - status.ConfiguredInterfaces = append(status.ConfiguredInterfaces, intf.Spec.Name) - } - } - return status, nil -} - func (p *Provider) EnsureEthernetSegment(_ context.Context, req *provider.EnsureEthernetSegmentRequest) error { p.Lock() defer p.Unlock() diff --git a/internal/provider/cisco/nxos/dhcprelay.go b/internal/provider/cisco/nxos/dhcprelay.go index daf5e7df3..41d0df1fd 100644 --- a/internal/provider/cisco/nxos/dhcprelay.go +++ b/internal/provider/cisco/nxos/dhcprelay.go @@ -9,7 +9,10 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.DataElement = (*DHCPRelayConfig)(nil) +var ( + _ gnmiext.DataElement = (*DHCPRelayConfig)(nil) + _ gnmiext.DataElement = (*DHCPRelay)(nil) +) // DHCPRelayConfig represents the complete DHCP relay configuration tree. type DHCPRelayConfig struct { @@ -22,7 +25,7 @@ func (*DHCPRelayConfig) XPath() string { // DHCPRelay represents the DHCP Relay configuration for a single interface. type DHCPRelay struct { - ID string `json:"id"` + ID string `json:"-"` AddrItems struct { AddrList gnmiext.List[netip.Addr, *DHCPRelayServer] `json:"RelayAddr-list,omitzero"` } `json:"addr-items"` @@ -34,6 +37,10 @@ func (d *DHCPRelay) Key() string { func (*DHCPRelay) IsListItem() {} +func (d *DHCPRelay) XPath() string { + return "System/dhcp-items/inst-items/relayif-items/RelayIf-list[id=" + d.ID + "]" +} + type DHCPRelayServer struct { Address netip.Addr `json:"address"` Vrf string `json:"vrf,omitempty"` diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index 4e1fc96b9..6963e4141 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -1763,6 +1763,10 @@ func (p *Provider) DeleteInterface(ctx context.Context, req *provider.InterfaceR } case v1alpha1.InterfaceTypeRoutedVLAN: + icmp := new(ICMPIf) + icmp.ID = name + sb.Delete(icmp) + svi := new(SwitchVirtualInterface) svi.ID = name sb.Delete(svi) @@ -3647,8 +3651,7 @@ func (p *Provider) GetLLDPStatus(ctx context.Context, req *provider.LLDPRequest) return s, nil } -// EnsureDHCPRelay configures DHCP relay on the specified interfaces. -// Replaces the entire DHCP relay configuration on the device with the provided configuration in the request. +// EnsureDHCPRelay configures DHCP relay for the specified interfaces. func (p *Provider) EnsureDHCPRelay(ctx context.Context, req *provider.DHCPRelayRequest) error { sb := new(gnmiext.SetBuilder).Limit(maxSetOperations) @@ -3657,57 +3660,63 @@ func (p *Provider) EnsureDHCPRelay(ctx context.Context, req *provider.DHCPRelayR f.AdminSt = AdminStEnabled sb.Update(f) - // undocumented default value for the VRF property in DME (can be verified via NX-API) - vrfName := "!unspecified" - if req.VRF != nil { - vrfName = req.VRF.Spec.Name - } - - dhcp := new(DHCPRelayConfig) - for _, intf := range req.Interfaces { - ifName, err := ShortName(intf.Spec.Name) + // relayFor generates per-interface relay config + relayFor := func(intfName string) (*DHCPRelay, error) { + name, err := ShortName(intfName) if err != nil { - return fmt.Errorf("dhcp relay: failed to get short name for interface %q: %w", intf.Spec.Name, err) + return nil, fmt.Errorf("dhcp relay: failed to get short name for interface %q: %w", intfName, err) } - - relay := &DHCPRelay{ID: ifName} + relay := &DHCPRelay{ID: name} for _, addr := range req.DHCPRelay.Spec.Servers { a, err := netip.ParseAddr(addr) if err != nil { - return fmt.Errorf("dhcp relay: invalid server address %q: %w", addr, err) + return nil, fmt.Errorf("dhcp relay: invalid server address %q: %w", addr, err) } - relay.AddrItems.AddrList.Set(&DHCPRelayServer{Address: a, Vrf: vrfName}) + srv := &DHCPRelayServer{Address: a, Vrf: "!unspecified"} + if req.VRF != nil { + srv.Vrf = req.VRF.Spec.Name + } + relay.AddrItems.AddrList.Set(srv) } - dhcp.RelayIfList.Set(relay) + return relay, nil } - sb.Update(dhcp) + // deprecated path + if req.Interface == nil { + dhcp := new(DHCPRelayConfig) + for _, intf := range req.Interfaces { + relay, err := relayFor(intf.Spec.Name) + if err != nil { + return err + } + dhcp.RelayIfList.Set(relay) + } + sb.Update(dhcp) + return p.Do(ctx, sb) + } + + relay, err := relayFor(req.Interface.Spec.Name) + if err != nil { + return err + } + sb.Update(relay) return p.Do(ctx, sb) } // DeleteDHCPRelay removes all DHCP relay configurations from the device. func (p *Provider) DeleteDHCPRelay(ctx context.Context, req *provider.DHCPRelayRequest) error { - return p.client.Delete(ctx, new(DHCPRelayConfig)) -} - -// GetDHCPRelayStatus retrieves the current DHCP relay status. -func (p *Provider) GetDHCPRelayStatus(ctx context.Context, req *provider.DHCPRelayRequest) (provider.DHCPRelayStatus, error) { - var s provider.DHCPRelayStatus - - config := new(DHCPRelayConfig) - if err := p.client.GetConfig(ctx, config); err != nil { - if errors.Is(err, gnmiext.ErrNil) { - return s, nil - } - return s, fmt.Errorf("dhcp relay: failed to get status: %w", err) + // deprecated path + if len(req.DHCPRelay.Spec.InterfaceRefs) > 0 { //nolint:staticcheck + return p.client.Delete(ctx, new(DHCPRelayConfig)) } - s.ConfiguredInterfaces = make([]string, 0, config.RelayIfList.Len()) - for _, relay := range config.RelayIfList { - s.ConfiguredInterfaces = append(s.ConfiguredInterfaces, relay.ID) + // normal path + ifName, err := ShortName(req.Interface.Spec.Name) + if err != nil { + return fmt.Errorf("dhcp relay: failed to get short name for interface %q: %w", req.Interface.Spec.Name, err) } - - return s, nil + relay := &DHCPRelay{ID: ifName} + return p.client.Delete(ctx, relay) } func (p *Provider) EnsureEthernetSegment(ctx context.Context, req *provider.EnsureEthernetSegmentRequest) error { diff --git a/internal/provider/provider.go b/internal/provider/provider.go index dd38163d5..bd917fb16 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -786,20 +786,14 @@ type DHCPRelayProvider interface { EnsureDHCPRelay(context.Context, *DHCPRelayRequest) error // DeleteDHCPRelay deletes the DHCP Relay configuration. DeleteDHCPRelay(context.Context, *DHCPRelayRequest) error - // GetDHCPRelayStatus call retrieves the current status of the DHCP Relay configuration. - GetDHCPRelayStatus(context.Context, *DHCPRelayRequest) (DHCPRelayStatus, error) } type DHCPRelayRequest struct { DHCPRelay *v1alpha1.DHCPRelay ProviderConfig *ProviderConfig - Interfaces []*v1alpha1.Interface + Interface *v1alpha1.Interface VRF *v1alpha1.VRF -} - -type DHCPRelayStatus struct { - // ConfiguredInterfaces contains the names of the interfaces on the device for which DHCP Relay is configured, e.g., eth1/1. - ConfiguredInterfaces []string + Interfaces []v1alpha1.Interface // deprecated } type EthernetSegmentProvider interface { diff --git a/internal/webhook/core/v1alpha1/dhcprelay_webhook.go b/internal/webhook/core/v1alpha1/dhcprelay_webhook.go new file mode 100644 index 000000000..49bb6daa8 --- /dev/null +++ b/internal/webhook/core/v1alpha1/dhcprelay_webhook.go @@ -0,0 +1,61 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import ( + "context" + + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/ironcore-dev/network-operator/api/core/v1alpha1" +) + +// log is for logging in this package. +var dhcpRelaylog = logf.Log.WithName("dhcprelay-resource") + +// SetupDHCPRelayWebhookWithManager registers the webhook for DHCPRelay in the manager. +func SetupDHCPRelayWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr, &v1alpha1.DHCPRelay{}). + WithValidator(&DHCPRelayCustomValidator{}). + Complete() +} + +// +kubebuilder:webhook:path=/validate-networking-metal-ironcore-dev-v1alpha1-dhcprelay,mutating=false,failurePolicy=Fail,sideEffects=None,groups=networking.metal.ironcore.dev,resources=dhcprelays,verbs=create;update,versions=v1alpha1,name=dhcprelay-v1alpha1.kb.io,admissionReviewVersions=v1 + +// DHCPRelayCustomValidator struct is responsible for validating the DHCPRelay resource +// when it is created, updated, or deleted. +type DHCPRelayCustomValidator struct{} + +var _ admission.Validator[*v1alpha1.DHCPRelay] = &DHCPRelayCustomValidator{} + +// ValidateCreate implements admission.Validator so a webhook will be registered for the type DHCPRelay. +func (v *DHCPRelayCustomValidator) ValidateCreate(_ context.Context, DHCPRelay *v1alpha1.DHCPRelay) (admission.Warnings, error) { + dhcpRelaylog.Info("Validation for DHCPRelay upon creation", "name", DHCPRelay.GetName()) + + var warnings admission.Warnings + if len(DHCPRelay.Spec.InterfaceRefs) > 0 { //nolint:staticcheck // handling deprecated field for backward compatibility + warnings = append(warnings, "spec.interfaceRefs is deprecated; use the interfaceRef field on the DHCPRelay resource instead") + } + + return warnings, nil +} + +// ValidateUpdate implements admission.Validator so a webhook will be registered for the type DHCPRelay. +func (v *DHCPRelayCustomValidator) ValidateUpdate(_ context.Context, _, DHCPRelay *v1alpha1.DHCPRelay) (admission.Warnings, error) { + dhcpRelaylog.Info("Validation for DHCPRelay upon update", "name", DHCPRelay.GetName()) + + var warnings admission.Warnings + if len(DHCPRelay.Spec.InterfaceRefs) > 0 { //nolint:staticcheck // handling deprecated field for backward compatibility + warnings = append(warnings, "spec.interfaceRefs is deprecated; use the interfaceRef field on the DHCPRelay resource instead") + } + + return warnings, nil +} + +// ValidateDelete implements admission.Validator so a webhook will be registered for the type DHCPRelay. +func (v *DHCPRelayCustomValidator) ValidateDelete(_ context.Context, _ *v1alpha1.DHCPRelay) (admission.Warnings, error) { + return nil, nil +} diff --git a/internal/webhook/core/v1alpha1/dhcprelay_webhook_test.go b/internal/webhook/core/v1alpha1/dhcprelay_webhook_test.go new file mode 100644 index 000000000..7faf99c2e --- /dev/null +++ b/internal/webhook/core/v1alpha1/dhcprelay_webhook_test.go @@ -0,0 +1,59 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/ironcore-dev/network-operator/api/core/v1alpha1" +) + +var _ = Describe("DHCPRelay Webhook", func() { + var ( + obj *v1alpha1.DHCPRelay + oldObj *v1alpha1.DHCPRelay + validator DHCPRelayCustomValidator + ) + + BeforeEach(func() { + obj = &v1alpha1.DHCPRelay{ + Spec: v1alpha1.DHCPRelaySpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: "leaf1"}, + }, + } + oldObj = &v1alpha1.DHCPRelay{} + validator = DHCPRelayCustomValidator{} + Expect(validator).NotTo(BeNil(), "Expected validator to be initialized") + Expect(oldObj).NotTo(BeNil(), "Expected oldObj to be initialized") + Expect(obj).NotTo(BeNil(), "Expected obj to be initialized") + }) + + AfterEach(func() { + // TODO (user): Add any teardown logic common to all tests + }) + + Context("Deprecated InterfaceRefs field", func() { + It("returns deprecation warning on create when InterfaceRefs is set", func() { + obj.Spec.InterfaceRefs = []v1alpha1.LocalObjectReference{{Name: "eth0"}} //nolint:staticcheck + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).NotTo(HaveOccurred()) + Expect(warnings).To(ContainElement(ContainSubstring("spec.interfaceRefs is deprecated"))) + }) + + It("returns no warning on create when InterfaceRefs is not set", func() { + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).NotTo(HaveOccurred()) + Expect(warnings).To(BeEmpty()) + }) + + It("returns deprecation warning on update when InterfaceRefs is set", func() { + newObj := obj.DeepCopy() + newObj.Spec.InterfaceRefs = []v1alpha1.LocalObjectReference{{Name: "eth0"}} //nolint:staticcheck + warnings, err := validator.ValidateUpdate(ctx, obj, newObj) + Expect(err).NotTo(HaveOccurred()) + Expect(warnings).To(ContainElement(ContainSubstring("spec.interfaceRefs is deprecated"))) + }) + }) +}) diff --git a/internal/webhook/core/v1alpha1/webhook_suite_test.go b/internal/webhook/core/v1alpha1/webhook_suite_test.go index 68d6f8549..7ae4c6b21 100644 --- a/internal/webhook/core/v1alpha1/webhook_suite_test.go +++ b/internal/webhook/core/v1alpha1/webhook_suite_test.go @@ -113,6 +113,9 @@ var _ = BeforeSuite(func() { err = SetupAccessControlListWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) + err = SetupDHCPRelayWebhookWithManager(mgr) + Expect(err).NotTo(HaveOccurred()) + // +kubebuilder:scaffold:webhook go func() { diff --git a/test/gnmi/testdata/cisco-nxos-gnmi/dhcprelay.txtar b/test/gnmi/testdata/cisco-nxos-gnmi/dhcprelay.txtar index 52aae6856..8147ce6aa 100644 --- a/test/gnmi/testdata/cisco-nxos-gnmi/dhcprelay.txtar +++ b/test/gnmi/testdata/cisco-nxos-gnmi/dhcprelay.txtar @@ -1,3 +1,14 @@ +-- vrfs/vrf-1 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: VRF +metadata: + name: vrf-1 + namespace: default +spec: + deviceRef: + name: device + name: VRF-PURPLE + -- vlans/vlan100 -- apiVersion: networking.metal.ironcore.dev/v1alpha1 kind: VLAN @@ -8,7 +19,19 @@ spec: deviceRef: name: device id: 100 - name: DHCP-VLAN + name: DHCP-VLAN1 + +-- vlans/vlan200 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: VLAN +metadata: + name: vlan200 + namespace: default +spec: + deviceRef: + name: device + id: 200 + name: DHCP-VLAN2 -- interfaces/svi100 -- apiVersion: networking.metal.ironcore.dev/v1alpha1 @@ -28,19 +51,57 @@ spec: addresses: - 192.168.100.1/24 --- dhcprelays/relay -- + +-- interfaces/svi200 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: Interface +metadata: + name: svi200 + namespace: default +spec: + deviceRef: + name: device + name: Vlan200 + adminState: Up + type: RoutedVLAN + vlanRef: + name: vlan200 + ipv4: + addresses: + - 192.168.101.1/24 + + +-- dhcprelays/relay100 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: DHCPRelay +metadata: + name: relay100 + namespace: default +spec: + deviceRef: + name: device + interfaceRef: + name: svi100 + servers: + - "10.0.0.10" + + +-- dhcprelays/relay200 -- apiVersion: networking.metal.ironcore.dev/v1alpha1 kind: DHCPRelay metadata: - name: relay + name: relay200 namespace: default spec: deviceRef: name: device + interfaceRef: + name: svi200 + vrfRef: + name: vrf-1 servers: - "10.0.0.10" - interfaceRefs: - - name: svi100 + - "10.0.1.10" -- state/preload -- { @@ -54,6 +115,12 @@ spec: -- state/expect -- { "System": { + "inst-items": { + "Inst-list": [{ + "name": "VRF-PURPLE", + "descr": "DME_UNSET_PROPERTY_MARKER" + }] + }, "bd-items": { "bd-items": { "BD-list": [ @@ -61,7 +128,13 @@ spec: "BdState": "active", "adminSt": "active", "fabEncap": "vlan-100", - "name": "DHCP-VLAN" + "name": "DHCP-VLAN1" + }, + { + "BdState": "active", + "adminSt": "active", + "fabEncap": "vlan-200", + "name": "DHCP-VLAN2" } ] } @@ -80,6 +153,15 @@ spec: ] }, "id": "vlan100" + }, + { + "addr-items": { + "RelayAddr-list": [ + {"address": "10.0.0.10", "vrf": "VRF-PURPLE"}, + {"address": "10.0.1.10", "vrf": "VRF-PURPLE"} + ] + }, + "id": "vlan200" } ] } @@ -103,6 +185,10 @@ spec: { "ctrl": "port-unreachable", "id": "vlan100" + }, + { + "ctrl": "port-unreachable", + "id": "vlan200" } ] }, @@ -125,6 +211,17 @@ spec: "tDn": "/System/inst-items/Inst-list[name='default']" }, "vlanId": 100 + }, + { + "adminSt": "up", + "descr": "", + "id": "vlan200", + "medium": "bcast", + "mtu": 1500, + "rtvrfMbr-items": { + "tDn": "/System/inst-items/Inst-list[name='default']" + }, + "vlanId": 200 } ] } @@ -148,6 +245,17 @@ spec: ] }, "id": "vlan100" + }, + { + "addr-items": { + "Addr-list": [{ + "addr": "192.168.101.1/24", + "pref": 0, + "tag": 0, + "type": "primary" + }] + }, + "id": "vlan200" } ] }, @@ -169,6 +277,9 @@ spec: "procsys-items": { "bootTime": "1700000000" }, + "inst-items": { + "Inst-list": [] + }, "bd-items": { "bd-items": { "BD-list": [] @@ -208,12 +319,7 @@ spec: { "name": "default", "if-items": { - "If-list": [ - { - "id": "vlan100", - "ctrl": "port-unreachable" - } - ] + "If-list": [] } } ] @@ -221,7 +327,11 @@ spec: } }, "dhcp-items": { - "inst-items": {} + "inst-items": { + "relayif-items": { + "RelayIf-list": [] + } + } } } } diff --git a/test/gnmi/testdata/cisco-nxos-gnmi/dhcprelay_deprecated.txtar b/test/gnmi/testdata/cisco-nxos-gnmi/dhcprelay_deprecated.txtar new file mode 100644 index 000000000..890e1048e --- /dev/null +++ b/test/gnmi/testdata/cisco-nxos-gnmi/dhcprelay_deprecated.txtar @@ -0,0 +1,315 @@ +-- vrfs/vrf-1 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: VRF +metadata: + name: vrf-1 + namespace: default +spec: + deviceRef: + name: device + name: VRF-PURPLE + +-- vlans/vlan100 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: VLAN +metadata: + name: vlan100 + namespace: default +spec: + deviceRef: + name: device + id: 100 + name: DHCP-VLAN1 + +-- vlans/vlan200 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: VLAN +metadata: + name: vlan200 + namespace: default +spec: + deviceRef: + name: device + id: 200 + name: DHCP-VLAN2 + +-- interfaces/svi100 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: Interface +metadata: + name: svi100 + namespace: default +spec: + deviceRef: + name: device + name: Vlan100 + adminState: Up + type: RoutedVLAN + vlanRef: + name: vlan100 + ipv4: + addresses: + - 192.168.100.1/24 + + +-- interfaces/svi200 -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: Interface +metadata: + name: svi200 + namespace: default +spec: + deviceRef: + name: device + name: Vlan200 + adminState: Up + type: RoutedVLAN + vlanRef: + name: vlan200 + ipv4: + addresses: + - 192.168.101.1/24 + +-- dhcprelays/relay -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: DHCPRelay +metadata: + name: relay + namespace: default +spec: + deviceRef: + name: device + servers: + - "10.0.0.10" + interfaceRefs: + - name: svi100 + - name: svi200 + vrfRef: + name: vrf-1 + +-- state/preload -- +{ + "System": { + "procsys-items": { + "bootTime": "1700000000" + } + } +} + +-- state/expect -- +{ + "System": { + "inst-items": { + "Inst-list": [{ + "name": "VRF-PURPLE", + "descr": "DME_UNSET_PROPERTY_MARKER" + }] + }, + "bd-items": { + "bd-items": { + "BD-list": [ + { + "BdState": "active", + "adminSt": "active", + "fabEncap": "vlan-100", + "name": "DHCP-VLAN1" + }, + { + "BdState": "active", + "adminSt": "active", + "fabEncap": "vlan-200", + "name": "DHCP-VLAN2" + } + ] + } + }, + "dhcp-items": { + "inst-items": { + "relayif-items": { + "RelayIf-list": [ + { + "addr-items": { + "RelayAddr-list": [ + { + "address": "10.0.0.10", + "vrf": "VRF-PURPLE" + } + ] + } + }, + { + "addr-items": { + "RelayAddr-list": [{ + "address": "10.0.0.10", + "vrf": "VRF-PURPLE" + }] + } + } + ] + } + } + }, + "fm-items": { + "dhcp-items": { + "adminSt": "enabled" + }, + "ifvlan-items": { + "adminSt": "enabled" + } + }, + "icmpv4-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "if-items": { + "If-list": [ + { + "ctrl": "port-unreachable", + "id": "vlan100" + }, + { + "ctrl": "port-unreachable", + "id": "vlan200" + } + ] + }, + "name": "default" + } + ] + } + } + }, + "intf-items": { + "svi-items": { + "If-list": [ + { + "adminSt": "up", + "descr": "", + "id": "vlan100", + "medium": "bcast", + "mtu": 1500, + "rtvrfMbr-items": { + "tDn": "/System/inst-items/Inst-list[name='default']" + }, + "vlanId": 100 + }, + { + "adminSt": "up", + "descr": "", + "id": "vlan200", + "medium": "bcast", + "mtu": 1500, + "rtvrfMbr-items": { + "tDn": "/System/inst-items/Inst-list[name='default']" + }, + "vlanId": 200 + } + ] + } + }, + "ipv4-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "if-items": { + "If-list": [ + { + "addr-items": { + "Addr-list": [ + { + "addr": "192.168.100.1/24", + "pref": 0, + "tag": 0, + "type": "primary" + } + ] + }, + "id": "vlan100" + }, + { + "addr-items": { + "Addr-list": [{ + "addr": "192.168.101.1/24", + "pref": 0, + "tag": 0, + "type": "primary" + }] + }, + "id": "vlan200" + } + ] + }, + "name": "default" + } + ] + } + } + }, + "procsys-items": { + "bootTime": "1700000000" + } + } +} + +-- state/delete -- +{ + "System": { + "procsys-items": { + "bootTime": "1700000000" + }, + "inst-items": { + "Inst-list": [] + }, + "bd-items": { + "bd-items": { + "BD-list": [] + } + }, + "fm-items": { + "ifvlan-items": { + "adminSt": "enabled" + }, + "dhcp-items": { + "adminSt": "enabled" + } + }, + "intf-items": { + "svi-items": { + "If-list": [] + } + }, + "ipv4-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [] + } + } + ] + } + } + }, + "icmpv4-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "if-items": { + "If-list": [] + } + } + ] + } + } + }, + "dhcp-items": { + "inst-items": {} + } + } +} From 750ded72c8a6007a430414bc8286a1d97c0eb5d5 Mon Sep 17 00:00:00 2001 From: Pujol Date: Fri, 4 Sep 2026 16:26:46 +0200 Subject: [PATCH 2/2] Make gNMI fixture cleanup lifecycle-aware Track fixture resources in creation order and tear them down in reverse, waiting for each deletion to complete before continuing. This preserves dependency ordering and avoids concurrent cleanup races during envtest. An example for this situation is the DHCPRelay, which references Interfaces and VRFs. If these resources are deleted the finalizer fails as it can't derive the name of the interface/vrf on the device (e.g., eth1/1). On NXOS this would imply having to leave the entire dhcp tree on the post-delete section of the fixture. Signed-off-by: Pujol --- test/gnmi/gnmi_suite_test.go | 2 + test/gnmi/gnmi_test.go | 75 ++++--------------- test/gnmi/testdata/cisco-nxos-gnmi/acl.txtar | 26 +------ .../interface_routed_vlan.txtar | 7 +- test/gnmi/testdata/openconfig/acl.txtar | 52 +------------ 5 files changed, 18 insertions(+), 144 deletions(-) diff --git a/test/gnmi/gnmi_suite_test.go b/test/gnmi/gnmi_suite_test.go index 7a67781cc..8d172e15c 100644 --- a/test/gnmi/gnmi_suite_test.go +++ b/test/gnmi/gnmi_suite_test.go @@ -13,6 +13,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -70,6 +71,7 @@ func TestGNMI(t *testing.T) { // It starts the gNMI test server, sets up the Kubernetes client, and starts the controller manager. var _ = BeforeSuite(func(ctx SpecContext) { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + format.MaxLength = 0 SetDefaultEventuallyTimeout(60 * time.Second) SetDefaultEventuallyPollingInterval(time.Second) diff --git a/test/gnmi/gnmi_test.go b/test/gnmi/gnmi_test.go index 804bdd5e7..35d8df90b 100644 --- a/test/gnmi/gnmi_test.go +++ b/test/gnmi/gnmi_test.go @@ -9,7 +9,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "time" . "github.com/benjamintf1/unmarshalledmatchers" @@ -20,11 +19,9 @@ import ( apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" - nxv1alpha1 "github.com/ironcore-dev/network-operator/api/cisco/nx/v1alpha1" "github.com/ironcore-dev/network-operator/api/core/v1alpha1" ) @@ -116,9 +113,13 @@ var _ = Describe("gNMI requests tests", func() { device.Status.Phase = v1alpha1.DevicePhaseRunning Expect(k8sClient.Status().Update(ctx, device)).To(Succeed()) + // Fixture resources must be declared from dependencies to dependents. + // Cleanup deletes them in reverse order. By(fmt.Sprintf("creating %d resource(s) from testdata", len(resources))) + createdResources := make([]client.Object, 0, len(resources)) for _, res := range resources { obj := createResourceFromTxtar(ctx, k8sClient, res, device.Name, testNamespace) + createdResources = append(createdResources, obj) waitForResource(ctx, k8sClient, obj) } @@ -131,8 +132,8 @@ var _ = Describe("gNMI requests tests", func() { g.Expect(stateJSON).To(MatchUnorderedJSON(statePost), "gNMI state does not match expected JSON") }).Should(Succeed()) - By("deleting all intermeadiate test resources created in test") - cleanupAllResources(k8sClient, testNamespace) + By("deleting all intermediate test resources created in test") + cleanupAllResources(k8sClient, createdResources) By("verifying gNMI state is empty after resource deletion") Eventually(func(g Gomega) { @@ -227,76 +228,26 @@ func extractConditions(obj *unstructured.Unstructured) ([]metav1.Condition, erro return conditions, json.Unmarshal(data, &conditions) } -// cleanupAllResources deletes all test resources in the proper order. +// cleanupAllResources deletes fixture resources in reverse creation order. // // This is an envtest workaround. In a real cluster, namespace deletion cascades to // all resources and the garbage collector handles ordering. But envtest runs without // kube-controller-manager, so there's no garbage collector and namespace deletion // just marks the namespace as Terminating without actually deleting anything. // See: https://book.kubebuilder.io/reference/envtest.html#testing-considerations -// -// The function: -// 1. Deletes resources with finalizers first and waits for their controllers -// to process the finalizers (cleaning up gNMI state) while Device still exists. -// 2. Deletes config-only resources (no finalizer, no controller) -// without waiting. -func cleanupAllResources(c client.Client, namespace string) { +func cleanupAllResources(c client.Client, createdResources []client.Object) { cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - allResources := listAllNetworkOperatorResources(cleanupCtx, c, namespace) - - var withFinalizers, withoutFinalizers []unstructured.Unstructured - for _, r := range allResources { - if len(r.GetFinalizers()) > 0 { - withFinalizers = append(withFinalizers, r) - } else { - withoutFinalizers = append(withoutFinalizers, r) - } - } - - // Delete resources with finalizers first and wait for controller to process - for i := range withFinalizers { - item := &withFinalizers[i] + for i := len(createdResources) - 1; i >= 0; i-- { + item := createdResources[i] Expect(client.IgnoreNotFound(c.Delete(cleanupCtx, item))).To(Succeed()) - } - for _, item := range withFinalizers { Eventually(func(g Gomega) { var check unstructured.Unstructured - check.SetGroupVersionKind(item.GroupVersionKind()) - err := c.Get(cleanupCtx, client.ObjectKeyFromObject(&item), &check) + check.SetGroupVersionKind(item.GetObjectKind().GroupVersionKind()) + err := c.Get(cleanupCtx, client.ObjectKeyFromObject(item), &check) + g.Expect(err).To(HaveOccurred()) g.Expect(client.IgnoreNotFound(err)).To(Succeed()) - g.Expect(apimeta.IsNoMatchError(err) || err != nil).To(BeTrue()) }).WithContext(cleanupCtx).Should(Succeed()) } - - // Delete config resources without finalizers (no wait needed) - for i := range withoutFinalizers { - item := &withoutFinalizers[i] - Expect(client.IgnoreNotFound(c.Delete(cleanupCtx, item))).To(Succeed()) - } -} - -// listAllNetworkOperatorResources lists all network-operator CRD instances in a namespace. -func listAllNetworkOperatorResources(ctx context.Context, c client.Client, namespace string) []unstructured.Unstructured { - var all []unstructured.Unstructured - - for gvk := range scheme.Scheme.AllKnownTypes() { - if strings.HasSuffix(gvk.Kind, "List") || gvk.Kind == "Device" { - continue - } - if gvk.Group != v1alpha1.GroupVersion.Group && gvk.Group != nxv1alpha1.GroupVersion.Group { - continue - } - - list := &unstructured.UnstructuredList{} - list.SetGroupVersionKind(gvk.GroupVersion().WithKind(gvk.Kind + "List")) - err := c.List(ctx, list, client.InNamespace(namespace)) - if apimeta.IsNoMatchError(err) { - continue - } - Expect(err).NotTo(HaveOccurred()) - all = append(all, list.Items...) - } - return all } diff --git a/test/gnmi/testdata/cisco-nxos-gnmi/acl.txtar b/test/gnmi/testdata/cisco-nxos-gnmi/acl.txtar index 12b4605e9..6c03855a8 100644 --- a/test/gnmi/testdata/cisco-nxos-gnmi/acl.txtar +++ b/test/gnmi/testdata/cisco-nxos-gnmi/acl.txtar @@ -72,7 +72,6 @@ spec: } -- state/delete -- - { "System": { "procsys-items": { @@ -81,30 +80,7 @@ spec: "acl-items": { "ipv4-items": { "name-items": { - "ACL-list": [ - { - "name": "BLOCK-EXTERNAL", - "seq-items": { - "ACE-list": [ - { - "seqNum": 20, - "action": "deny", - "protocol": 0, - "srcPrefix": "0.0.0.0", - "dstPrefix": "0.0.0.0" - }, - { - "seqNum": 10, - "action": "permit", - "protocol": 0, - "srcPrefix": "10.0.0.0", - "srcPrefixLength": 8, - "dstPrefix": "0.0.0.0" - } - ] - } - } - ] + "ACL-list": [] } } } diff --git a/test/gnmi/testdata/cisco-nxos-gnmi/interface_routed_vlan.txtar b/test/gnmi/testdata/cisco-nxos-gnmi/interface_routed_vlan.txtar index cbf426e3e..183b09264 100644 --- a/test/gnmi/testdata/cisco-nxos-gnmi/interface_routed_vlan.txtar +++ b/test/gnmi/testdata/cisco-nxos-gnmi/interface_routed_vlan.txtar @@ -173,12 +173,7 @@ spec: { "name": "default", "if-items": { - "If-list": [ - { - "id": "vlan10", - "ctrl": "port-unreachable" - } - ] + "If-list": [] } } ] diff --git a/test/gnmi/testdata/openconfig/acl.txtar b/test/gnmi/testdata/openconfig/acl.txtar index 78948654c..0f2653ee0 100644 --- a/test/gnmi/testdata/openconfig/acl.txtar +++ b/test/gnmi/testdata/openconfig/acl.txtar @@ -91,57 +91,7 @@ spec: { "openconfig-acl:acl": { "acl-sets": { - "acl-set": [ - { - "config": { - "name": "MGMT-ACL", - "type": "openconfig-acl:ACL_IPV4" - }, - "acl-entries": { - "acl-entry": [ - { - "sequence-id": 10, - "config": { - "sequence-id": 10, - "description": "Allow management subnet" - }, - "ipv4": { - "config": { - "source-address": "10.0.0.0/24", - "destination-address": "0.0.0.0/0", - "protocol": "6" - } - }, - "actions": { - "config": { - "forwarding-action": "openconfig-acl:ACCEPT" - } - } - }, - { - "sequence-id": 20, - "config": { - "sequence-id": 20, - "description": "Deny all other traffic" - }, - "ipv4": { - "config": { - "source-address": "0.0.0.0/0", - "destination-address": "0.0.0.0/0" - } - }, - "actions": { - "config": { - "forwarding-action": "openconfig-acl:DROP" - } - } - } - ] - }, - "name": "MGMT-ACL", - "type": "openconfig-acl:ACL_IPV4" - } - ] + "acl-set": [] } } }