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

Merging release-0.8 to master #5590

Merged
merged 99 commits into from
May 15, 2018
Merged

Merging release-0.8 to master #5590

merged 99 commits into from
May 15, 2018

Conversation

hklai
Copy link
Contributor

@hklai hklai commented May 14, 2018

We are close to cutting a RC in release-0.8 for testing, so we are merging the commits in release-0.8 back to the master, to prepare for unlocking master.

andraxylia and others added 30 commits April 27, 2018 13:54
* Adde list of container ports to the injected inbound ports

* Add support for helm

* [test pr] check if 503s and other known bugs are fixed

removing the t.Skip()

Should fail in CI until we have a fix

* prune old version resources that no longer exist (#5107)

Automatic merge from submit-queue.

prune old version resources that no longer exist

* [vendor-change] CloudWatch Mixer adapter (#4617)

Automatic merge from submit-queue.

[vendor-change] CloudWatch Mixer adapter

Adding an adapter to send metrics to cloudwatch

* Enable Ingress/Egress gateways in Helm for bookinfo demos (#5120)

Automatic merge from submit-queue.

Enable Ingress/Egress gateways in Helm for bookinfo demos

* Consume labeled multicluster secrets on startup (#5117)

Automatic merge from submit-queue.

Consume labeled multicluster secrets on startup

This patch when run against istio.yaml or istio-auth.yaml
runs in the new config mode using only labels rather than
configmaps.  The configmap functionality can be removed in
0.9.

* Add a linter check to make sure types.go are generated. (#5110)

Automatic merge from submit-queue.

Add a linter check to make sure types.go are generated.

addresses #4418

* Remove outdated manifests from install/kubernetes (#4882)

* Remove orig_ manifests

* Remove istio-mixer-validator and istio-mixer-with-health-check manifests

* Remove unwanted manifests before archiving

* Remove istio-sidecar-injector.yaml from install/README.md

* Remove *one-namespace*.yaml from install/README.md

* Make helm-generated manifests overwrite updateVersion_orig.sh manifests

* Add support for per-metric namespace configuration to prom config (#5112)

* Adding CI workflow for checking vendor diff (#5051)

Automatic merge from submit-queue.

Adding CI workflow for checking vendor diff

This aims to help ensure that a PR contains the correct vendor change,
by running `dep ensure` and seeing if git detects any changes.

* Introduce galley/pkg/server (#4974)

Automatic merge from submit-queue.

Introduce galley/pkg/server

galley/pkg/server implements logic performs both CRD synchronization, along with resource synchronization operations. The resource synchronizers are started/stopped as CRDs (of interest) are added/deleted.

* [vendor change] Add metrics command to istioctl experimental cli (#4945)

Automatic merge from submit-queue.

[vendor change] Add metrics command to istioctl experimental cli

This PR adds a new command for retrieving service-level metrics
for services within an Istio service mesh. In combination with
the `watch` command, this tool may be used to display a rudimentary
service dashboard from the commandline.

This command requires the deployment of a prometheus instance for
monitoring the mesh. It discovers a prometheus pod, establishes a
port-forward to that pod, and executes a series of queries to extract
the metrics for display.

Currently, this command pulls all metrics from the current time, 
calculating rates and latencies over a time window of 1 minute. In 
the future, it will be possible to add support for flexible time
windows.

Example usage (bookinfo example):

```
$ istioctl experimental metrics productpage reviews ratings details
productpage:
  Total RPS:     7.872870
  Error RPS:     0.000000
  P50 Latency:   40ms
  P90 Latency:   80ms
  P99 Latency:   98ms
reviews:
  Total RPS:     7.909235
  Error RPS:     0.000000
  P50 Latency:   4ms
  P90 Latency:   9ms
  P99 Latency:   21ms
ratings:
  Total RPS:     5.309187
  Error RPS:     0.000000
  P50 Latency:   2ms
  P90 Latency:   4ms
  P99 Latency:   4ms
details:
  Total RPS:     7.872870
  Error RPS:     0.000000
  P50 Latency:   3ms
  P90 Latency:   38ms
  P99 Latency:   48ms
``` 

This tool is intended primarily to aid with debugging, as discovering
what is happening with a mesh and/or a particular service can be somewhat
cumbersome.

Reviewers: please let me know if there is a more appropriate place for 
such a tool and if there is more/different information that you think
is relevant to display for a service.

Vendor PR: istio/old_vendor-istio_repo#58

* unset IFS, minor fix for perf setup (#5124)

Automatic merge from submit-queue.

unset IFS, minor fix for perf setup

* perf setup update: add grafana, misc fixes (#5028)

* need git pull --tags to get latest_release movement, use DUR variable for duration

* Add grafana ingress

Doesn’t work because of mixer/telemetry split yet but almost

Also had to disable mtls for grafana - this should be the default

* Add annotation for no mtls in helm template

* From 0.8 prometheus is already in the yaml

See #5111

* Assert requried circle CI envs in ci2gubernator (#5137)

Automatic merge from submit-queue.

Assert requried circle CI envs in ci2gubernator

There has been cases where tests on circle failed when calling ci2gubernator because `CIRCLE_PR_NUMBER` unbound. This PR asserts the existence of the circle ci envs required by ci2gubernator and resort to no op if any of those is not defined.

* Add Mixer perf tests that includes the RPC path. (#5013)

Automatic merge from submit-queue.

Add Mixer perf tests that includes the RPC path.

The perf tests included two sets of tests (proper v.s. with _R2 suffix).
The tests with _R2 suffix was for testing runtime2 implementation.

Now that there is only one runtime, repurposing some of the tests to
include the gRpc layer as well.

* verify 200 status code in addition to header value (#5163)

* Add/Update Mixer e2e tests to cover more attributes sent from Envoy. (#5152)

* Add/Update Mixer e2e tests to cover more attributes sent from Envoy.

* Fix indent.

* Assorted bug fixes for 0.8 (#5133)

* assorted bug fixes

Signed-off-by: Shriram Rajagopalan <[email protected]>

* lint

Signed-off-by: Shriram Rajagopalan <[email protected]>

* Updated zipkin to 2.7 for istio. (#5155)

Automatic merge from submit-queue.

Updated zipkin to 2.7 for istio.

This is a follow up PR for #4726

/cc @ldemailly

* fix path for go 1.10 on perf vm (#5168)

* Move mixer filter to per_filter_config (#5073)

Automatic merge from submit-queue.

Move mixer filter to per_filter_config

Move the per route mixer filter config from the metadata field to per_filter_config and turn it into a ServiceConfig proto.

* Enable test

* [vendor change] Add B3 codec to Jaeger tracer to enable mixer trace to be included in… (#5116)

Automatic merge from submit-queue.

[vendor change] Add B3 codec to Jaeger tracer to enable mixer trace to be included in…

… the application trace - and extended zipkin test to check for the mixer span

Installs the B3 codec into the Jaeger tracer to enable B3 headers to be understood and therefore associate any spans with the existing application trace.

The PR also updates the zipkin e2e test to check that the mixer spans are included in the application trace instance. 

Once an initial review of the PR has been approved I'll commit the vendor change - using "dep ensure"? Locally this has resulted in a number of dependencies being deleted under `vendor/k8s.io/client-go/`.

Signed-off-by: Gary Brown <[email protected]>

* remove prometheus from release archives (#5150)

Automatic merge from submit-queue.

remove prometheus from release archives

* Add Galley command-line flags "server" and "purge" (#4977)

Automatic merge from submit-queue.

Add Galley command-line flags "server" and "purge"

Add command-line flags for server and purge commands.

* Simplify the auth test

Thanks Andra for pointing out that version should fail/work the same as
using pod IP directly as the destination container never sees the
original cluster IP

* adds guard for kube client (#5140)

* adds guard for kube client

- there may not always be one, especially in
the case of CF.
- made CF case more explicit

* ci2gubernator: stop checking for unset variables

* Fix single endpoint pilot ads look up (#5165)

* Add an experiment subcommand rbac to istioctl. (#5093)

Automatic merge from submit-queue.

Add an experiment subcommand rbac to istioctl.

The subcommand is used to interact with Istio RBAC policies, this PR
adds the basic interface and the actual logic will be added in a later
PR.

See #4856.

* Fixing race test failure in TestAdsEds (#5161)

Automatic merge from submit-queue.

Fixing race test failure in TestAdsEds

introduced by #4694
addresses #4235

* v1alpha1 to v1alpha3 rule conversion tool bug fixes and subset merging (#5178)

* v1 to v3 conversion enhancements and tests

* Handle DestinationPolicy w/o labels

* Remove AddJwtAuth (#5194)

Automatic merge from submit-queue.

Remove AddJwtAuth

There is a compile error.
# istio.io/istio/mixer/test/client/env
../../../../../mixer/test/client/env/mixer_filter_config.go:167:47: undefined: client.JWT
../../../../../mixer/test/client/env/mixer_filter_config.go:168:21: mfConf.PerRouteConf.EndUserAuthnSpec undefined (type *client.ServiceConfig has no field or method EndUserAuthnSpec)
../../../../../mixer/test/client/env/mixer_filter_config.go:168:42: undefined: client.EndUserAuthenticationPolicySpec
../../../../../mixer/test/client/env/mixer_filter_config.go:169:21: mfConf.PerRouteConf.EndUserAuthnSpec undefined (type *client.ServiceConfig has no field or method EndUserAuthnSpec)

Remove AddJwtAuth function.

cc @diemtvu

* Skip bad routes instead of erroring (#5183)

* Skip bad routes instead of erroring

Signed-off-by: Shriram Rajagopalan <[email protected]>

* nits

Signed-off-by: Shriram Rajagopalan <[email protected]>

* final nits

Signed-off-by: Shriram Rajagopalan <[email protected]>

* fix rules

* BlackHole with a capital H

* validate clusters false

Signed-off-by: Shriram Rajagopalan <[email protected]>

* Fetch/cache/refresh JWT public key, construct localJwks in sidecar filter config (#5061)

Automatic merge from submit-queue.

Fetch/cache/refresh JWT public key, construct localJwks in sidecar filter config

#4917

This PR includes 
1. fetch JWT public key, and cache the key.
2. key rotation - a refresher job refresh key periodically.
3. use the key to construct localJwks in sidecar filter config.

* Introduce dynamic proto3 encoder (#5122)

* WIP commit

* Remove dead code

* Rearrange code

* split code into encoderUtil

* Everything except ENUM

* use protoc 3.5.1 to ensure json names are generated

* expose internal funcs

* WIP3. all dynamic and static elementry types. No repeated or packed

* support packed static primitive types

* use switch in place of if

* primitives with eval and packed repeated

* all primitives with expressions

* add test with enum constants and expressions

* add expressions in repeated fields

* Refactor 2

* linter checks

* fix linter2

* split encoder and builder

* rename eval to primitive

* add all dynamic tests

* Add dependency for messagediff

* add full dynamic test

* update comment

* fix linter error

* Update vendor. Add messagediff.v1 for test verification

* add all positive tests

* improve test coverage

* remove updated to lang.compiled

* fix linter error

* handle float64 inputs for integers

* Builder.Build() takes msgName and data

* WIP2

* review comments

* review comments

* rename messagediff to diff

* add more tests

* Update deps

* improve test coverage

* add log message while skipping fields

* increase test coverage

* update dep status

* Add more files to gitignore (#5198)

* Fix Mixer dashboard CPU reporting (#5145)

Automatic merge from submit-queue.

Fix Mixer dashboard CPU reporting

A previous PR seems to have accidentally removed the "rate" component of
the CPU calculations for the Mixer Dashboard. This results in an ever-increasing
CPU graph.

This PR restores a proper rate-based display for CPU calculation. It also
renames the jobs in the Prometheus config to better align with the split
from Mixer to Istio-Telemetry and Istio-Mixer (providing easier to understand
tracking between cAdvisor metrics and the self-reported metrics.

This PR should be cherry-picked onto the 0.8 branch.

* fix nil reference error when mock server fails to start (#5216)

* [WIP] refactor bookinfo to use different gateway definitions for envoy v1/routing v1alpha1 and envoy v2/routing v1alpha3  (#5113)

* restrict the tests to either v1alpha1 or v1alpha3

* move applying defaultRules into setUpDefaultRouting

* extract Ingress (Gateway) definition from bookinfo.yaml

it is different for v1alpha1 and v1alpha3

* make the gateway rule first in defaultRules, so it will be applied first

* fixed wrong variable names in mixer tests

* fixed the location of bookinfo gateway yaml

* fixed wrong variable in mixer test

* add missing spec and name to destination-policy-reviews

* remove comment line in samples/bookinfo/routing/bookinfo-gateway.yaml

* add port 9080 to the new bookinfo gateway

* remove using a special destination rule for reviews

* refactor GetIngress to make it reusable for GetIngressGateway

extract functions for getting Kubernetes Ingress and NodePort

* remove a shadowing variable

* refactor GetIngressPod, add GetIngressGateway

* add IngressGateway() to framework Kube

* added using IngressGateway() of framework Kube in bookinfo e2e tests

* use load balancer ingress IP to get the IP of the nodeport

* use ingress IP for nodeport

* remove commented out line

* fixed getting the ingress as the IP for a NodePort

* Revert "fixed getting the ingress as the IP for a NodePort"

This reverts commit 594e58d.

* Revert "use ingress IP for nodeport"

This reverts commit 333b80f.

* Revert "use load balancer ingress IP to get the IP of the nodeport"

This reverts commit 3c138e4.

* add generate_yaml-envoyv2_transition_loadbalancer_ingressgateway

to generate istio configurations without ingress and with ingressgateway as
a LoadBalancer service

* use generate_yaml-envoyv2_transition_loadbalancer_ingressgateway in test/local/noauth/e2e_bookinfo_envoyv2

* added LoadBalancerServiceType and NodePortServiceType constants

* rewrote the ingress related logic

use LoadBalancer type for non-local and NodePort for local tests

* lint fixes

* fix lint errors

* *sync.Locker -> sync.Locker, use interface instead of a pointer to interface

* refactor: extract getServicePort() from getServiceNodePort()

* add isKubernetesIngress flag to tests/util.GetIngress()

* fix the destination port in the virtual service of the gateway

* Revert "add isKubernetesIngress flag to tests/util.GetIngress()"

This reverts commit 8dbe13c.

* set different retry values for LoadBalancer and NodePort

according to the original implementation

* fix logging message

* fix a typo

* Introduce pkg/ctrlz, Istio's introspection package. (#5123)

* Introduce pkg/ctrlz, Istio's introspection package.

Processes that integrate with ControlZ open up a port that enables operators
to connect with a web browser and interact with the process. Through the browser,
the operator can adjust logging scope levels, see the process' command-line arguments
and envirinment variables, see statistics about heap use, and more.

Integration with ControlZ is nominally two line deal for processes. Optionally,
processes can extend the base ControlZ UI and integrate their own screens into the
main UI.

In addition to the browser interface, there is a REST API enabling access to all
the same things that the UI shows.

Mixer is integrated with ControlZ but doesn't currently have custom UI. We should
integrate ControlZ with our other server components in due time.

* Add myself to owners. (#5039)

* pod Ip is actually required

Service vip doesn’t exist for non existent port and we need a non
existent port to get the bad routing behavior

* Expose image of each istio component for istio chart. (#5222)

Automatic merge from submit-queue.

Expose image of each istio component for istio chart.

Make `image` for each Istio component be configurable. 
This is useful in case that users build or retag Istio image.

/cc @gyliu513 @linsun @sdake

* Undoing accidental merge to master

* Adding zone/region node labeling if missing (#5164)

* Fixing missing INSTANCE_IP

* Fix yaml error

* Rename v1alpha3.ExternalSerivce to v1alpha3.ServiceEntry (#5195)

* first pass renaming v1alpha3.ExternalSerivce to v1alpha3.ServiceEntry

* rename ServiceEntry.Discovery to ServiceEntry.Resolution

* update vendor to latest istio/api

* fix cloudfoundry copilot e2e test (#5188)

* initial changes to fix both pilot endpoints

* they now should be curl'ing the right things

properly booting an envoy with dynamic
template now

new port name for building listeners

Include port for Cloud Foundry services

* Building listeners now requires named ports.

* always run cloudfoundry tests

* moves cloudfoundry circleci test to own run

* adds cloudfoundry test to all

* want to just use default env vars

* need GOPATH/bin on path for envoy

* switch to defaults which uses da container

* disable zipkin test in pilot

* add missing clusters to ads mesh response (#5221)

* e2e test for JWT authn policy (#5144)

Automatic merge from submit-queue.

e2e test for JWT authn policy

#5078

1. JWT token used here expires in year 2132 (borrowed from https://github.com/istio/proxy/blob/master/src/envoy/http/jwt_auth/sample/correct_jwt). 
2. will add another e2e test for fetching JWT public key scenario after #5061 is in.

* Set listeners h2 max streams to override nghttp2 client default of 100 (#5232)

Automatic merge from submit-queue.

Set listeners h2 max streams to override nghttp2 client default of 100

Reference issue: envoyproxy/envoy#3076
Signed-off-by: Kuat Yessenov <[email protected]>

* Enable ControlZ to fetch the current process' known logging scopes. (#5245)

Automatic merge from submit-queue.

Enable ControlZ to fetch the current process' known logging scopes.

* Add more parameters to sidecar injector helm template (#5044)

Automatic merge from submit-queue.

Add enableCoreDump and policy parameters to sidecar injector helm template

* Fixing fallout of renames in earlier commit + restore auth for e2e-simple on circle (#5241)

* Fixing fallout of renames in earlier commit

* Re fixing lost fix that e2e-simple should run with auth

Technically it should run with both auth and no auth like on prow but
if it runs only 1 mode it should be with auth

* follow output log pattern for cloudfoundry e2e test (#5234)

- and tee to a new file so it doesn't overwrite

* bootstrapv2: Stop using deprecated cluster_names (#5225)

Using cluster_names in GRPC resource config is deprecated:
envoyproxy/envoy@ad02e4a

Signed-off-by: Romain Lenglet <[email protected]>

* Address a few causes of Gateway/Filterchain failures (#5185)

* Sort HTTP route virtual hosts before sending listeners to Envoy.
Listeners with multiple filter chains containing HTTP filters require
that the HTTP filters have consistent ordering due to how Envoy computes
updates.

* don't respond with empty listeners

* address review comments

* fix linter

* linters, once more

* use configurable paths for envoy and envoy config locations (#5248)

* re-add istioctl unit tests to Makefile (#5205)

* re-add istioctl unit tests to Makefile

#3820 moved istioctl out of pilot
subdirectory but forgot to re-add istioctl unit tests to top-level
Makefile. Fix that problem and also the currently broken tests.

* add missing test data

* return an error when Envoy fails to start (#5251)

mixer and backend should also do this, but that involves slightly more
work.
* add helm testing

* adding a few supporting methods for helm

* test: modify to invoke helm install

* Revert "test: modify to invoke helm install"

This reverts commit 0083f3c.

* adding a few function to install tiller

* add pod name in log

* customize values for helm install

* try enable helm installer

* change to the right time

* fix build issue

* fix build issue

* set correct helm path and params

* fix e-2-e error in helm dry run

* use the correct install dir

* use the correct namespace for the testing
* Crash fix

* Adjusting the fix
Automatic merge from submit-queue.

check in #5238 to 0.8 branch 

check in #5238 to 0.8 branch, which is required for jwt authn policy to work in v2.
* added printing unexpected version in version migration tests

* print the diffs with the compared versions in case migration test fails

* apply default rules after every bookinfo test

in v1alpha3 there is no rule precendence, a new rule just deletes the old one
there is no possibility to have two rules on the same host

* apply all the default rules instead of only allRule after each test
* Crash fix

* Adjusting the fix

* fixing Hostname assignement

* Fix collateral from the change

* Adding inbound to if
* Enable mTLS for pilot e2e tests

* Change generate_yaml-envoyv2_transition to output to istio-auth.yaml as test is in auth enabled mode

* Add grpc ports to containerPort list as inboundPorts are limitted by these since #5070

* Disable rbac e2e test as it crash when authn enabled.

* Disable egressgateway when mTLS enable.
There will be error like this if this field is missing:
Object 'Kind' is missing in ...
* Fix doc

* Revert code change to pass test
This change makes the output denser and easier to read.

Example usage (bookinfo example):

$ istioctl experimental metrics productpage reviews ratings details
    SERVICE    TOTAL RPS    ERROR RPS  P50 LATENCY  P90 LATENCY  P99 LATENCY
productpage        7.873        0.000         40ms         80ms         98ms
    reviews        7.909        0.000          4ms          9ms         21ms
    ratings        5.309        0.000          2ms          4ms          4ms
    details        7.873        0.000          3ms         38ms         48ms

Signed-off-by: Piotr Sikora <[email protected]>
…#5326)

* use env.Mesh.IngressService instead of hardcoded string

* add definition of IngressService to the mock mesh in the proxy config test

* add dot to prefix comparison of Ingress Service
* Update proxy sha to latest.

* update to newer proxy sha
* Change service entry for egressgateway to b, which is in the mesh, so that test works when authn is enabled.

* Disable mTLS for service t so it can be used as fake external service.

* Add missing policy yaml.

* Add comment to explain the purpose of authn policy for egressgateway test.

* Revert accidental revert.

* Correct fix: disable mTLS for egressgateway instead.

* Correct authn policy yaml file.

* Correct policy target name.
* Update envoy_telemetry.yaml.tmpl

* Update envoy_policy.yaml.tmpl
RBAC consistently failed for days - the other tests were broken in post-submit as well
* Test and more bug fixes.

Adding more coverage to the local tests showed that mixer can break
listeners in some cases - this is a P0, we shouldn't cut release until
this is in.

* Remove select used for debug, too verbose message

* Fix lint, format. Add few metrics on rejected configs

* More debug/monitoring help

* More testing and debuggability. Refactored the cluster method to allow more info in the message and simplify

* Update timeout

* More varz, fix lint/race

* Move controller test out, seems to be interfering with the other tests

* Use default timeout, add the moved controller_test

* If AuthPolicy is MTLS, use the MTLS port
No code change, needed to fix the branch.
@googlebot googlebot added cla: yes and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels May 14, 2018
@hklai hklai requested a review from costinm May 15, 2018 00:19
@googlebot
Copy link
Collaborator

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@googlebot googlebot added cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. and removed cla: yes labels May 15, 2018
@costinm
Copy link
Contributor

costinm commented May 15, 2018

Main problem seems to be lint - make depend.diff needs to be fixed or everyone will be blocked.

@hklai hklai added cla: yes and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels May 15, 2018
@googlebot
Copy link
Collaborator

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

@hklai hklai requested a review from ldemailly May 15, 2018 22:51
@ldemailly
Copy link
Member

/lgtm

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ldemailly

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

The pull request process is described here

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

@hklai
Copy link
Contributor Author

hklai commented May 15, 2018

Discussed with @ldemailly and @costinm and we are merging this using an API version in the release-0.8 branch.

The latest API version in master is using a newer codegen (due to istio/api#477) and the generated code is not compatible. It needs to be fixed separately.

/CC @geeknoid

@istio-testing
Copy link
Collaborator

istio-testing commented May 15, 2018

@hklai: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/istio-pilot-e2e-envoyv2-v1alpha3.sh 32caec8 link /test istio-pilot-e2e-envoyv2-v1alpha3
prow/istio-pilot-e2e.sh 32caec8 link /test istio-pilot-e2e
prow/e2e-bookInfoTests-v1alpha3.sh 32caec8 link /test e2e-bookInfo-envoyv2-v1alpha3

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.

@geeknoid
Copy link
Contributor

I've got a pending PR to master which addresses the code gen stuff: #5623

I can merge this immediately to master, or immediately after release 0.8 is merged to master, or I could potentially merge it into the 0.8 branch. Please let me know what works best in the grand scheme of things.

@hklai
Copy link
Contributor Author

hklai commented May 15, 2018

TestTCPMixerFilter is failing in circle but passed in prow.

If it is a persistent issue we will deal with it later.

@hklai hklai merged commit 26262e8 into istio:master May 15, 2018
@hklai
Copy link
Contributor Author

hklai commented May 16, 2018

@geeknoid Please do not merge to release-0.8 if it is not needed in 0.8.x.

The codegen problem happened when I tried to use the latest API version, which I don't think #5623 is doing. Could you try updating the API version?

@ldemailly
Copy link
Member

ldemailly commented May 16, 2018

@geeknoid issues we found when updating api past your tooling change:

  • gogo and protobuf needed upgrade to 1.0.0 (for 0.5) and 1.1.0 (from 1.0.0) respectively (in the toml followed by dep ensure) to compile without getting InternalMessageInfo compile error on the new xxx_ generated code
  • because a different generator is used for v1alpha1 and v1alpha3 networking, code in istioctl/convert needs to change to manually copy Uri and Authority

In short api tooling change should be preceded by trying to build istio/istio and immediately followed by an istio/istio PR to fix/adapt to the changes (or we should move istio/api inside istio/istio as was discussed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.