Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support reservation of IpRanges #130

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Support reservation of IpRanges #130

wants to merge 16 commits into from
+3,158 −73

Conversation

bruelea
Copy link
Collaborator

@bruelea bruelea commented Nov 15, 2024 •
edited by henrybear327
Loading

Implements #137

@bruelea bruelea self-assigned this Nov 15, 2024
@bruelea bruelea marked this pull request as draft November 15, 2024 07:58
@bruelea bruelea force-pushed the feat/support-reservation-of-ipranges branch 2 times, most recently from 6dd0a08 to 549e32a Compare November 18, 2024 13:06
@bruelea bruelea force-pushed the feat/support-reservation-of-ipranges branch from 549e32a to ff7f155 Compare November 25, 2024 12:43
@bruelea bruelea force-pushed the feat/support-reservation-of-ipranges branch from e45d379 to 10e2307 Compare November 26, 2024 09:34
@bruelea bruelea force-pushed the feat/support-reservation-of-ipranges branch from a58fc71 to 16ae6db Compare November 26, 2024 09:56
alexandernorth
alexandernorth requested changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@alexandernorth alexandernorth left a comment

Choose a reason for hiding this comment

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

Nice work, please take a look at the comments and let me know if there is something I should clarify :)

api/v1/iprange_types.go Show resolved Hide resolved
api/v1/iprange_types.go Outdated Show resolved Hide resolved
api/v1/iprangeclaim_types.go Outdated Show resolved Hide resolved
api/v1/iprangeclaim_types.go Outdated Show resolved Hide resolved
cmd/main.go Outdated Show resolved Hide resolved
internal/controller/iprangeclaim_controller.go Outdated Show resolved Hide resolved
internal/controller/iprangeclaim_controller.go Outdated Show resolved Hide resolved
internal/controller/iprangeclaim_controller.go Outdated Show resolved Hide resolved
internal/controller/iprangeclaim_controller.go Outdated Show resolved Hide resolved
internal/controller/iprangeclaim_controller.go Outdated Show resolved Hide resolved
henrybear327
henrybear327 reviewed Nov 26, 2024
View reviewed changes
api/v1/iprangeclaim_types.go Outdated Show resolved Hide resolved
api/v1/iprangeclaim_types.go Outdated Show resolved Hide resolved
internal/controller/iprange_controller.go Outdated Show resolved Hide resolved
internal/controller/iprange_controller.go Outdated Show resolved Hide resolved
internal/controller/iprangeclaim_controller.go Show resolved Hide resolved
kind/load-data-job/local-demo-data.sql Outdated Show resolved Hide resolved
kind/load-data-job/local-demo-data.sql Outdated Show resolved Hide resolved
pkg/netbox/api/ip_range_claim.go Show resolved Hide resolved
pkg/netbox/api/ip_range_claim.go Outdated Show resolved Hide resolved
@henrybear327
Copy link
Collaborator

Good work @bruelea :)

bruelea and others added 2 commits November 27, 2024 08:20
@bruelea bruelea force-pushed the feat/support-reservation-of-ipranges branch from 7f5720b to 5a96faa Compare November 27, 2024 14:55
@bruelea bruelea force-pushed the feat/support-reservation-of-ipranges branch from 5a96faa to 87bdd6c Compare November 27, 2024 15:12
alexandernorth
alexandernorth reviewed Nov 27, 2024
View reviewed changes
internal/controller/ipaddress_controller.go Outdated Show resolved Hide resolved
internal/controller/iprange_controller.go Show resolved Hide resolved
internal/controller/iprange_controller.go Outdated Show resolved Hide resolved
internal/controller/iprangeclaim_controller.go Show resolved Hide resolved
internal/controller/prefix_controller.go Outdated Show resolved Hide resolved
internal/controller/utils.go Outdated Show resolved Hide resolved
internal/controller/utils.go Outdated Show resolved Hide resolved
internal/controller/utils.go Outdated Show resolved Hide resolved
pkg/netbox/api/ip_range_claim.go Show resolved Hide resolved
pkg/netbox/api/ip_range_claim.go Outdated Show resolved Hide resolved
@bruelea bruelea force-pushed the feat/support-reservation-of-ipranges branch from df836d6 to 9c96d13 Compare November 28, 2024 09:46
@henrybear327 henrybear327 marked this pull request as ready for review November 28, 2024 09:46
@henrybear327
Copy link
Collaborator

LGTM! When CI pipelines are fixed and the comments are all resolved, I will approve it ;)

Good job @bruelea

@bruelea bruelea force-pushed the feat/support-reservation-of-ipranges branch from 9c96d13 to 19bdf4f Compare November 28, 2024 09:58
@henrybear327
Copy link
Collaborator

Please create a tracking GitHub issue for the IPRange Testing :)

@bruelea bruelea force-pushed the feat/support-reservation-of-ipranges branch from 19bdf4f to afc40a6 Compare November 29, 2024 07:28
alexandernorth
alexandernorth approved these changes Nov 29, 2024
View reviewed changes
Copy link
Contributor

@alexandernorth alexandernorth left a comment

Choose a reason for hiding this comment

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

Great work @bruelea !

henrybear327
henrybear327 approved these changes Nov 29, 2024
View reviewed changes
Copy link
Collaborator

@henrybear327 henrybear327 left a comment

Choose a reason for hiding this comment

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

Good job @bruelea !

api/v1/iprange_types.go Show resolved Hide resolved
jstudler
jstudler reviewed Nov 29, 2024
View reviewed changes
api/v1/iprangeclaim_types.go
//+kubebuilder:validation:Required
//+kubebuilder:validation:Minimum=2
//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'size' is immutable"
Size int `json:"size,omitempty"`
Copy link
Contributor

@jstudler jstudler Nov 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There is a current de-facto hard limit here which is at 50 (should be reproducible by trying to create a iprc with .spec.size 51 in an empty /16 prefix) and a soft limit that might be even lower (depending on the fragmentation of IPs/IP Ranges in the prefix). I assume this is due to pagination in the API. I propose to add validation Maximum=50 here and add a comment to this field but leave the code as is for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a validation rule for maximum value for size until pagination is added to the function searching an available iprange.

@bruelea bruelea requested a review from jstudler December 2, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henrybear327 henrybear327 henrybear327 approved these changes

@alexandernorth alexandernorth alexandernorth approved these changes

@jstudler jstudler Awaiting requested review from jstudler

Assignees

@bruelea bruelea

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bruelea @henrybear327 @alexandernorth @jstudler