Skip to content

Conversation

@Strangerxxx
Copy link

@Strangerxxx Strangerxxx commented Oct 21, 2025

What this PR does / why we need it:
It basically strips subnet mask suffix from the NIC IP for BaremetalHost, so it became correctly recognized as IP by other providers of ClusterAPI ecosystem.

Previously IP was a string like: 37.27.255.255/16

After this changes IP will become a string 37.27.255.255

TODOs:

  • squash commits
  • include documentation
  • add unit tests

@Strangerxxx Strangerxxx changed the title strip subnet mask from ip Strip subnet mask from HetznerBareMetalHost IP Address Oct 21, 2025
@guettli
Copy link
Collaborator

guettli commented Oct 29, 2025

@Strangerxxx looks good. But please elaborate the PR description first. Please provide an example of the current and the new IP

@Strangerxxx
Copy link
Author

@guettli thank you for review!

I've add examples you've asked for.

Please let me know what else should be done.

I'm happy to fix it asap :)

@guettli
Copy link
Collaborator

guettli commented Oct 29, 2025

@Strangerxxx I know that the term "IP" does not match the content (37.27.255.255/16), because it is a "subnet".

If you are looking for the IP, then you can use spec.status.ipv4.

You propose a change to spec.status.hardwareDetails.nics[].ip. This field is not defined in cluster api. The reason "capi ecosystem" ... please elaborate what you mean.

Currently, I am slightly against this change, because I have concerns that removing /16 could break existing tooling build upon caph.

Please elaborate your current pain point, and how this PR would help.

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