Create default Mixer config YAMLs by douglas-reid · Pull Request #866 · istio/istio

douglas-reid · 2017-09-19T15:49:20Z

This PR moves the "default" configs for mixer from the samples/app/bookinfo/rules dir to the main install/kubernetes/** tree.

This PR is needed because, without it, the install of mixer will be missing many important configs for basic operation, including:

attribute manifests
default metrics
configuration of prometheus and stdio adapters

To make use of the new location for the Mixer configs, the e2e framework code was updated to replace the istio-config-default namespace with the namespace of the test. The various, now unnecessary YAML files in the samples directory were deleted.

cc: @debianmaster, @karmab

Related issue: istio/old_issues_repo#59

Release note:

Provide default Mixer configuration for prometheus metrics and stdio logs.

rshriram

Question/nit (non blocking): Can we bundle logs, metrics, prometheus as one YAML ? Does it make sense to deploy them individually, such that one can be used without the other? If not, I suggest using a single mixer-metrics-addon.yaml (Not even sure if this should be an addon or just be enabled by default).
Even if prometheus is unavailable, mixer keeps collecting metrics right? So, we can have it on by default.

douglas-reid · 2017-09-19T20:01:17Z

@rshriram I don't want to bundle logs with any of the metrics stuff. It arguably could be two files itself (one for the adapter config, one for the logentry). But it is distinct, in the Mixer sense, from the metrics stuff.

The metrics stuff can be used separately from prometheus adapter (if you wanted to use stdio metrics handler or stackdriver, for instance). But maybe that is a detail we worry about later.

@mandarjog thoughts on the addons config organization? should we bundle metrics.yaml and prometheus.yaml ? which is easier for users (to understand AND to use)?

mandarjog · 2017-09-19T21:28:57Z

We should bundle prometheus with related metrics.
Prometheus adapter is built-in so it is safe to do so.

Regarding using fully qualified instance and handler names inside actions, namespace can be elided from those names. We do this is configs checked into Mixer repo.

Let us get some feedback from users if short names are desirable over fully qualified names while operating within a namespace.
istio/old_mixer_repo#1323

douglas-reid · 2017-09-20T04:10:59Z

PTAL. I moved the "default" Mixer YAMLs for metrics and logs into the template for mixer itself. This simplifies the number of files, etc. I also stripped some of the explicit namespacing.

mandarjog · 2017-09-20T09:04:14Z

I don't fully like the idea of installing config resources in a namespace of our choice - istio-config-default. It does create a good out of the box experience, but I don't see a way for a user to specify a different namespace, unless I have missed something.

That said, this does get us back to parity with 0.1.

mandarjog

lgtm

mandarjog · 2017-09-20T09:04:51Z

/lgtm

istio-merge-robot · 2017-09-20T09:05:52Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mandarjog
We suggest the following additional approver: andraxylia

Assign the PR to them by writing /assign @andraxylia in a comment when ready.

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

Details

Needs approval from an approver in each of these OWNERS Files:

OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

douglas-reid · 2017-09-20T15:05:43Z

@mandarjog Filed #878

douglas-reid · 2017-09-20T15:09:03Z

Merging, in advance of internal testing to allow for proper out-of-the-box Mixer experience.

Automatic merge from submit-queue. Add release-note collector **Release note**: ```release-note none ``` It's a binary to collect release-note from repos. Example command: ``` Bash $ bazel-bin/toolbox/release_note_collector/release_note_collector --repos istio --previous_release 0.2.2 --current_release 0.2.4 --pr_link ``` And the release-note would be put into a local file, default is in current directory, named "<repo>.releasenote" The example output is: ```Bash $ cat istio.releasenote istio: istio -- 0.2.4 release note Previous release version: 0.2.2 * Provide default Mixer configuration for prometheus metrics and stdio logs. istio/istio#866 * test setup doc update istio/istio#831 * New config model (breaking change). Migration instructions are [here](https://github.com/istio/istio/blob/release-0.2/samples/CONFIG-MIGRATION.md) istio/istio#829 ```

Add the Handler configuration logic. Handler configurer identifies and configures all the handlers that needs to be configure and infer the types to pass them. Former-commit-id: d5caf7b7fd750f103390ca71ea999dd6e30bc50a

* Create default Mixer config YAMLs * Update bookinfo BUILD * Refactor configs to mixer yaml template * delete metrics.yaml Former-commit-id: 70a8218

Add the Handler configuration logic. Handler configurer identifies and configures all the handlers that needs to be configure and infer the types to pass them. Former-commit-id: 52092f162abd315ce406139749097ebfe72b1643

* Create default Mixer config YAMLs * Update bookinfo BUILD * Refactor configs to mixer yaml template * delete metrics.yaml Former-commit-id: 70a8218

Automatic merge from submit-queue. Add release-note collector **Release note**: ```release-note none ``` It's a binary to collect release-note from repos. Example command: ``` Bash $ bazel-bin/toolbox/release_note_collector/release_note_collector --repos istio --previous_release 0.2.2 --current_release 0.2.4 --pr_link ``` And the release-note would be put into a local file, default is in current directory, named "<repo>.releasenote" The example output is: ```Bash $ cat istio.releasenote istio: istio -- 0.2.4 release note Previous release version: 0.2.2 * Provide default Mixer configuration for prometheus metrics and stdio logs. istio/istio#866 * test setup doc update istio/istio#831 * New config model (breaking change). Migration instructions are [here](https://github.com/istio/istio/blob/release-0.2/samples/CONFIG-MIGRATION.md) istio/istio#829 ```

googlebot added the cla: yes label Sep 19, 2017

istio-merge-robot added the release-note label Sep 19, 2017

douglas-reid requested review from ldemailly and mandarjog September 19, 2017 15:49

douglas-reid mentioned this pull request Sep 19, 2017

grafana nor servicegraph display data since 0.2.2 istio/old_issues_repo#59

Closed

rshriram approved these changes Sep 19, 2017

View reviewed changes

douglas-reid added 3 commits September 19, 2017 21:06

Create default Mixer config YAMLs

53c2cf5

Update bookinfo BUILD

91c9bb8

Refactor configs to mixer yaml template

b7b71ba

delete metrics.yaml

19252f6

mandarjog approved these changes Sep 20, 2017

View reviewed changes

istio-testing assigned mandarjog Sep 20, 2017

istio-testing added the lgtm label Sep 20, 2017

douglas-reid mentioned this pull request Sep 20, 2017

Provide way for users to change Mixer config default namespace #878

Closed

douglas-reid merged commit 70a8218 into istio:master Sep 20, 2017

rshriram pushed a commit that referenced this pull request Oct 30, 2017

Create default Mixer config YAMLs (#866)

79181e2

* Create default Mixer config YAMLs * Update bookinfo BUILD * Refactor configs to mixer yaml template * delete metrics.yaml Former-commit-id: 70a8218

vbatts pushed a commit to vbatts/istio that referenced this pull request Oct 31, 2017

Create default Mixer config YAMLs (istio#866)

7314efa

* Create default Mixer config YAMLs * Update bookinfo BUILD * Refactor configs to mixer yaml template * delete metrics.yaml Former-commit-id: 70a8218

mandarjog pushed a commit that referenced this pull request Nov 2, 2017

Create default Mixer config YAMLs (#866)

ddcc94c

* Create default Mixer config YAMLs * Update bookinfo BUILD * Refactor configs to mixer yaml template * delete metrics.yaml Former-commit-id: 70a8218

0x01001011 pushed a commit to thedemodrive/istio that referenced this pull request Jul 16, 2020

Delete obsolete leftover file. (istio#866)

693bbe9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create default Mixer config YAMLs#866

Create default Mixer config YAMLs#866
douglas-reid merged 4 commits intoistio:masterfrom
douglas-reid:update-mixer

douglas-reid commented Sep 19, 2017 •

edited

Loading

Uh oh!

rshriram left a comment

Uh oh!

douglas-reid commented Sep 19, 2017

Uh oh!

mandarjog commented Sep 19, 2017

Uh oh!

douglas-reid commented Sep 20, 2017

Uh oh!

mandarjog commented Sep 20, 2017

Uh oh!

mandarjog left a comment

Uh oh!

mandarjog commented Sep 20, 2017

Uh oh!

istio-merge-robot commented Sep 20, 2017

Uh oh!

douglas-reid commented Sep 20, 2017

Uh oh!

douglas-reid commented Sep 20, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

douglas-reid commented Sep 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rshriram left a comment

Choose a reason for hiding this comment

Uh oh!

douglas-reid commented Sep 19, 2017

Uh oh!

mandarjog commented Sep 19, 2017

Uh oh!

douglas-reid commented Sep 20, 2017

Uh oh!

mandarjog commented Sep 20, 2017

Uh oh!

mandarjog left a comment

Choose a reason for hiding this comment

Uh oh!

mandarjog commented Sep 20, 2017

Uh oh!

istio-merge-robot commented Sep 20, 2017

Uh oh!

douglas-reid commented Sep 20, 2017

Uh oh!

douglas-reid commented Sep 20, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

douglas-reid commented Sep 19, 2017 •

edited

Loading