Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serializing struct field to nil #257

Closed
kyessenov opened this issue Jul 28, 2017 · 10 comments
Closed

Serializing struct field to nil #257

kyessenov opened this issue Jul 28, 2017 · 10 comments

Comments

@kyessenov
Copy link

Field sizeLimit defined here https://godoc.org/k8s.io/kubernetes/pkg/api#EmptyDirVolumeSource, can never be nil. How do I invoke the special k8s serializer to make it omit from the resulting JSON/YAML?

@mandarjog
Copy link

@jingxu97 Can you help us here? kubernetes/kubernetes@85f030c

For us this breaks k8s 1.6 deployment with Istio. Details are in the referenced issue.
Thanks.

@caesarxuchao
Copy link
Member

Why is it necessary to omit the field during marshalling? Normally adding a new field doesn't break backwards compatibility. Decoder will just ignore the unknown fields.

I also tried marshalling with the standard json encoder:

	e := v1.EmptyDirVolumeSource{}
	b, _ := json.Marshal(e)
	fmt.Printf("%s\n", b)

The output is {"sizeLimit":"0"}. So the problem is not related to "special k8s serializer".

@kyessenov
Copy link
Author

kyessenov commented Aug 3, 2017

Here's what happens. We take k8s objects, read them with client-go 1.7, serialize with client-go 1.7, and then send them to k8s 1.6 with kubectl apply. sizeLimit does not exist in k8s API definition, and the apiserver rejects our processed objects:
error: error validating "STDIN": error validating data: found invalid field sizeLimit for v1.EmptyDirVolumeSource; if you choose to ignore these errors, turn validation off with --validate=false

@caesarxuchao
Copy link
Member

The error message pasted in istio/old_pilot_repo#975 is from kubectl, not generated by apiserver. What if you do kubectl apply --validate=false as suggested by the error message?

@kyessenov
Copy link
Author

I believe it would work, but we have to support other cloud providers for now, who are still on 1.6 and have 1.6 apiservers.

@caesarxuchao
Copy link
Member

client-go is working as expected. client-go doesn't guarantee compatibility with kubectl.
You can try to fix that particular sizeLimit field, but I suspect there are more such new fields in 1.7.
I think you should avoid the round-tripping in client-go, either

  • write dump server's response body to a file directly
  • or use the dynamic client, which umarshals to map[string]interface{} and won't introduce new fields.

@kyessenov
Copy link
Author

That's unfortunate. What's your recommendation for k8s JSON processing library that's backwards compatible?

@caesarxuchao
Copy link
Member

The golang standard library behaves the same, so i don't think there will be a library that does what you want.

Do you know why the field is not omitted even though the field has the omitempty tag?

@kyessenov
Copy link
Author

omitempty cannot be applied to struct field. if it was a pointer, it would work.

@caesarxuchao
Copy link
Member

@kyessenov indeed the API is wrong, but it has to be fixed in kubernetes rather than in client-go. According to the api-convention.md, "Therefore, we ask that pointers always be used with optional fields that do not have a built-in nil value." Hence, SizeLimit being a non-pointer is a bug. Could you file an issue in kubernetes, assign it to the author of that API, and request it to be cherrypicked to 1.7?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants