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

Pools should allow for NOT specifying gateways #89

Closed
tylerschultz opened this issue Mar 14, 2023 · 1 comment · Fixed by #90
Closed

Pools should allow for NOT specifying gateways #89

tylerschultz opened this issue Mar 14, 2023 · 1 comment · Fixed by #90

Comments

@tylerschultz
Copy link
Contributor

The IPAM CRDs supplied by CAPI suggest that Gateway is not a required field. CAPV is written so that gateway can be supplied on the VSphereMachine, removing the need to configure it on the pool. CAIP Validation allows for an empty Gateway, but later fails to Parse the empty Gateway here:

https://github.com/telekom/cluster-api-ipam-provider-in-cluster/blob/main/internal/poolutil/pool.go#L59

CAIP should honor it's validations and not error on an empty gateway.
adobley pushed a commit to tylerschultz/cluster-api-ipam-provider-in-cluster that referenced this issue Mar 14, 2023
@tylerschultz
Gateway can be omitted from pool spec
a90f6b3 
This change allows the Gateway to be omitted from the Pool, preventing
an error. Also, prior to this change, Gateway was required when specifying
`poo.spec.addresses`.

We took this opportunity to move what we considered business logic out
of the pool utitility package, and brought it back closer to the other
business logic in the controller. The gateway is conditionally added to
the list of in use IPs in the controller.

fixes: kubernetes-sigs#89

Signed-off-by: Tyler Schultz <[email protected]>
Co-authored-by: Aidan Obley <[email protected]>
adobley pushed a commit to tylerschultz/cluster-api-ipam-provider-in-cluster that referenced this issue Mar 14, 2023
@tylerschultz
Gateway can be omitted from pool spec
fe46e55 
This change allows the Gateway to be omitted from the Pool, preventing
an error. Also, prior to this change, Gateway was required when specifying
`poo.spec.addresses`.

We took this opportunity to move what we considered business logic out
of the pool utitility package, and brought it back closer to the other
business logic in the controller. The gateway is conditionally added to
the list of in use IPs in the controller.

fixes: kubernetes-sigs#89

Signed-off-by: Tyler Schultz <[email protected]>
Co-authored-by: Aidan Obley <[email protected]>
adobley pushed a commit to tylerschultz/cluster-api-ipam-provider-in-cluster that referenced this issue Mar 14, 2023
@tylerschultz
Gateway can be omitted from pool spec
cda5bba 
This change allows the Gateway to be omitted from the Pool, preventing
an error. Also, prior to this change, Gateway was required when specifying
`pool.spec.addresses`.

We took this opportunity to move what we considered business logic out
of the pool utility package, and brought it back closer to the other
business logic in the controller. The gateway is conditionally added to
the list of in use IPs in the controller.

fixes: kubernetes-sigs#89

Signed-off-by: Tyler Schultz <[email protected]>
Co-authored-by: Aidan Obley <[email protected]>
@tylerschultz tylerschultz mentioned this issue Mar 14, 2023
Merged
@tylerschultz
Copy link
Contributor Author

This issue is a duplicate of this other I opened a while back.
#70
--Sir-spam-a-lot

flawedmatrix added a commit to tylerschultz/cluster-api-ipam-provider-in-cluster that referenced this issue Mar 23, 2023
@tylerschultz @flawedmatrix
Gateway can be omitted from pool spec
e6d192f 
This change allows the Gateway to be omitted from the Pool, preventing
an error. Also, prior to this change, Gateway was required when specifying
`pool.spec.addresses`.

We took this opportunity to move what we considered business logic out
of the pool utility package, and brought it back closer to the other
business logic in the controller. The gateway is conditionally added to
the list of in use IPs in the controller.

fixes: kubernetes-sigs#89

Signed-off-by: Tyler Schultz <[email protected]>
Co-authored-by: Aidan Obley <[email protected]>
Co-authored-by: Edwin Xie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Gateway can be omitted from pool spec tylerschultz/cluster-api-ipam-provider-in-cluster
1 participant
@tylerschultz