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.

Sign up for GitHub

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

Jump to bottom

configs: Update configs v2 -> v3 #13562

Merged
merged 9 commits into from
Oct 16, 2020
Merged

configs: Update configs v2 -> v3 #13562

merged 9 commits into from
Oct 16, 2020

Conversation

phlax
Copy link
Member

@phlax phlax commented Oct 14, 2020

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message: configs: Update configs v2 -> v3
Additional Description:

Following on from #13529 (comment)

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]

@phlax phlax changed the title configs: Update configs v2 -> v3 [WIP] configs: Update configs v2 -> v3 Oct 14, 2020
@phlax phlax marked this pull request as draft October 14, 2020 10:28
@phlax
Copy link
Member Author

phlax commented Oct 14, 2020

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13562 (comment) was created by @phlax.

see: more, trace.

@phlax
Copy link
Member Author

phlax commented Oct 14, 2020

i have renamed all yaml files containing v2 in name to equiv v3 - with the exception of the following:

./test/server/test_data/server/valid_v3_but_invalid_v2_bootstrap.yaml
./test/server/test_data/server/valid_v2_but_invalid_v3_bootstrap.yaml
./examples/ext_authz/config/grpc-service/v2.yaml
./examples/ext_authz/config/opa-service/v2.yaml
./configs/using_deprecated_config.v2.yaml

i wasnt sure about these

im just about to update the v2 configs in the renamed files...

@phlax
Copy link
Member Author

phlax commented Oct 14, 2020 •
edited
Loading

grepping v2 in configs now gives;

$ git grep v2 configs/ | grep "\.yaml"
configs/freebind/freebind.yaml:          "@type": type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager
configs/google-vrp/envoy-edge.yaml:      "@type": type.googleapis.com/envoy.config.resource_monitor.fixed_heap.v2alpha.FixedHeapConfig
configs/google-vrp/envoy-origin.yaml:      "@type": type.googleapis.com/envoy.config.resource_monitor.fixed_heap.v2alpha.FixedHeapConfig
configs/original-dst-cluster/proxy_config.yaml:          "@type": type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager
configs/using_deprecated_config.v2.yaml:          "@type": type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager
configs/using_deprecated_config.v2.yaml:        "@type": type.googleapis.com/envoy.api.v2.auth.UpstreamTlsContext
configs/using_deprecated_config.v2.yaml:      envoy.deprecated_features:envoy.config.trace.v2.ZipkinConfig.HTTP_JSON_V1: true
configs/using_deprecated_config.v2.yaml:      envoy.deprecated_features:envoy.api.v2.route.CorsPolicy.allow_origin: true

im not sure which of these need updating

@htuch htuch self-assigned this Oct 14, 2020
@htuch
Copy link
Member

htuch commented Oct 14, 2020

@phlax at this point, everything other than using_deprecated_config should be converted or deleted.

@phlax
Copy link
Member Author

phlax commented Oct 14, 2020

@htuch does that mean removing these two ?

./test/server/test_data/server/valid_v3_but_invalid_v2_bootstrap.yaml
./test/server/test_data/server/valid_v2_but_invalid_v3_bootstrap.yaml

@htuch
Copy link
Member

htuch commented Oct 14, 2020

@phlax actually, keep those, thanks.

@mattklein123
Copy link
Member

@phlax at this point, everything other than using_deprecated_config should be converted or deleted.

Can we move that one to test somewhere?

@htuch
Copy link
Member

htuch commented Oct 14, 2020

I think the v2 aspect of configs/using_deprecated_config.v2.yaml isn't super important, we should have coverage elsewhere of that. What we do need is a test using a runtime static_layer to override individual fields as an example I think, so maybe update this to v3 and v3 deprecated fields?

@phlax
Copy link
Member Author

phlax commented Oct 15, 2020

at this point - there is the following

$ find . -name "*v2*\.yaml"
./test/server/test_data/server/valid_v3_but_invalid_v2_bootstrap.yaml
./test/server/test_data/server/valid_v2_but_invalid_v3_bootstrap.yaml
./examples/ext_authz/config/grpc-service/v2.yaml
./examples/ext_authz/config/opa-service/v2.yaml
./configs/using_deprecated_config.v2.yaml

$ git grep v2 configs/ | grep "\.yaml"
configs/google-vrp/envoy-edge.yaml:      "@type": type.googleapis.com/envoy.config.resource_monitor.fixed_heap.v2alpha.FixedHeapConfig
configs/google-vrp/envoy-origin.yaml:      "@type": type.googleapis.com/envoy.config.resource_monitor.fixed_heap.v2alpha.FixedHeapConfig
configs/using_deprecated_config.v2.yaml:          "@type": type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager
configs/using_deprecated_config.v2.yaml:        "@type": type.googleapis.com/envoy.api.v2.auth.UpstreamTlsContext
configs/using_deprecated_config.v2.yaml:      envoy.deprecated_features:envoy.config.trace.v2.ZipkinConfig.HTTP_JSON_V1: true
configs/using_deprecated_config.v2.yaml:      envoy.deprecated_features:envoy.api.v2.route.CorsPolicy.allow_origin: true

