Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-1.4] update telemetryv2 templates #591

Merged
merged 5 commits into from Dec 6, 2019

Conversation

@richardwxn
Copy link
Contributor

richardwxn commented Dec 5, 2019

This change is for 1.4 specifically, including the filter part, proxymatch verison part

@richardwxn richardwxn requested a review from istio/release-managers-1-4 as a code owner Dec 5, 2019
@googlebot googlebot added the cla: yes label Dec 5, 2019
@istio-testing istio-testing added the size/L label Dec 5, 2019
@@ -1,5 +1,234 @@
{{- if and .Values.telemetry.enabled .Values.telemetry.v2.enabled }}
{{ .Files.Get "metadata-exchange-v2.yaml" }}
apiVersion: networking.istio.io/v1alpha3

This comment has been minimized.

Copy link
@mandarjog

mandarjog Dec 5, 2019

Contributor

This should be enabled if either stackdriver or prom is enabled.

This comment has been minimized.

Copy link
@richardwxn

richardwxn Dec 5, 2019

Author Contributor

then there is no meaning of the v2.enabled setting? also if we have some more other filters in the future then this check would become lengthy

This comment has been minimized.

Copy link
@mandarjog

mandarjog Dec 5, 2019

Contributor

Ok, that makes sense, this is only enabled when v2 is enabled .

@richardwxn

This comment has been minimized.

Copy link
Contributor Author

richardwxn commented Dec 5, 2019

/test noauth_installer_release-1.4

@richardwxn

This comment has been minimized.

Copy link
Contributor Author

richardwxn commented Dec 5, 2019

/retest

1 similar comment
@richardwxn

This comment has been minimized.

Copy link
Contributor Author

richardwxn commented Dec 5, 2019

/retest

@richardwxn richardwxn force-pushed the richardwxn:1.4-tv2 branch from ef30d1d to eb8bb39 Dec 5, 2019
@richardwxn richardwxn requested review from mandarjog and howardjohn Dec 6, 2019
Copy link
Contributor

mandarjog left a comment

/lgtm

@richardwxn richardwxn requested a review from howardjohn Dec 6, 2019
@howardjohn

This comment has been minimized.

Copy link
Member

howardjohn commented Dec 6, 2019

We need to have 1.4 have this, and master have 1.5+1.4. Otherwise when we upgrade its just going to prune this one, right?

@richardwxn

This comment has been minimized.

Copy link
Contributor Author

richardwxn commented Dec 6, 2019

@howardjohn yes I updated the master PR as well: #590. Plz check

@richardwxn

This comment has been minimized.

Copy link
Contributor Author

richardwxn commented Dec 6, 2019

@howardjohn plz check again?

@howardjohn

This comment has been minimized.

Copy link
Member

howardjohn commented Dec 6, 2019

@richardwxn can we merge master first so we ensure they are in sync? And have telemetry experts approve that one and I will merge

@richardwxn richardwxn changed the title update telemetryv2 templates [release-1.4] update telemetryv2 templates Dec 6, 2019
@istio-testing istio-testing merged commit 44d47f4 into istio:release-1.4 Dec 6, 2019
6 of 7 checks passed
6 of 7 checks passed
tide Not mergeable.
Details
base_installer_release-1.4 Job succeeded.
Details
build_installer_release-1.4 Job succeeded.
Details
cla/google All necessary CLAs are signed
demo_installer_release-1.4 Job succeeded.
Details
lintmodern_installer_release-1.4 Job succeeded.
Details
noauth_installer_release-1.4 Job succeeded.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.