Skip to content

Add metrics #106

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 2 commits into from
Jul 14, 2020
Merged

Add metrics #106

openshift-merge-robot merged 2 commits into from
Jul 14, 2020

Conversation

@Danil-Grigorev
Copy link

@Danil-Grigorev Danil-Grigorev commented Jun 19, 2020

What this PR does / why we need it:

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 #100

Special notes for your reviewer:

This PR only ensures that build process won't break with latest MAO version, but not implementing collection of API call failures during machine lifetime. Leaving this on maintainers to consider.

Release note:

NONE

@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 19, 2020

/retest

@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 19, 2020 •
edited
Loading

Can you please try running go mod vendor?

@Danil-Grigorev Danil-Grigorev force-pushed the add-metrics branch 2 times, most recently from ebb86ea to 707afda Compare June 19, 2020 10:47
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 19, 2020
@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 19, 2020

/retest

5 similar comments
@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 19, 2020

/retest

@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 20, 2020

/retest

@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 21, 2020

/retest

@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 22, 2020

/retest

@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 22, 2020

/retest

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 23, 2020
@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 23, 2020

Please squash that horror that I've made; we have to wait for the CI to get back in shape anyway 🙏

@mandre
Copy link
Member

mandre commented Jun 24, 2020

/approve
/test e2e-openstack

Thanks for the PR @Danil-Grigorev ! We'll merge as soon as our CI shows some green - it's currently having a hard time due to other issues.

@pierreprinetti
Copy link
Member

pierreprinetti commented Jun 24, 2020

/retest

1 similar comment
@mandre
Copy link
Member

mandre commented Jun 25, 2020

/retest

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2020
@Danil-Grigorev Danil-Grigorev mentioned this pull request Jul 2, 2020
Merged
@Danil-Grigorev Danil-Grigorev force-pushed the add-metrics branch 2 times, most recently from f860f48 to 737a57a Compare July 2, 2020 09:36
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2020
@Danil-Grigorev Danil-Grigorev force-pushed the add-metrics branch 2 times, most recently from ed0d340 to 890b0e3 Compare July 2, 2020 10:09
@Danil-Grigorev
Copy link
Author

Danil-Grigorev commented Jul 2, 2020

@pierreprinetti May I get another LGTM? Updated vendor as it was skewed.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2020
@mandre
Copy link
Member

mandre commented Jul 4, 2020

/retest

1 similar comment
@pierreprinetti
Copy link
Member

pierreprinetti commented Jul 6, 2020

/retest

@mandre
Copy link
Member

mandre commented Jul 9, 2020

/test e2e-openstack

1 similar comment
@mandre
Copy link
Member

mandre commented Jul 9, 2020

/test e2e-openstack

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 10, 2020
@pierreprinetti
Copy link
Member

pierreprinetti commented Jul 10, 2020

@Danil-Grigorev I have created a commit for resolving conflicts. Can you please check and squash? CI is now expected to work, we should be able to merge right away 🤞

@pierreprinetti
Copy link
Member

pierreprinetti commented Jul 10, 2020

ugh cmd/manager/main.go needs a go fmt

@Danil-Grigorev
Copy link
Author

Danil-Grigorev commented Jul 10, 2020

@pierreprinetti Should be ok now. I'm not sure why, but the CI is still failing.

@pierreprinetti
Copy link
Member

pierreprinetti commented Jul 10, 2020

Let's see if it's a flake
/retest

@pierreprinetti
Copy link
Member

pierreprinetti commented Jul 10, 2020

/retest

@Danil-Grigorev
Copy link
Author

Danil-Grigorev commented Jul 13, 2020

/retest

1 similar comment
@Danil-Grigorev
Copy link
Author

Danil-Grigorev commented Jul 13, 2020

/retest

Set metrics on :8081 addr
a1ee52c 
- Add configv1 to scheme to automatically get clusterId from infrastructure resource introduced in PR: openshift/machine-api-operator#608
@Danil-Grigorev
Copy link
Author

Danil-Grigorev commented Jul 14, 2020

CI was failing, as the PR openshift/machine-api-operator#608 was missing log error message (fix for that - openshift/machine-api-operator#639), which should have notified about errors getting infrastructure resource. I included the missing scheme injection in a1ee52c @pierreprinetti Should be green now.

@Danil-Grigorev Danil-Grigorev mentioned this pull request Jul 14, 2020
Closed
@mandre
Copy link
Member

mandre commented Jul 14, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 14, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Danil-Grigorev, mandre, pierreprinetti

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [mandre,pierreprinetti]

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

@mandre
Copy link
Member

mandre commented Jul 14, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 14, 2020
@openshift-merge-robot openshift-merge-robot merged commit fe69c20 into openshift:master Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iamemilio iamemilio Awaiting requested review from iamemilio

@mandre mandre Awaiting requested review from mandre

1 more reviewer

@pierreprinetti pierreprinetti pierreprinetti approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mandre mandre

@pierreprinetti pierreprinetti

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable metrics to support updated MAO metrics integration

5 participants

@Danil-Grigorev @pierreprinetti @mandre @openshift-ci-robot @openshift-merge-robot