Skip to content

Conversation

@Danil-Grigorev
Copy link

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

This change ensures compatibility with the ServiceMonitor resource
introduced in openshift/machine-api-operator#609

Close #75

Copy link

@dhellmann dhellmann left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 19, 2020
@dhellmann
Copy link

/test e2e-metal-ipi

@Danil-Grigorev
Copy link
Author

/retest

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

21 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2020
@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
Copy link
Author

@dhellmann I tried running unit tests, and got similar results to ipi e2e - machines are reconciled but not changed. Same thing happens on master, so don't think that's a problem of this PR WDYT?

@dhellmann
Copy link

dhellmann commented Jul 2, 2020

@dhellmann I tried running unit tests, and got similar results to ipi e2e - machines are reconciled but not changed. Same thing happens on master, so don't think that's a problem of this PR WDYT?

Which unit test failed? I see a failure locally for the Remediation test that @n1r1 added recently. I've opened #80 to see if I can reproduce that in CI.

Locally I see

--- FAIL: TestRemediation (0.00s)
    actuator_test.go:1785: expected a requeue err but err was nil
    actuator_test.go:1852:
        	Error Trace:	actuator_test.go:1852
        	Error:      	Not equal:
        	            	expected: map[string]string{"annName1":"annValue1", "annName2":"annValue2"}
        	            	actual  : map[string]string(nil)

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,5 +1,2 @@
make actuator tests more verbose for debugging
        	            	-(map[string]string) (len=2) {
        	            	- (string) (len=8) "annName1": (string) (len=9) "annValue1",
        	            	- (string) (len=8) "annName2": (string) (len=9) "annValue2"
        	            	-}
        	            	+(map[string]string) <nil>

        	Test:       	TestRemediation
        	Messages:   	Node annotations before remediation and after remediation ares not equal
    actuator_test.go:1855:
        	Error Trace:	actuator_test.go:1855
        	Error:      	Not equal:
        	            	expected: map[string]string{"labelName1":"labelValue1", "labelName2":"labelValue2"}
        	            	actual  : map[string]string(nil)

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,5 +1,2 @@
        	            	-(map[string]string) (len=2) {
        	            	- (string) (len=10) "labelName1": (string) (len=11) "labelValue1",
        	            	- (string) (len=10) "labelName2": (string) (len=11) "labelValue2"
        	            	-}
        	            	+(map[string]string) <nil>

        	Test:       	TestRemediation
        	Messages:   	Node labels before remediation and after remediation ares not equal
FAIL

@Danil-Grigorev
Copy link
Author

/retest

2 similar comments
@Danil-Grigorev
Copy link
Author

/retest

@Danil-Grigorev
Copy link
Author

/retest

@Danil-Grigorev Danil-Grigorev force-pushed the fix-metrics branch 2 times, most recently from 7db4c59 to d0d9b1a Compare July 15, 2020 07:27
This change ensure compatibility with serviceMonitor resource
introduced in openshift/machine-api-operator#609
@hardys
Copy link

hardys commented Jul 20, 2020

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Danil-Grigorev, dhellmann, hardys

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:

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

@dhellmann
Copy link

/test e2e-metal-ipi

@Danil-Grigorev
Copy link
Author

/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 21, 2020
@Danil-Grigorev
Copy link
Author

/retest

1 similar comment
@Danil-Grigorev
Copy link
Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit 71c61cc into openshift:master Jul 21, 2020
@Danil-Grigorev Danil-Grigorev deleted the fix-metrics branch July 21, 2020 09:58
honza pushed a commit to honza/cluster-api-provider-baremetal that referenced this pull request Feb 7, 2022
Makefile: Add kubeubilder and kustomize checks.
honza pushed a commit to honza/cluster-api-provider-baremetal that referenced this pull request Feb 7, 2022
✨ Extract IPAM and reference its repo
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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Metrics port change to support updated MAO metrics integration

6 participants