v1beta3: make kubeadm types better support CRDs

[neolit123]

EDIT: neolit123

issue repurposed to track the addition of +optional tags.
ObjectMeta is now out of scope for the time being.

PR:
kubernetes/kubernetes#101925

---

users such as kind or those that apply kustomize patches over the kubeadm config types need to manually add:

metadata:
  name: config

to your types in serialized form as kustomize requires them.

however this later results in strict unmarshal errors (warnings) when the same types are fed into kubeadm because "metadata" is not a known field.

this can be resolved if we start embedding ObjectMeta in the types.

this seems to be a quirk of the ecosystem (kustomize) and while our types are not really "API" types (more of a component-config), it seems reasonable to make the change.

also i think @fabriziopandini already said that this seems OK to him.
(please correct me if needed)

alternatively a subset of ObjectMeta via a custom type can work too, but i will prefer the full type even if we don't need most of the fields.

---

ComponentConfig ObjectMeta KEP:
kubernetes/enhancements#1816

[rosti]

The only field we can make use of is the name field. Everything else is targeted into, either supporting the object type by the API server, or supporting multiple instances of that object.
In particular, the name field is meant to distinguish multiple instances of the object type (described in TypeMeta).
For some of the types (notably, InitConfiguration and JoinConfiguration) it may be appropriate to distinguish multiple different objects, based not on metadata.name, but on nodeRegistration.name (after all, that value will be used for the Node object's metadata.name value).
ClusterConfiguration is a completely different thing. You don't expect more than one of these in a cluster. Hence, it's a "singleton" and having a separate metadata.name field is a bit senseless.

If the only user of metadata.name is kustomize, then I do believe, that there is a workaround to that. Kustomize supports redirecting that field to something else, hence users can redirect it to nodeRegistration.name for InitConfiguration and JoinConfiguration, and to kind for ClusterConfiguration.

Hence, I don't like the idea of adding a full blown ObjectMeta to types, that are opaque to the API server and usually stored in files and config maps.

[neolit123]

Hence, I don't like the idea of adding a full blown ObjectMeta to types, that are opaque to the API server and usually stored in files and config maps.

CAPI did something similar because of bugs in unmarshaling related to kubebuilder.
they created a custom ObjectMeta with only the fields they need.

no strong opinions on my side, so i think a custom ObjectMeta is fine.

ClusterConfiguration is a completely different thing. You don't expect more than one of these in a cluster. Hence, it's a "singleton" and having a separate metadata.name field is a bit senseless.

unless someone creates multi-cluster topologies using kubeadm?

[rosti]

ClusterConfiguration is a completely different thing. You don't expect more than one of these in a cluster. Hence, it's a "singleton" and having a separate metadata.name field is a bit senseless.

unless someone creates multi-cluster topologies using kubeadm?

Yep, I forgot about that. In that case clusterName is the field to be used in place of metadata.name.

[neolit123]

it really seems that metada.name for our types is to only satisfy kustomize.

[neolit123]

it seems patches can use a few more fields like namespace and annotations:
https://github.com/kubernetes/kubeadm/pull/1724/files#diff-d937f45ffd1a04bea321ca4d15977b0cR65

[fabriziopandini]

I'm +1 making our config API objects more similar to other K8s objects, with this regard, I expect support at least for name, labels, and annotations

[fejta-bot]

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

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

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[neolit123]

/remove-lifecycle stale

[fejta-bot]

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

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

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[neolit123]

/remove-lifecycle stale

[fejta-bot]

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

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

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fabriziopandini]

/remove-lifecycle stale

[neolit123]

/lifecycle active

[neolit123]

note, we decided to not add ObjectMeta in kubeadm types for the time being (in v1beta3)
we are only adding +optional to omitempty fields.

the open PR will close this issue as it is now reporposed:
kubernetes/kubernetes#101925

in the future we can revisit the topic, but currently there is no evidence of demand outside of kustomize.