Add Vlan and VlanClaim support - #618
Conversation
bruelea
left a comment
bruelea
left a comment
There was a problem hiding this comment.
Thanks for taking over the implementation of the controllers for vlans, @flynn-nrg .
The structure of the controllers and test coverage looks good! Currently, some changes are needed for the claim/update logic of the vlans in NetBox to make sure different custom resources won't interfere with each other.
| return fmt.Errorf("failed to list vlans: %w", err) | ||
| } | ||
| if httpResp.StatusCode != http.StatusOK { | ||
| return fmt.Errorf("failed to list vlans: status %d", httpResp.StatusCode) |
There was a problem hiding this comment.
Here the response body could be added to the error message, for some information about why the request failed.
| @@ -0,0 +1,16 @@ | |||
| --- | |||
There was a problem hiding this comment.
This sample failed to claim a vlan when I tested it. "{"site":["Select a valid choice. DM-Akron is not one of the available choices."]}"
| @@ -0,0 +1,16 @@ | |||
| --- | |||
There was a problem hiding this comment.
If two vlans with only a different metadata.name and vid are created, they "overwrite" each other in NetBox. Is the name field needed or could metadata.name be used? Instead of by name the vlans could be looked up by site and id.
| } | ||
|
|
||
| // vlan cannot be restored from netbox | ||
| if o.Spec.Vid != 0 { |
There was a problem hiding this comment.
The other controllers of NetBox Operattor do not support to define the desired value in the claim. This option could be a good addition to the claim, but in this case the claim controller should check if the object is still free in NetBox before assigning the Vid to a vlan CR.
2 participants
Add Vlan and VlanClaim support
Adds VLAN management to the NetBox Operator, following the same claim/resource pattern already used for IP addresses, prefixes, and IP ranges.
New CRDs
Vlanrepresents a single VLAN in NetBox (name, vid, site, tenant, status, comments, description, custom fields, preserveInNetbox).VlanClaimclaims a VLAN ID either as an exactvidor from avidRangeStart/vidRangeEndrange, and creates an ownedVlanCR.vidand the range fields are mutually exclusive (CEL validation) and immutable once set. VID allocation is scoped to.spec.site, since VLAN IDs are commonly only unique within a site.Only
tenantandsitereferences are supported for now (both reuse existing lookup helpers).Controllers
VlanReconcilerreserves/updates the VLAN in NetBox (ReserveOrUpdateVlan), finalizer-based cleanup (vlan.netbox.dev/finalizer), lease-locks the parent VID range while the Vlan is not yet Ready, reports conditions and events.VlanClaimReconcilerrestores a previously assigned VID by hash before falling back to the explicitvidor a new range-based allocation, syncs mutable fields down to the ownedVlan, lease-locks per site+range to avoid races between concurrent claims (vlanclaim.netbox.dev/finalizer).NetBox client
New methods on
NetboxCompositeClient:ReserveOrUpdateVlan,DeleteVlan,RestoreExistingVlanByHash,GetAvailableVlanByClaim(scans existing VLANs scoped to the claim's site, since NetBox has no dedicated "available VLAN IDs" endpoint outside of VLAN Groups). VLAN support was added directly to the existingIpamAPIinterface/adapter. NewVlan/VlanClaimmodels and regenerated mocks for theIpamVlans*API surface. ThenetboxOperatorRestorationHashcustom field is extended to coveripam.vlanin the kind data-load job.Tests & docs
pkg/netbox/api/vlan_test.go,vlan_claim_test.go) and for the controllers (internal/controller/vlan_controller_test.go,vlanclaim_controller_test.go), covering reservation, update, restoration-hash mismatch, reserve failure, range allocation, restore-by-hash, site-scoping, and lease-lock contention.tests/e2e/vlan: explicit/range apply-update, range restore, range-exhausted, invalid tenant/custom-field, and owner-reference cases.config/samples/.Vlan/VlanClaimmodel and usage.Behaviour notes
preserveInNetbox: truekeeps the Vlan in NetBox after CR deletion, enabling later reclaim.name,vid,site, andtenantimmutability enforced via CEL validation rules on bothVlanandVlanClaim.site, which (unlike a controlled enum) is free text, so it's sanitized to satisfy the Lease resource's DNS-1123 naming requirements.This was done with the help of Claude.