API returns numbers from get that it can't parse when fed into update

[a-robinson]

I spin up a new cluster using GKE at version 0.9.2. Then I run:
$kubectl get minion -o yaml > minion.yaml

This returns a yaml file:
apiVersion: v1beta1
creationTimestamp: 2015-02-03T01:16:46Z
hostIP:
id:
kind: Node
labels:
diskSize: "massive"
resourceVersion: 12
resources:
capacity:
cpu: "1"
memory: 4.0265318e+09
selfLink: /api/v1beta1/minions/
status:
conditions:

• kind: Ready
  lastTransitionTime: null
  status: Full
  uid: 526a4156-ab42-11e4-9817-42010af0258d

I delete the hostIp and status so that the update request won't get rejected (#3005 should fix that), then run:
$ kubectl update -f minion.yaml
ERROR: F0202 18:08:19.054514 767 resource.go:212] error unmarshaling JSON: json: cannot unmarshal number 4.0265318e+09 into Go value of type int

I'm not sure whether the marshaling or the unmarshaling is what needs to be fixed, but the API shouldn't be returning numbers that it can't parse.

[thockin]

It looks like Quantity is unmarshalling wrong?

On Mon, Feb 2, 2015 at 6:10 PM, Alex Robinson [email protected]
wrote:

I spin up a new cluster using GKE at version 0.9.2. Then I run:
$kubectl get minion -o yaml > minion.yaml

This returns a yaml file:
apiVersion: v1beta1
creationTimestamp: 2015-02-03T01:16:46Z
hostIP:
id:
kind: Node
labels:
diskSize: "massive"
resourceVersion: 12
resources:
capacity:
cpu: "1"
memory: 4.0265318e+09
selfLink: /api/v1beta1/minions/
status:
conditions:

• kind: Ready lastTransitionTime: null status: Full uid:
  526a4156-ab42-11e4-9817-42010af0258d

I delete the hostIp and status so that the update request won't get
rejected (#3005
#3005 should
fix that), then run:
$ kubectl update -f minion.yaml
ERROR: F0202 18:08:19.054514 767 resource.go:212] error unmarshaling JSON:
json: cannot unmarshal number 4.0265318e+09 into Go value of type int

I'm not sure whether the marshaling or the unmarshaling is what needs to
be fixed, but the API shouldn't be returning numbers that it can't parse.

Reply to this email directly or view it on GitHub
#4041.

[a-robinson]

That's very likely. And sorry, I didn't realize yaml didn't paste into comments well. Here's the actual yaml:

apiVersion: v1beta1
creationTimestamp: 2015-02-03T01:16:46Z
hostIP: <ip>
id: <minion-id>
kind: Node
labels:
    diskSize: "massive"
resourceVersion: 12
resources:
  capacity:
    cpu: "1"
    memory: 4.0265318e+09
selfLink: /api/v1beta1/minions/<minion-id>
status:
  conditions:
  - kind: Ready
    lastTransitionTime: null
    status: Full
uid: 526a4156-ab42-11e4-9817-42010af0258d

[a-robinson]

Yeah, on the client-side, apparently, since the error is in kubectl/cmd/resource.go. It's possible that was missed during the Quantity refactoring.
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/kubectl/cmd/resource.go#L212

It may be troublesome that the v1beta1 API accepts a util.IntOrString for resources rather than a Quantity:
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/api/v1beta1/types.go#L635

[bgrant0607]

/cc @vishh

[vishh]

The (external) yaml library that we use is the cause for this. I will send a patch to fix the yaml library.

[vishh]

Sent a PR to our yaml dependency.

[cjcullen]

If we are only fixing our yaml unmarshalling, we are still losing precision.

E.g.
The yaml that Alex posted above is supposed to be representing 3.75 GiB.
3.75 GiB * 2^30 = 4026531840, which we are representing as 4.0265318e+09.
The amount of memory we are "losing" here is trivial, but if this is actually using scientific notation, we could be rounding up half the time as well (creating a very very low chance of an unexpected OOM).

Both of these are pretty small problems, but it seems silly to needlessly lose precision.
Can we just not write these in scientific notation?

[thockin]

Resource quantities must be lossless.

On Wed, Feb 4, 2015 at 11:20 AM, CJ Cullen [email protected] wrote:

If we are only fixing our yaml unmarshalling, we are still losing
precision.

E.g.
The yaml that Alex posted above is supposed to be representing 3.75 GiB.
3.75 GiB * 2^30 = 4026531840, which we are representing as 4.0265318e+09.
The amount of memory we are "losing" here is trivial, but if this is
actually using scientific notation, we could be rounding up half the time
as well (creating a very very low chance of an unexpected OOM).

Both of these are pretty small problems, but it seems silly to needlessly
lose precision.
Can we just not write these in scientific notation?

Reply to this email directly or view it on GitHub
#4041 (comment)
.

[bgrant0607]

+1. These are supposed to be strings, not integers nor floats.

[bgrant0607]

What does -o json return?

[a-robinson]

{
    ...
    "resources": {
        "capacity": {
            "cpu": "1",
            "memory": 4026531800
        }
    },
    ...
}

[bgrant0607]

That does look like an API bug. It should be a string, IIUC.

[bgrant0607]

Legacy CPU and memory are int and int64, respectively.
Resource.Quantity should always produce strings.

[bgrant0607]

This can't possibly work. IntOrString is a 32-bit signed int. This value doesn't fit in 32 bits.

[bgrant0607]

#2264

[ghodss]

Yes - this isn't a problem with the YAML library. This is a problem with trying to render a number larger than 32 bits (4026531840) as a 32-bit integer (IntOrString), which instead ends up rendering as a floating point. #2264 (or something that keeps this value as a 64-bit integer the whole time) is the right fix.

[vishh]

@ghodss: The yaml library converts an int64 into a float64 as part of Marshal. Isn't that an issue with the yaml library? I agree that the kubernetes API has another issue which is that of not being able to represent resources greater than MaxInt. But to me these are two separate issues.

[vishh]

@ghodss: Even an int32 gets marshaled into a floating point number. I don't see how that is valid. The underlying golang json package has separate APIs to address this behavior and the YAML library will also have to do that.

[lavalamp]

IntOrString stores things as "int", which is 32 or 64 bits depending on the platform. We should explicitly use int32 or int64 in our api objects.

These values should be stored as a Quantity in v1beta3, and converted to a string in v1beta1 & 2.

[vishh]

@dbsmith: Ack

On Fri, Feb 13, 2015 at 1:29 PM, Daniel Smith [email protected]
wrote:

IntOrString stores things as "int", which is 32 or 64 bits depending on
the platform. We should explicitly use int32 or int64 in our api objects.

These values should be stored as a Quantity in v1beta3, and converted to a
string in v1beta1 & 2.

—
Reply to this email directly or view it on GitHub
#4041 (comment)
.

[thockin]

If someone changed IntOrString to use int64 I would approve.

On Fri, Feb 13, 2015 at 1:32 PM, Vish Kannan [email protected]
wrote:

@dbsmith: Ack

On Fri, Feb 13, 2015 at 1:29 PM, Daniel Smith [email protected]
wrote:

IntOrString stores things as "int", which is 32 or 64 bits depending on
the platform. We should explicitly use int32 or int64 in our api objects.

These values should be stored as a Quantity in v1beta3, and converted to
a
string in v1beta1 & 2.

Reply to this email directly or view it on GitHub
<
#4041 (comment)

.

Reply to this email directly or view it on GitHub
#4041 (comment)
.

[vishh]

@thockin: Assigned #2264 to myself.