Skip to content

Conversation

@benjaminapetersen
Copy link
Contributor

Ensure route.host has a value before tracking console_url metric to avoid the value "".

https://bugzilla.redhat.com/show_bug.cgi?id=1768684

@openshift-ci-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Nov 13, 2019
@openshift-ci-robot
Copy link
Contributor

@benjaminapetersen: This pull request references Bugzilla bug 1768684, which is invalid:

  • expected dependent Bugzilla bug 1764313 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1768684: Avoid console_url metric as empty string

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 13, 2019
@benjaminapetersen
Copy link
Contributor Author

/bugzilla refresh

@openshift-ci-robot
Copy link
Contributor

@benjaminapetersen: This pull request references Bugzilla bug 1768684, which is invalid:

  • expected dependent Bugzilla bug 1764313 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 13, 2019
@benjaminapetersen
Copy link
Contributor Author

Hmm, didn't expect it to reference Bugzilla bug 1764313. This is fine.

}
// if status.consoleURL is different than consoleURL,
// clear the old, then set new
if consoleConfig.Status.ConsoleURL != consoleURL {
Copy link
Member

Choose a reason for hiding this comment

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

will this ever be true? since this check is already done in SyncConsoleConfig() and is setting the consoleConfig.Status.ConsoleURL to consoleURL ?

@benjaminapetersen benjaminapetersen force-pushed the bug/1768684/metric/console_url/empty-string branch 2 times, most recently from b9f7628 to 2873ec8 Compare November 13, 2019 16:06
@spadgett
Copy link
Member

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Nov 13, 2019
@openshift-ci-robot
Copy link
Contributor

@spadgett: This pull request references Bugzilla bug 1768684, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@benjaminapetersen benjaminapetersen force-pushed the bug/1768684/metric/console_url/empty-string branch from 2873ec8 to 6a4bcb2 Compare November 13, 2019 16:55
@spadgett spadgett added this to the v4.3 milestone Nov 13, 2019
@benjaminapetersen benjaminapetersen force-pushed the bug/1768684/metric/console_url/empty-string branch 2 times, most recently from 8810461 to e2c2f17 Compare November 13, 2019 18:29
@spadgett
Copy link
Member

Looks like you have govet errors

@benjaminapetersen benjaminapetersen force-pushed the bug/1768684/metric/console_url/empty-string branch from e2c2f17 to ecb0748 Compare November 15, 2019 16:50
@benjaminapetersen
Copy link
Contributor Author

Removed the initialized bool & double checked gofmt. Should be good.

@benjaminapetersen
Copy link
Contributor Author

 --- FAIL: TestRemoved (46.23s) 

See #344 for a fix for this test, which adds 3x retries before reporting failure.

@openshift-ci-robot
Copy link
Contributor

@benjaminapetersen: This pull request references Bugzilla bug 1768684, which is valid.

Details

In response to this:

Bug 1768684: Avoid console_url metric as empty string

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@benjaminapetersen
Copy link
Contributor Author

This should now pass with the fix that went in with the downloads e2e.

@benjaminapetersen benjaminapetersen force-pushed the bug/1768684/metric/console_url/empty-string branch from ecb0748 to ce44340 Compare November 19, 2019 03:33
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 19, 2019
Copy link
Member

@jhadvig jhadvig left a comment

Choose a reason for hiding this comment

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

nit and a question :)

@benjaminapetersen benjaminapetersen force-pushed the bug/1768684/metric/console_url/empty-string branch from ce44340 to dfc6b1e Compare November 19, 2019 15:03
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 19, 2019
@spadgett
Copy link
Member

error: received unexpected HTTP status: 500 Internal Server Error

/retest

Copy link
Member

@spadgett spadgett 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 the lgtm Indicates that a PR is ready to be merged. label Nov 19, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benjaminapetersen, spadgett

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 [benjaminapetersen,spadgett]

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

@openshift-bot
Copy link
Contributor

/retest

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

4 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 4037330 into openshift:master Nov 19, 2019
@openshift-ci-robot
Copy link
Contributor

@benjaminapetersen: All pull requests linked via external trackers have merged. Bugzilla bug 1768684 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1768684: Avoid console_url metric as empty string

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@benjaminapetersen benjaminapetersen deleted the bug/1768684/metric/console_url/empty-string branch December 4, 2019 21:06
@benjaminapetersen
Copy link
Contributor Author

/cherry-pick release-4.2

@openshift-cherrypick-robot

@benjaminapetersen: #346 failed to apply on top of branch "release-4.2":

error: mode change for pkg/console/metrics/metrics.go, which is not in current HEAD
error: could not build fake ancestor
Patch failed at 0001 Ensure route.host has a value before tracking console_url metric to avoid ""

Details

In response to this:

/cherry-pick release-4.2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Dec 10, 2019

Manual it is :)

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. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants