⚠️ Changes to make IPAM deployable with CAPI

[peppi-lotta]

These changes allow metal3 ip-address-manager(IPAM) to be deployed with cluster-api (CAPI). Currently deploying IPAM with CAPI assumes the CRDs are in a git release and the release has a metadata.yaml. IPAM's current release doesn't have a metadata.yaml so this need to be added. Action need to be taken to get IPAM as an ipam-provider for CAPI so that metal3IPAM can be specified when running clusterctl init with the --ipam flag.

IPAM's ipam-components.yaml has CRDs for ipClaim and ipAddresses which work but IPAM is unable to resolve the ipAddresClaims that CAPI has defined. This is a next step after these changes are
meged to make IPAM resolve CAPI's ipAddressClaims. Check more in CAPI ipam contract

[metal3-io-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign furkatgofurov7 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Rozzii]

/cc @lentzi90 @zaneb @kashifest @schrej @mboukhalfa

[Rozzii]

/cc @furkatgofurov7

[lentzi90]

This is a bit of a mouth full. Could we simplify it a bit?
I suggest ipam-metal3-system. For reference the CAPI in-cluster IPAM has the namespace capi-ipam-in-cluster-system so following that we would get capi-ipam-metal3-system, but since we can use this also without CAPI, I think we can drop the capi- prefix.

Suggested change

	namespace: ipam-provider-m3-system
	namespace: ipam-metal3-system

[furkatgofurov7]

+1

[schrej]

for the in-cluster provider I went with capi-ipam-in-cluster-, so it could be capi-ipam-metal3- to stay consistent here. I'd argue that it might help to be consistent, so it's easier to navigate for people that are new to CAPI, but it's entirely up to you.

[lentzi90]

This prefix must match so that it + the namespace name (=system) becomes the namespace above.

Suggested change

	namePrefix: ipam-
	namePrefix: ipam-metal3-

[kashifest]

@furkatgofurov7 @schrej can we get some reviews from you folks on this ?

[furkatgofurov7]

AFAIK there is no way to make it work without CAPM3 changes, can we verify both changes in CI (should be possible to run CI from CAPM3 pointing to the branch of current change)

[furkatgofurov7]

nit: we could directly place ns inside the manager.yaml same way as in https://github.com/kubernetes-sigs/cluster-api-ipam-provider-in-cluster/blob/main/config/manager/manager.yaml#L1-L6

[furkatgofurov7]

+1

[schrej]

Currently deploying IPAM with CAPI assumes the path to the CRDs has a directory named 'in-cluster' in it

What exactly do you mean with this? That the --ipam flag of clusterctl expects in-cluster? Or did we hardcode some directory into capi tooling somewhere? If yes, that's probably a mistake.

IPAM's ipam-components.yaml has CRDs for ipClaim and ipAddresses which work but IPAM is unable to resolve the ipAddresClaims that CAPI has defined.

Imho we should avoid deploying custom claims and addresses when using clusterctl to install metal3 ipam. If the plan is to support the CAPI variants of those resources anyway, deploying the custom resources is probably superfluous, and will only lead to confusion (and annoying autocompletion). As long as the official resources aren't supported, I'd also not accept the addition of the metal3 ipam to the official list of providers integrated in clusterctl, as it's not compatible with other providers. Not sure if the CAPI team has the same opinion here.

[lentzi90]

@peppi-lotta can you update the description? The part about "in-cluster directory" was a misunderstanding if I recall correctly

[kashifest]

Imho we should avoid deploying custom claims and addresses when using clusterctl to install metal3 ipam. If the plan is to support the CAPI variants of those resources anyway, deploying the custom resources is probably superfluous, and will only lead to confusion (and annoying autocompletion). As long as the official resources aren't supported, I'd also not accept the addition of the metal3 ipam to the official list of providers integrated in clusterctl, as it's not compatible with other providers. Not sure if the CAPI team has the same opinion here.

I want to understand this part a bit more. I can agree to the point that metal3 ipam needs to handle the CAPI variant of the IPAM resources to be accepted as a CAPI IPAM provider. But on the point of deploying metal3 specific resources and CRDs, it should remain as it is now since if a user want to deploy a metal3 IPAM then the user is aware that metal3 IPAM allows you to have this custom claims on top and offers you some additional workflow. So I dont see any harm deploying those. Otherwise if someone doesnt want metal3 IPAM but only CAPI specific variants then the current IPAM provider should be enough.

[Rozzii]

What is the current state of this ticket ? @peppi-lotta could you please summarize it here? The PR went silent in February.

[peppi-lotta]

These changes along metal3-io/cluster-api-provider-metal3#1442 help make it possible to deploy this ip-address-manager with clusterctl. This has been on hold while I've been researching the possibility to switch to use in-cluster-ipam in CAPM3. This is currently not possible so I'll continue to make this ip-address-manager an IPAM-provider for CAPI. I'll hopefully be able to continue with this soon.

[metal3-io-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues will close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[Rozzii]

/remove-lifecycle stale