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

Bump the pod memory to higher levels to work on power #73016

Merged
merged 1 commit into from
Jan 19, 2019

Conversation

mkumatag
Copy link
Member

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change

/kind bug

/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

ppc64le architecture machines by default pagesize is 64K (vs 4K on intel), this usually end up using more memory for th workloads, so to run this testcase successfully, need to increase the pod memory limits to higher lavel.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 17, 2019
@mkumatag
Copy link
Member Author

/sig testing

@k8s-ci-robot k8s-ci-robot added sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 17, 2019
@mkumatag
Copy link
Member Author

/assign @vishh

@mkumatag
Copy link
Member Author

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 17, 2019
@mkumatag
Copy link
Member Author

/assign @Random-Liu

@mkumatag
Copy link
Member Author

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jan 17, 2019
@mkumatag
Copy link
Member Author

/test pull-kubernetes-e2e-kops-aws

@mkumatag
Copy link
Member Author

/cc @dims

@dims
Copy link
Member

dims commented Jan 17, 2019

/assign @yujuhong

@kubernetes/sig-node-pr-reviews

@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Jan 17, 2019
@fedebongio
Copy link
Contributor

/remove-sig api-machinery

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jan 17, 2019
@yujuhong
Copy link
Contributor

I think the summary API test only checks that the numbers are in the right range (with no intention to track usage regression), so bumping the number should be fine.
/cc @dashpole
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mkumatag, yujuhong

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 19, 2019
@dashpole
Copy link
Contributor

Agree with the above, this is mostly meant to check "Is this about what we expect the pod to use?". The lower bound is also much more important than the upper bound for checking, so it is fine to raise, especially if it is failing on valid setups.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 19, 2019
@k8s-ci-robot k8s-ci-robot merged commit fed3d75 into kubernetes:master Jan 19, 2019
@mkumatag
Copy link
Member Author

Thanks a lot, guys @dashpole @yujuhong @dims.

@mkumatag mkumatag deleted the ppc64le_node_test branch January 22, 2019 05:10
@mkumatag
Copy link
Member Author

assertion for pod spec is working fine but .Node.SystemContainers[pods].Memory: is failing because I'm running tests with parallel=8(default settings) for the node test. which is resulting in exceeding the memory limit check for the whole node.


    Timed out after 60.000s.
    Expected
        <string>: Summary
    to match fields: {
    .Node.SystemContainers[pods].Memory:
    	Expected
    	    <string>: MemoryStats
    	to match fields: {
    	.UsageBytes:
    		Expected
    		    <uint64>: 430702592
    		to be <=
    		    <int64>: 400000000
    	.WorkingSetBytes:
    		Expected
    		    <uint64>: 420806656
    		to be <=
    		    <int64>: 400000000
    	}

    }

code located at: https://github.com/kubernetes/kubernetes/blob/master/test/e2e_node/summary_test.go#L116:L118

IMO we should change the limit to memoryLimit := memoryCapacity.Value() what do you say? @dashpole @yujuhong

@dashpole
Copy link
Contributor

@mkumatag That sounds fine to me.

@mkumatag
Copy link
Member Author

@mkumatag That sounds fine to me.

I have submitted another PR to fix this - #73196

k8s-ci-robot added a commit that referenced this pull request Feb 14, 2019
…16-upstream-release-1.12

Automated cherry pick of #73016: Bump the pod memory to higher levels to work on power
k8s-ci-robot added a commit that referenced this pull request Feb 14, 2019
…16-upstream-release-1.11

Automated cherry pick of #73016: Bump the pod memory to higher levels to work on power
k8s-ci-robot added a commit that referenced this pull request Feb 18, 2019
…16-upstream-release-1.13

Automated cherry pick of #73016: Bump the pod memory to higher levels to work on power
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-none Denotes a PR that doesn't merit a release note. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants