duplicated properties in CRD / not working hardware.spec.interfaces.netboot.ipxe.content

firecyberice commented 1 year ago

There is a bit of confusion when defining a CR for hardware because some fields can be defined in different subtrees of the hardware.tinkerbell.org CRD

Expected Behaviour

I would expect only one place to define e.g. ipxe configuration or userdata

Current Behaviour

For ipxe there is this

hardware.spec.interfaces[].netboot.ipxe.contents
hardware.spec.interfaces[].netboot.ipxe.url

and this hardware.spec.metadata.instance.ipxe_script_url config option available.

Similar for hardware.spec.metadata.instance.userdata and

hardware.spec.userData
hardware.spec.vendorData

Additionally it looks like those hardware.spec.interfaces.netboot.ipxe.[ url | content ] options are not working.

Possible Solution

If there is an old and a new location for those settings I would like to see at least a deprecation notice in the description of the specific properties.

Your Environment

I am running tinkerbell via helm install in kubernetes to bootstrap a harvesterhci.io cluster

Steps to Reproduce (for bugs)

After installing the stack (with helm install) execute

kubectl explain hardware.spec.interfaces.netboot.ipxe.contents
kubectl explain hardware.spec.interfaces.netboot.ipxe.url
kubectl explain hardware.spec.metadata.instance.ipxe_script_url

kubectl explain hardware.spec.metadata.instance.userdata
kubectl explain hardware.spec.userData

firecyberice commented 1 year ago

@jacobweinstock @chrisdoherty4 Do you have any opinions / news on that?

chrisdoherty4 commented 1 year ago

Hi @firecyberice

I agree the existing API is confusing. We have an unreleased v1alpha2 API that reworks all the CRDs to be much more understandable and easier to work with so the issues you describe should disappear reasonably soon.

If there's more you think we should do we'd love to hear your feedback. You can find the API proposal at https://github.com/tinkerbell/roadmap/blob/main/design/20230222_tinkerbell_crd_refactor.md and the code in https://github.com/tinkerbell/tink/tree/main/api/v1alpha2.

firecyberice commented 1 year ago

Because those duplicate fields are already mentioned here I am closing this but feel free to open it if necessary.

tinkerbell / charts