Skip to content

use prometheus statsd_exporter v0.6.0#1922

Merged
knative-prow-robot merged 2 commits intoknative:release-0.1from
simingweng:fix-statsd-exporter-docker-image-version
Oct 12, 2018
Merged

use prometheus statsd_exporter v0.6.0#1922
knative-prow-robot merged 2 commits intoknative:release-0.1from
simingweng:fix-statsd-exporter-docker-image-version

Conversation

@simingweng
Copy link
Copy Markdown

Fixes #1921

Proposed Changes

  • set the statsd_exporter Docker image to v0.6.0, so it still supports single dash command line argument

@knative-prow-robot knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 22, 2018
@knative-prow-robot knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 22, 2018
Copy link
Copy Markdown
Contributor

@sdake sdake left a comment

Choose a reason for hiding this comment

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

/lgtm

See #1921 (comment) for my analysis. Also, I tested this change against the release-0.1 branch. I was wondering why the documentation was causing statsd to break. A problem in Istio 0.8. That said, I'm not sure when/if Istio will release a 0.8 z stream.

cc @hklai

@knative-prow-robot
Copy link
Copy Markdown
Contributor

@sdake: changing LGTM is restricted to assignees, and only knative/serving repo collaborators may be assigned issues.

Details

In response to this:

/lgtm

See #1921 (comment) for my analysis. Also, I tested this change against the release-0.1 branch. I was wondering why the documentation was causing statsd to break. A problem in Istio 0.8. That said, I'm not sure when/if Istio will release a 0.8 z stream.

cc @hklai

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.

@sdake
Copy link
Copy Markdown
Contributor

sdake commented Aug 26, 2018

/assign @mdemirhan

@sdake
Copy link
Copy Markdown
Contributor

sdake commented Aug 26, 2018

See istio/istio#8249 for upstream Istio resolution.

@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 2, 2018

/lgtm
/approve
/ok-to-test

@knative-prow-robot knative-prow-robot added lgtm Indicates that a PR is ready to be merged. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 2, 2018
@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 12, 2018

/test pull-knative-serving-integration-tests

1 similar comment
@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 26, 2018

/test pull-knative-serving-integration-tests

@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 26, 2018

/assign @mattmoor
/assign @mdemirhan
for approval

@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 26, 2018

/test pull-knative-serving-integration-tests

@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 26, 2018

This should fix #1921

@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 26, 2018

/retest

@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 26, 2018

Seems integration tests no longer passing on our release branch. I'll try running on my cluster.

@mdemirhan
Copy link
Copy Markdown
Contributor

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 27, 2018
@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 27, 2018

/test pull-knative-serving-integration-tests

2 similar comments
@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Sep 28, 2018

/test pull-knative-serving-integration-tests

@mdemirhan
Copy link
Copy Markdown
Contributor

/test pull-knative-serving-integration-tests

@mdemirhan
Copy link
Copy Markdown
Contributor

@adrcunha Can you please take a look and let us know how we can get past the integration tests?

@mdemirhan
Copy link
Copy Markdown
Contributor

/test pull-knative-serving-integration-tests

@mdemirhan
Copy link
Copy Markdown
Contributor

@adrcunha @jessiezcc This PR is blocked due to infra issues. This is something we should checkin as it is breaking some users. Can this please be prioritized?

@jessiezcc
Copy link
Copy Markdown
Contributor

@srinivashegde86 to help take a look. Test seems to fail with "main.go:309: Something went wrong: failed to prepare test environment: stat /root/.ssh/google_compute_engine: no such file or directory"

@srinivashegde86
Copy link
Copy Markdown
Contributor

this was the ssh issue, which was fixed 2 days ago by http://go/gh/knative/serving/pull/2125

Maybe its not picking the latest test-infra scripts

@srinivashegde86
Copy link
Copy Markdown
Contributor

/test pull-knative-serving-integration-tests

@jessiezcc
Copy link
Copy Markdown
Contributor

the test seems only fail for this PR, not others

@srinivashegde86
Copy link
Copy Markdown
Contributor

can we rebase and try again? For some reason, we seem to be using the older version of the test-infra scripts under vendor/ for this PR.

The other PR's have passing runs.

@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Oct 5, 2018

This PR is on the release-0.1 branch, not the master branch.

@srinivashegde86
Copy link
Copy Markdown
Contributor

we removed the ssh requirement recently, but it has not been ported back to the release branch and only to master. That looks like a breaking change atleast for running prow.

@tcnghia
Copy link
Copy Markdown
Contributor

tcnghia commented Oct 5, 2018

May be we should run test manually and merge manually for release branch?

@bbrowning
Copy link
Copy Markdown
Contributor

I'm working on unblocking the release branch in issue #2165 and PR #2166.

@simplytunde
Copy link
Copy Markdown

I will like this to get good tracking for the fix to be pushed out for GKE setup

@mdemirhan
Copy link
Copy Markdown
Contributor

/test pull-knative-serving-integration-tests

@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 11, 2018
@mdemirhan
Copy link
Copy Markdown
Contributor

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2018
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mdemirhan, simingweng, tcnghia

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

@knative-prow-robot knative-prow-robot merged commit 88a115b into knative:release-0.1 Oct 12, 2018
@jonjohnsonjr jonjohnsonjr mentioned this pull request Oct 17, 2018
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. 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.

10 participants