Add CloudInitNetworkConfigFormat to ProxmoxCluster spec#593
Add CloudInitNetworkConfigFormat to ProxmoxCluster spec#593voteblake wants to merge 1 commit intoionos-cloud:mainfrom
Conversation
Add a new field to ProxmoxCluster spec that controls the format of cloud-init network-config generation. Supports two formats: - netplan (default): Generates config with top-level "network:" key, suitable for netplan-based distributions like Ubuntu and Debian. - nocloud: Generates config without the "network:" wrapper, required for cloud-init nocloud datasource consumers like Talos Linux.
voteblake
requested review from
65278,
mcbenjemaa,
pborn-ionos and
wikkyk
as code owners
TestsPlease note that running unit and e2e tests requires manual approval from a team member. e2e testsWe use labels to control which e2e tests contexts are run:
ℹ️ Ask a team member to add the requested labels if you don't have enough permissions. |
|
Thanks for the contribution. |
|
|
This is definitely interesting and valuable. As v1alpha1 is frozen and we're focusing on v1alpha2, I'll put this on hold until v1alpha2 is released and we'll revisit it then. |
|
@voteblake Now that v0.8.0 has been released, are you able to revisit your PR? It needs porting to v1alpha2 but that shouldn't be a huge change. |
wikkyk
left a comment
wikkyk
left a comment
There was a problem hiding this comment.
This is a good PR. It needs just a few tweaks.
| "github.com/luthermonson/go-proxmox" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" |
There was a problem hiding this comment.
| infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" | |
| infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha2" |
And update references to infrav1alpha1 elsewhere.
| // ErrNoCloudUnsupportedFeature is returned when nocloud format is used with | ||
| // features only supported by netplan (VRFs, routing-policy). | ||
| ErrNoCloudUnsupportedFeature = errors.New("nocloud format does not support VRFs or routing-policy; use netplan format or remove these features") |
There was a problem hiding this comment.
It's a pedantic nitpick but can you make capitalisation consistent? Either nocloud or NoCloud.
I know that not everything is consistent but I want the consistency to improve :-)
| "gopkg.in/yaml.v3" | ||
| "k8s.io/utils/ptr" | ||
|
|
||
| infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" |
There was a problem hiding this comment.
| infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" | |
| infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha2" |
| IPAddress: "10.10.10.12/24", | ||
| Gateway: "10.10.10.1", | ||
| Metric: ptr.To(uint32(100)), | ||
| DNSServers: []string{"8.8.8.8", "8.8.4.4"}, |
There was a problem hiding this comment.
Please use documentation ranges as per #693 (I'll update contribution guidelines).
There was a problem hiding this comment.
Could use more test scenarios:
- 1 NIC that is not
eth0 - multiple NICs with unordered names
- IPv6
- no gateway/metric/DNS
Plus exercise error handling more, e.g. missing mandatory fields, invalid values.
2 participants
Issue #, if available: Fixes #94, #51
Description of changes:
Add a new field to ProxmoxCluster spec that controls the format of cloud-init network-config generation. Supports two formats:
Explicit fails on two CAPMOX supported features that cannot be articulated in a pure nocloud cloud-init network-config, that otherwise rely on netplan passthrough. There would be a maintenance burden to this, as mentioned in #94.
I appreciate your time and effort maintaining this repo and considering this change. I'm happy to make any requested changes to the extent of my knowledge and ability.
Testing performed:
In tree tests (fmt, lint, vet, test), using an iso with the new generated network-config with a Talos Linux VM.