Skip to content

Conversation

appkins
Copy link

@appkins appkins commented Oct 11, 2025

This pull request introduces dynamic IP address allocation for Tinkerbell machines by integrating with Cluster API IPAM providers. The changes add support for specifying an IPAM pool in the machine spec, automatically creating and managing IP address claims, updating hardware resources with allocated IPs, and cleaning up claims on deletion. Comprehensive unit tests for the new IPAM logic have also been added.

IPAM integration and dynamic IP allocation:

  • Added the IPAMPoolRef field to the TinkerbellMachineSpec, allowing users to specify an IPAM pool for dynamic IP address allocation.
  • Implemented the IPAM workflow in controller/machine/ipam.go, including logic for creating IP address claims, updating hardware with allocated IPs, and cleaning up claims.

Controller logic enhancements:

  • Updated the machine reconciliation logic in scope.go to handle IPAM pool configuration, trigger IP allocation, and update hardware resources accordingly.
  • Enhanced machine deletion logic to remove the associated IPAddressClaim if IPAM was configured, ensuring proper resource cleanup. [1] [2]

Testing improvements:

  • Added unit tests in ipam_test.go to cover netmask conversion, hardware patching, and IPAM pool reference extraction, improving reliability and maintainability of the new IPAM logic.## Description

Fixes: #

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

@appkins appkins force-pushed the main branch 13 times, most recently from 0dcfd63 to b33eb28 Compare October 12, 2025 06:21
Copy link
Member

@jacobweinstock jacobweinstock left a comment

Choose a reason for hiding this comment

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

Thanks for this, @appkins. I actually tried to use ko but i couldn't get it to allow overwriting the registry. I'd also prefer the changes to the build and release process be a separate PR. I'll start reviewing the rest of the PR. Thanks again for this!

@appkins appkins force-pushed the main branch 2 times, most recently from 601847e to 8661d4b Compare October 17, 2025 04:56
- Added RBAC rules
- Fixed reconcile

Signed-off-by: appkins <[email protected]>
@appkins
Copy link
Author

appkins commented Oct 17, 2025

Thanks for this, @appkins. I actually tried to use ko but i couldn't get it to allow overwriting the registry. I'd also prefer the changes to the build and release process be a separate PR. I'll start reviewing the rest of the PR. Thanks again for this!

All ko related changes are removed and will be in a new PR shortly.

@jacobweinstock
Copy link
Member

jacobweinstock commented Oct 18, 2025

Hey @appkins, I tried running this with the CAPT playground and got a bunch of nil pointer errors. I threw in some fixes for nil values but then ended up with errors around failed to ensure hardware: failed to reconcile IPAM: IPAddressClaim is nil. Have you been able to run this? Maybe I'm just doing something wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants