Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

add persist storage to prometheus #546

Closed
wants to merge 1 commit into from

Conversation

yacut
Copy link

@yacut yacut commented Nov 12, 2019

related to istio/istio#18740

@yacut yacut requested a review from a team as a code owner November 12, 2019 08:46
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Nov 12, 2019
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 12, 2019
@istio-testing
Copy link

Hi @yacut. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Copy link
Member

@morvencao morvencao left a comment

Choose a reason for hiding this comment

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

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Nov 13, 2019
@istio-testing
Copy link

@yacut: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
gencheck_installer 79fbb8d link /test gencheck_installer

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. I understand the commands that are listed here.

@sdake
Copy link
Member

sdake commented Nov 25, 2019

@yacut you will need to run make gen with BUILD_WITH_CONTAINERS=1. It may be necessary to unset your HUB and TAG environment variables if they are set (although I believe @howardjohn fixed that problem).

make gen will trigger the generation of binary blobs from the source manifests in the repository.

Cheers
-steve

@sdake
Copy link
Member

sdake commented Nov 25, 2019

@howardjohn I am of the opinion that we should punt the third party component install to the upstreams and instead focus on integration with the third-party components. For this specific issue, you had pointed at a path to integration, and @douglas-reid had indicated that path was not going to happen in 1.5 😁

As such, I feel we should just freeze any changes to third party components (unless they involve the resolution of a security vulnerability or general image update) until such time as we can produce an RFC and obtain a sign off or rejection of the RFC from the telemetry WG.

I'll produce the RFC this afternoon.

Cheers
-steve

@johscheuer
Copy link
Member

johscheuer commented Nov 29, 2019

I agree with @sdake also most "production" installations will already have their Prometheus/Grafana installation somehow managed and just putting Istio on top of it (at least in my experience) and the Installer should refer to the official installation docs of the specific tools and just document how to integrate them (or provide some basic dashboards/alerts/..).

One thing that would be interesting would be some kind of a compatibility matrix (I know this can be a lot of work) similar like Istio already states "tested with Kubernetes version X Y Z". The Istio docs could state tested with Tools X Y Z in the following combinations ?

@istio-testing
Copy link

@yacut: PR needs rebase.

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.

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 12, 2020
@adamrbennett
Copy link

@sdake @johscheuer Has an official position been taken with regard to third party components? It sounds like the sentiment here is that out-of-the-box third-party component integration is generally not supported for non-trival use-cases.

@sdake
Copy link
Member

sdake commented Jun 1, 2020

@adamrbennett yes a decision has been made for third party components. The decision, as I understand it, is that Istio will provide documentation on istio.io that describes how to integrate with third party components that make sense for Istio to integrate with, assuming there is someone to maintain said integration.

For more context, please review this heavily debated and agreed upon plan and design:

https://docs.google.com/document/d/1jJ4kkALCUPWIlz_Ldu6jpeelMNxyY_SZDMaZmeSpfkM/edit#heading=h.qlmnvhzb2ib2

@sdake
Copy link
Member

sdake commented Jun 1, 2020

This PR has been subsumed by https://docs.google.com/document/d/1jJ4kkALCUPWIlz_Ldu6jpeelMNxyY_SZDMaZmeSpfkM/edit#heading=h.qlmnvhzb2ib2. If you disagree, please reopen the issue.

Cheers,
-steve

@sdake sdake closed this Jun 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. needs-rebase Indicates a PR needs to be rebased before being merged ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. 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