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

resource.Quantity.Format isn't set consistently for each v1/type resource #81379

Closed
egernst opened this issue Aug 13, 2019 · 3 comments

Comments

@egernst
Copy link
Contributor

commented Aug 13, 2019

What happened:
Each v1/type Resource type should have a particular format for its quantity. In assessing appropriate values for resource's Quantity.Format (ie, BinarySI, DecimalSI), I see that usage for a given type is not consistent (particularly within test code).

What you expected to happen:

I think these should be set consistently for a given define resource type.

How to reproduce it (as minimally and precisely as possible):

For example, looking at the type ResourceStorage, from top of k/k source tree:
git grep ResourceStorage | grep -E BinarySI | wc
4 16 467
git grep ResourceStorage | grep -E DecimalSI | wc
20 80 2752

Anything else we need to know?:

Happy to fix this - just wanted confirmation first that it should be consistent, and follow what is detailed here:

        switch resourceName {
        case v1.ResourceCPU:
                requestQuantity = resource.Quantity{Format: resource.DecimalSI}
        case v1.ResourceMemory:
                requestQuantity = resource.Quantity{Format: resource.BinarySI}
        case v1.ResourceStorage:
                requestQuantity = resource.Quantity{Format: resource.BinarySI}
        case v1.ResourceEphemeralStorage:
                requestQuantity = resource.Quantity{Format: resource.BinarySI}
        }

Environment:

Master k/k codebase

@egernst egernst added the kind/bug label Aug 13, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

@egernst: There are no sig labels on this issue. Please add a sig label by either:

  1. mentioning a sig: @kubernetes/sig-<group-name>-<group-suffix>
    e.g., @kubernetes/sig-contributor-experience-<group-suffix> to notify the contributor experience sig, OR

  2. specifying the label manually: /sig <group-name>
    e.g., /sig scalability to apply the sig/scalability label

Note: Method 1 will trigger an email to the group. See the group list.
The <group-suffix> in method 1 has to be replaced with one of these: bugs, feature-requests, pr-reviews, test-failures, proposals.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@egernst

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

/cc @dashpole for help on clarifying expectations.

@dashpole

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

The proposed values look correct to me

@egernst egernst closed this Aug 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.