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

Add an integration test that checks for the metrics we expect to be exported from the master #6941

Merged
merged 1 commit into from Apr 17, 2015

Conversation

a-robinson
Copy link
Contributor

Fixes #6887.

t.Fatalf("unexpected error: %v", err)
}

var m *master.Master
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you just moved this code, but there's no reason to do it in this order. Create the master first, then create the http server.

@roberthbailey roberthbailey self-assigned this Apr 17, 2015
// Make a request to the apiserver to ensure there's at least one data point
// for the metrics we're expecting -- otherwise, they won't be exported.
client := client.NewOrDie(&client.Config{Host: s.URL, Version: testapi.Version()})
_, err := client.Pods(api.NamespaceDefault).List(labels.Everything())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if _, err := ...; err != nil

@a-robinson
Copy link
Contributor Author

Fixed up all comments except the one I responded to about passing testing.T around.

@roberthbailey
Copy link
Contributor

LGTM. Will merge on green.

@roberthbailey roberthbailey added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 17, 2015
a-robinson added a commit that referenced this pull request Apr 17, 2015
Add an integration test that checks for the metrics we expect to be exported from the master
@a-robinson a-robinson merged commit 6213f61 into kubernetes:master Apr 17, 2015
@a-robinson a-robinson deleted the metrics branch April 17, 2015 21:40
@smarterclayton
Copy link
Contributor

This is broken on the Mac (cadvisor is disabled due to compile) - going to open a pull to skip this when cadvisor is not real.

@smarterclayton
Copy link
Contributor

Oops, I mean, because prometheus does not not support procfs on macs

@a-robinson
Copy link
Contributor Author

Sorry, I didn't consider that. SGTM, although I wonder how it made it this long if it's been failing on macs since it was merged.

@smarterclayton
Copy link
Contributor

Most people probably aren't running integration :)

@smarterclayton
Copy link
Contributor

#7723

@roberthbailey
Copy link
Contributor

Integration tests were already failing on macs (see #7282) so no one would have noticed this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apiserver's metrics page missing process-related metrics since 0.14
4 participants