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..f0d082c68 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,33 @@ 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) + } + + 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 +705,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..7d764412f 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()) @@ -272,6 +369,95 @@ var _ = Describe("DHCPRelay Controller", func() { Expect(k8sClient.Delete(ctx, duplicateDHCPRelay)).To(Succeed()) }) + 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() { By("Creating the custom resource for the Kind DHCPRelay") dhcprelay = &v1alpha1.DHCPRelay{ @@ -280,11 +466,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 +527,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 +545,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 +649,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} - }) + 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) - 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()) - }) - - 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 +844,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 +996,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 +1004,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..e502ac1ed 100644 --- a/internal/controller/core/suite_test.go +++ b/internal/controller/core/suite_test.go @@ -1046,19 +1046,6 @@ func (p *Provider) DeleteDHCPRelay(_ context.Context, req *provider.DHCPRelayReq 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/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/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": {} + } + } +} 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": [] } } }