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.

Sign up for GitHub

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

Jump to bottom

large number in ThirdPartyResource encoded as float #30213

Closed
tatsuhiro-t opened this issue Aug 8, 2016 · 5 comments · Fixed by #44026
Closed

large number in ThirdPartyResource encoded as float #30213

tatsuhiro-t opened this issue Aug 8, 2016 · 5 comments · Fixed by #44026
Labels
area/apiserver area/custom-resources sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@tatsuhiro-t
Copy link
Contributor

When we submit the resource object containing large integer, its field is encoded as floating point with exponents, like 1.03e+9. We want to encode the number in integer, not floating point, since with the latter we may lose precision, and Go cannot decode such field into int64.

For example, submitting the following third party resource object:

apiVersion: foo.example.com/v1alpha1
kind: Foo
metadata:
  name: f1
spec:
  num: 1000000009

and getting result from apiserver looks like this:

"size":1.000000009e+09

Small integer, like 100, is encoded as just 100 as we expected.

We use Kuberentes 1.4.0-alpha.2
@k8s-github-robot k8s-github-robot added area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Aug 9, 2016
@adohe-zz
Copy link

adohe-zz commented Aug 9, 2016

@brendandburns

@tatsuhiro-t
Copy link
Contributor Author

Any news for this issue?

@tatsuhiro-t
Copy link
Contributor Author

It looks like 999999 is OK. But 1000000 is encoded as 1e+06.

@bgrant0607 bgrant0607 mentioned this issue Feb 7, 2017
Closed
nikhita added a commit to nikhita/kubernetes that referenced this issue Mar 28, 2017
@nikhita
[WIP] Preserve int data when unmarshalling for TPR
9aa5f6f 
The Go json package converts all numbers to float64.
This exposes many of the int64 fields to corrution when marshalled back to json.

The json package provided by kubernetes also provides a way to defer conversion of numbers
(https://golang.org/pkg/encoding/json/#Decoder.UseNumber) and does the conversions to int or float.

Fixes kubernetes#30213
nikhita added a commit to nikhita/kubernetes that referenced this issue Mar 28, 2017
@nikhita
[WIP] Preserve int data when unmarshalling for TPR
86d54b2 
The Go json package converts all numbers to float64.
This exposes many of the int64 fields to corruption when marshalled back to json.

The json package provided by kubernetes also provides a way to defer conversion of numbers
(https://golang.org/pkg/encoding/json/#Decoder.UseNumber) and does the conversions to int or float.

Fix for kubernetes#30213
@liggitt
Copy link
Member

liggitt commented Apr 4, 2017 •
edited

same root issue as #16964

looks like something in the TPR pipeline needs to switch to use k8s.io/apimachinery/pkg/util/json#Unmarshal to preserve int data

nikhita added a commit to nikhita/kubernetes that referenced this issue Apr 4, 2017
@nikhita
Preserve int data when unmarshalling for TPR
badca51 
The Go json package converts all numbers to float64.
This exposes many of the int64 fields to corruption when marshalled back to json.

The json package provided by kubernetes also provides a way to defer conversion of numbers
(https://golang.org/pkg/encoding/json/#Decoder.UseNumber) and does the conversions to int or float.

This is also implemented in the custom json package. See:
(https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/json/json.go)

Fixes kubernetes#30213
@nikhita nikhita mentioned this issue Apr 4, 2017
Merged
@nikhita
Copy link
Member

nikhita commented Apr 4, 2017

Please see #44026

nikhita added a commit to nikhita/kubernetes that referenced this issue Apr 5, 2017
@nikhita
Preserve int data when unmarshalling for TPR
eb88c4b 
The Go json package converts all numbers to float64.
This exposes many of the int64 fields to corruption when marshalled back to json.

The json package provided by kubernetes also provides a way to defer conversion of numbers
(https://golang.org/pkg/encoding/json/#Decoder.UseNumber) and does the conversions to int or float.

This is also implemented in the custom json package. See:
(https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/json/json.go)

Fixes kubernetes#30213

Update bazel build and add one more test case

Fix for gofmt error
k8s-github-robot pushed a commit that referenced this issue Apr 14, 2017
Merge pull request #44026 from nikinath/precision-json
9ef911e 
Automatic merge from submit-queue (batch tested with PRs 44424, 44026, 43939, 44386, 42914)

Preserve int data when unmarshalling for TPR

**What this PR does / why we need it**:

The Go json package converts all numbers to float64 while unmarshalling.
This exposes many of the int64 fields to corruption when marshalled back to json.

The json package provided by kubernetes also provides a way to defer conversion of numbers
(https://golang.org/pkg/encoding/json/#Decoder.UseNumber) and does the conversions to int or float.

This is also implemented in the custom json package. See:
(https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/json/json.go)

Now, the number is preserved as an integer till the highest int64 number - `9223372036854775807`.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #30213

**Special notes for your reviewer**: See also #16964

**Release note**:

```
NONE
```
jayunit100 pushed a commit to jayunit100/kubernetes that referenced this issue Apr 25, 2017
@nikhita @jayunit100
Preserve int data when unmarshalling for TPR
c69a2a2 
The Go json package converts all numbers to float64.
This exposes many of the int64 fields to corruption when marshalled back to json.

The json package provided by kubernetes also provides a way to defer conversion of numbers
(https://golang.org/pkg/encoding/json/#Decoder.UseNumber) and does the conversions to int or float.

This is also implemented in the custom json package. See:
(https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/json/json.go)

Fixes kubernetes#30213

Update bazel build and add one more test case

Fix for gofmt error
nikhita added a commit to nikhita/kubernetes that referenced this issue May 18, 2017
@nikhita
Add integration test for kube-apiextensions-server.
e9a8af9 
Check if integers are present after decoding.
Originally an issue for TPRs: kubernetes#30213
@nikhita nikhita mentioned this issue May 18, 2017
Merged
crassirostris pushed a commit to crassirostris/kubernetes that referenced this issue May 19, 2017
Merge pull request kubernetes#46059 from nikhita/test-int-preserve
ff46073 
Automatic merge from submit-queue (batch tested with PRs 46075, 46059, 46095, 46097)

Integration test for kube-apiextensions-server: integers

**What this PR does / why we need it**: Check if integers are present after decoding.
Originally an issue for TPRs: kubernetes#30213

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: for kubernetes#45511 

**Special notes for your reviewer**:

**Release note**:

```
NONE
```
@sttts
@deads2k deads2k mentioned this issue May 18, 2017
Closed
19 tasks
ruvr pushed a commit to sapcc/kubernetes-operators that referenced this issue Dec 7, 2017
workaround kubernetes/kubernetes#30213
6cd0cda
ruvr pushed a commit to sapcc/openstack-helm that referenced this issue Dec 7, 2017
workaround kubernetes/kubernetes#30213
044492e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/apiserver area/custom-resources sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Preserve int data when unmarshalling for TPR nikhita/kubernetes
6 participants
@tatsuhiro-t @liggitt @adohe-zz @grodrigues3 @k8s-github-robot @nikhita