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

Fix TestCadvisorListPodStats failure under mac/darwin #57637

Merged

Conversation

dims
Copy link
Member

@dims dims commented Dec 27, 2017

What this PR does / why we need it:
GetPodCgroupNameSuffix is not really implemented under darwin
(or windows for that matter). So let's just skip over the check
for CPU and Memory if that is not set.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #57636

Special notes for your reviewer:

Release note:

NONE

GetPodCgroupNameSuffix is not really implemented under darwin
(or windows for that matter). So let's just skip over the check
for CPU and Memory if that is not set.
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 27, 2017
@dixudx
Copy link
Member

dixudx commented Dec 27, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 27, 2017
@geojaz
Copy link
Member

geojaz commented Dec 27, 2017

/lgtm
@dims thanks!

@geojaz
Copy link
Member

geojaz commented Dec 29, 2017

@dchen1107 @vishh Could we get a quick look at this one please? :)

@dims
Copy link
Member Author

dims commented Dec 31, 2017

/test pull-kubernetes-unit

@dims
Copy link
Member Author

dims commented Jan 2, 2018

/test pull-kubernetes-node-e2e

@dims
Copy link
Member Author

dims commented Jan 3, 2018

cc @yujuhong @Random-Liu

@dims
Copy link
Member Author

dims commented Jan 5, 2018

@k8s-github-robot
Copy link

/test all

Tests are more than 96 hours old. Re-running tests.

1 similar comment
@k8s-github-robot
Copy link

/test all

Tests are more than 96 hours old. Re-running tests.

@dims
Copy link
Member Author

dims commented Jan 11, 2018

/retest

@k8s-github-robot
Copy link

/test all

Tests are more than 96 hours old. Re-running tests.

@derekwaynecarr
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, dims, dixudx, geojaz

Associated issue: #57636

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your 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 11, 2018
@k8s-github-robot
Copy link

/test all

Tests are more than 96 hours old. Re-running tests.

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-github-robot pushed a commit that referenced this pull request Apr 11, 2018
Automatic merge from submit-queue (batch tested with PRs 62192, 61866, 62206, 62360). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Make the test TestCRIListPodStats pass for Darwin and Windows

GetPodCgroupNameSuffix is only implemented for Linux, which mean
that CPU and Memory stats are only available on Linux.

My fix to make the test pass on other OS:es than Linux
is to just check CPU and Memory stats on Linux.

(This is similar to #57637 which fixed the same problem for the
test TestCadvisorListPodStats.)



**What this PR does / why we need it**:
To make all unit tests pass on macOS/Darwin

**Which issue(s) this PR fixes**:
Fixes #62177

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestCadvisorListPodStats is segfaulting on master with MacOs
8 participants