✨ Add IPAM for nodes #142
✨ Add IPAM for nodes #142
Conversation
Mattes83
requested review from
piepmatz,
lubedacht,
gfariasalves-ionos,
mcbenjemaa,
wikkyk and
jriedel-ionos
as code owners
Mattes83
force-pushed
the
ipam
branch
4 times, most recently
from
30775fb
to
354436f
Compare
|
History should now be clean |
|
|
The commit history of this PR is somehow really broken. There are too many unrelated files marked as changed. |
|
Ah...again |
| @@ -155,6 +155,9 @@ type IonosCloudMachineSpec struct { | |||
| //+optional | |||
| AdditionalNetworks Networks `json:"additionalNetworks,omitempty"` | |||
There was a problem hiding this comment.
| AdditionalNetworks Networks `json:"additionalNetworks,omitempty"` | |
| // +listType=map | |
| // +listMapKey=networkID | |
| AdditionalNetworks Networks `json:"additionalNetworks,omitempty"` |
There was a problem hiding this comment.
For the documentation, I'd just write a new markdown file in docs/. Also have you tested your changes? If so, please proceed with e2e tests, as #137 has already been merged
| // IPAMConfig contains the config for ip address management. | ||
| type IPAMConfig struct { | ||
| // IPv4PoolRef is a reference to an IPAMConfig Pool resource, which exposes IPv4 addresses. | ||
| // The nic will use an available IP address from the referenced pool. |
| } | ||
|
|
||
| func (r *IonosCloudMachineReconciler) reconcileNormal( | ||
| ctx context.Context, cloudService *cloud.Service, machineScope *scope.Machine, | ||
| ctx context.Context, machineScope *scope.Machine, cloudService *cloud.Service, |
There was a problem hiding this comment.
I changed it so that both reconcileNormal funcs have the same order of arguments.
Shall I revert this?
There was a problem hiding this comment.
I'd prefer the inverted order (cluster, machine) but just a nitpick
internal/service/ipam/ipam.go
Outdated
| } | ||
| } | ||
|
|
||
| // default nic ipv6. |
Yes, it works and we have a running cluster built with this change. |
|
I will try to add docs and tests this week, but I am not sure if I will find time for it. Next week and the week after I'll be on holidays |
| @@ -0,0 +1,40 @@ | |||
| /* | |||
There was a problem hiding this comment.
move this to types.go. I think it is small enough to also include those types
| */ | ||
|
|
||
| // Package ipam offers services for IPAM management. | ||
| package ipam |
There was a problem hiding this comment.
I'm not sure about an ipam package. As we already have a cloud package, which contains the general functions for IONOS Cloud, it would rather make sense to me if we then had a k8s package, containing functions to interact with Kubernetes.
Otherwise you would have a core.NewHelper and apps.NewHelper and so on
| PrimaryNICFormat = "nic-%s" | ||
|
|
||
| // AdditionalNICFormat is the format used for IPAddressClaims for additional nics. | ||
| AdditionalNICFormat = "nic-%s-%d" | ||
|
|
||
| // IPV4Format is the IP v4 format. | ||
| IPV4Format = "ipv4" | ||
|
|
||
| // IPV6Format is the IP v6 format. | ||
| IPV6Format = "ipv6" |
There was a problem hiding this comment.
Is there a need to export these consts to other packages?
| IPv4PoolRef *corev1.TypedLocalObjectReference `json:"ipv4PoolRef,omitempty"` | ||
|
|
||
| // IPv6PoolRef is a reference to an IPAMConfig pool resource, which exposes IPv6 addresses. | ||
| // The nic will use an available IP address from the referenced pool. |
What is the purpose of this pull request/Why do we need it?
We would like to get IPs from a fixed pool of IPs instead of relying on the DHCP.
Issue #, if available:
#130
Description of changes:
Special notes for your reviewer:
I did not write tests yet as I am waiting for #137 to be merged and I'd like to get some feedback about this PR first.
I am also unsure where I should put the docs, I did not find anything for the other api stuff beside the api definition itself. Maybe this is already enough?
Checklist: