Warnings in registry-k8s logs

[szvincze]

The logs of registry-k8s contains the following warnings from time to time:

W1129 08:04:22.757792       1 warnings.go:70] unknown field "status"
W1129 08:04:28.957701       1 warnings.go:70] unknown field "status"

It seems a warning cannot be printed out because of the unknown field. I observed the same behavior in NSM 1.10.0 up to 1.11.2-rc.1 when just running the kernel2ethernet2kernel example, but I found it in logs received from other environments too.

[arp-est]

Hi

I think the problem is the missing status here:

diff --git a/apps/registry-k8s/crd-nse.yaml b/apps/registry-k8s/crd-nse.yaml
index fde462a8dd8..3367c6d78a6 100644
--- a/apps/registry-k8s/crd-nse.yaml
+++ b/apps/registry-k8s/crd-nse.yaml
@@ -26,3 +26,5 @@ spec:
           properties:
             spec:
               x-kubernetes-preserve-unknown-fields: true
+            status:
+              x-kubernetes-preserve-unknown-fields: true

In a bit more detail, the structs used in types.go does not match the ones in crd-nse.yaml

Now its a bit weird as NetworkService struct too has a Status field and no warning was printed out, I guess kubernetes is not so fussy about an empty struct. Also I couldn't really find where or how these Status fields are used in nsm, so If they aren't used anywhere, then removing them from the struct could also be a possible fix. (I checked both, either adding the upper field to the yaml, or removing the Status field from types.go, seemed to make the warnings disappear)

[denis-tingaikin]

Hello, @arp-est,

Thanks for doing this.

From my point of view, the problem with generating status fields comes from https://github.com/networkservicemesh/sdk-k8s/blob/771dae1f75a5a96c110a4fceccef4bf4e80ea36c/pkg/tools/k8s/gen.go#L19-L24.

We can start by looking at the K8S generator and trying to play with it to prevent generating status fields.

If it's not possible, then we can merge your suggested patch with adding the status field to both nses and ns.

After that, I think we could create a separate ticket with removing x-kubernetes-preserve-unknown-fields: true and defining the nse/ns model schemes.

[arp-est]

I'll summarize my findings,

So I think that removing the staus field, could be as simple as removing them from types.go.

Now I see that at the top in 'types.go' it states in a comment that "// Code generated by client-gen. DO NOT EDIT.", but based on the docs I've read up so far, and actually running the code generator I came to the conclusion that this comment is likely false.

According to these documents: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-api-machinery/generating-clientset.md https://www.redhat.com/en/blog/kubernetes-deep-dive-code-generation-customresources

The client-gen is only used to generate stuff under the 'client' folder, and the deepcopy function and is based on the contents of types.go.

Also when running gen.go, you can observe the same behaviour, that the deepcopy, and client code is generated, but types.go is not.

Thirdly the license headers are also different, the dates do not match, let's say between the deepcopy and types.go.

A bit different topic, but here, they frame the stentence in a way, that having a status field is the recommendation, while not necessary. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#objects

All objects that represent a physical resource whose state may vary from the user's desired intent SHOULD have a spec and a status. Objects whose state cannot vary from the user's desired intent MAY have only spec, and MAY rename spec to a more appropriate name.

So overall I think its a lot less painful to just go with the first thought, and add the field to the crd yamls :'D What do you think?

[denis-tingaikin]

Fixed with version v1.14.0.

@arp-est Thanks!