Skip to content

feat: Add IPAM integration#505

Open
appkins wants to merge 1 commit intotinkerbell:mainfrom
tinkerbell-community:main
Open

feat: Add IPAM integration#505
appkins wants to merge 1 commit intotinkerbell:mainfrom
tinkerbell-community:main

Conversation

@appkins
Copy link
Copy Markdown

@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
jacobweinstock
jacobweinstock reviewed Oct 12, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@jacobweinstock jacobweinstock left a comment •
edited
Loading

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!

jacobweinstock
jacobweinstock requested changes Oct 12, 2025
View reviewed changes
@appkins appkins force-pushed the main branch 2 times, most recently from 601847e to 8661d4b Compare October 17, 2025 04:56
@appkins
feat: Add IPAM integration
94ecbf2 
- Added RBAC rules
- Fixed reconcile

Signed-off-by: appkins <nbatkins@gmail.com>
@appkins
Copy link
Copy Markdown
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
Copy Markdown
Member

jacobweinstock commented Oct 18, 2025 •
edited
Loading

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

Reviewers

@jacobweinstock jacobweinstock Awaiting requested review from jacobweinstock

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@appkins @jacobweinstock