im not 100% clear about what to do with using_deprecated_.. but ill update with how i understand...

@phlax phlax force-pushed the configs-deprecated-configs branch 2 times, most recently from b07c160 to f8f8231 Compare October 15, 2020 18:00
@htuch
Copy link
Member

htuch commented Oct 16, 2020

Looks good, thanks. Let me know when you have the using_deprecated update. It should be as simple as just changing the deprecated fields to be v3 ones (grep for deprecated in v3 packages). Feel free to reach out on Slack if you want more details.

@phlax phlax changed the title [WIP] configs: Update configs v2 -> v3 configs: Update configs v2 -> v3 Oct 16, 2020
@phlax phlax marked this pull request as ready for review October 16, 2020 07:57
@phlax
Copy link
Member Author

phlax commented Oct 16, 2020 •
edited
Loading

@htuch this should be ready for review.

I wasnt sure if the fields being deprecated in use_deprecated_ needed to change - i left them as they are.

also wondering if i should rename the file v2 -> v3

@phlax phlax force-pushed the configs-deprecated-configs branch 2 times, most recently from 24f90bd to 50926ed Compare October 16, 2020 08:02
@phlax phlax mentioned this pull request Oct 16, 2020
Closed
@phlax phlax force-pushed the configs-deprecated-configs branch 2 times, most recently from 616cfa8 to d31ab1e Compare October 16, 2020 14:54
@phlax
configs: Update configs v2 -> v3
56bef58 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
temp remove pipeline precheck dep
d17e213 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
update using_deprecated_config
d806a29 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
readd azure precheck pipeline dep
68e5163 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
disable pipeline check
d5de85a 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
fix access log strings
1014d3c 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
restore precheck dep
ffc61e1 
Signed-off-by: Ryan Northey <ryan@synca.io>
@mattklein123 mattklein123 self-assigned this Oct 16, 2020
mattklein123
mattklein123 reviewed Oct 16, 2020
View reviewed changes
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks LGTM with one question.

/wait-any

ci/Dockerfile-envoy Outdated Show resolved Hide resolved
@phlax
remove version suffix from config files
4c8a4d5 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
remove version suffix from config files
cc43e3a 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Member Author

phlax commented Oct 16, 2020

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13562 (comment) was created by @phlax.

see: more, trace.

@mattklein123 mattklein123 merged commit 85784cb into envoyproxy:master Oct 16, 2020
mpuncel added a commit to mpuncel/envoy that referenced this pull request Oct 17, 2020
@mpuncel
Merge branch 'master' into mpuncel/http2-hc-goaway
b76c46e 
* master: (22 commits)
  delay health checks until transport socket secrets are ready. (envoyproxy#13516)
  test, oauth2: Make sure config test runs field validation (envoyproxy#13496)
  [http] swap codec implementations to default new (envoyproxy#13579)
  wasm: update proxy-wasm-cpp-host (envoyproxy#13606)
  postgres: do not copy and linearize received data when it is not going to be used (envoyproxy#13393)
  configs: Update configs v2 -> v3 (envoyproxy#13562)
  http2: Remove RELEASE_ASSERTs in sendPendingFrames() error handling (envoyproxy#13546)
  dependencies: track untracked implied dependencies, wrapup dashboard. (envoyproxy#13571)
  listener: add match all filter chain (envoyproxy#13449)
  fix mistakes in docstrings (envoyproxy#13603)
  ratelimit: add route entry metadata to ratelimit actions (envoyproxy#13269)
  cluster manager: avoid immediate activation for dynamic inserted cluster when initialize (envoyproxy#12783)
  ext_authz: Avoid calling check multiple times (envoyproxy#13288)
  docs: Unexclude remaining configs from validation (envoyproxy#13534)
  build: update rules_rust to allow Rustc in RBE (envoyproxy#13595)
  docs: Update sphinxext.rediraffe (envoyproxy#13589)
  Deprecate moonjit support on Windows before beta (envoyproxy#13541)
  dependencies: bump LuaJIT to 2.1 branch HEAD @ e9af1ab. (envoyproxy#13474)
  docs: add TLS stats to cluster stats doc (envoyproxy#13561)
  ci: stop building alpine-debug images in favor of ubuntu-based debug image (envoyproxy#13598)
  ...

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
@phlax phlax mentioned this pull request Oct 21, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@htuch htuch htuch left review comments

@mattklein123 mattklein123 mattklein123 approved these changes

Assignees

@mattklein123 mattklein123

@htuch htuch

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@phlax @htuch @mattklein123