Skip to content

config subscription bazel settings#4871

Merged
istio-testing merged 3 commits intorelease-1.18from
grpcsub-filesystem
Aug 7, 2023
Merged

config subscription bazel settings#4871
istio-testing merged 3 commits intorelease-1.18from
grpcsub-filesystem

Conversation

@zirain
Copy link
Copy Markdown
Member

@zirain zirain commented Aug 7, 2023

@zirain zirain requested a review from a team August 7, 2023 07:36
@istio-testing istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 7, 2023
@zirain zirain changed the base branch from master to release-1.18 August 7, 2023 07:37
@zirain zirain requested a review from a team August 7, 2023 07:37
@istio-testing istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 7, 2023
@pmerrison
Copy link
Copy Markdown

pmerrison commented Aug 7, 2023

/retest

mhyllander
mhyllander reviewed Aug 7, 2023
View reviewed changes
Comment thread bazel/extension_config/extensions_build_config.bzl Outdated
Comment on lines +424 to +436

#
# Config Subscription
#
"envoy.config_subscription.rest": "//source/extensions/config_subscription/rest:http_subscription_lib",
"envoy.config_subscription.filesystem": "//source/extensions/config_subscription/filesystem:filesystem_subscription_lib",
"envoy.config_subscription.filesystem_collection": "//source/extensions/config_subscription/filesystem:filesystem_subscription_lib",
"envoy.config_subscription.grpc": "//source/extensions/config_subscription/grpc:grpc_subscription_lib",
"envoy.config_subscription.delta_grpc": "//source/extensions/config_subscription/grpc:grpc_subscription_lib",
"envoy.config_subscription.ads": "//source/extensions/config_subscription/grpc:grpc_subscription_lib",
"envoy.config_subscription.aggregated_grpc_collection": "//source/extensions/config_subscription/grpc:grpc_collection_subscription_lib",
"envoy.config_subscription.aggregated_delta_grpc_collection": "//source/extensions/config_subscription/grpc:grpc_collection_subscription_lib",
"envoy.config_subscription.ads_collection": "//source/extensions/config_subscription/grpc:grpc_collection_subscription_lib",
Copy link
Copy Markdown

@mhyllander mhyllander Aug 7, 2023

Choose a reason for hiding this comment

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

I'm not an expert, but shouldn't this have been added to ENVOY_EXTENSIONS instead of ENVOY_CONTRIB_EXTENSIONS?

Copy link
Copy Markdown
Member Author

@zirain zirain Aug 7, 2023

Choose a reason for hiding this comment

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

thanks for remind, it's a mistake.

hanxiaop
hanxiaop reviewed Aug 7, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@hanxiaop hanxiaop left a comment

Choose a reason for hiding this comment

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

Should this be in the master branch as well and then do a cherry-pick to 1.18?

@istio-testing istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Aug 7, 2023
@zirain
Copy link
Copy Markdown
Member Author

zirain commented Aug 7, 2023

Should this be in the master branch as well and then do a cherry-pick to 1.18?

master is right, I'm not sure why this's not sync. see #4642

@istio-testing istio-testing added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 7, 2023
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Aug 7, 2023 •
edited
Loading

@zirain: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
release-test-arm64_proxy fb26940 link true /test release-test-arm64
test-tsan_proxy fb26940 link true /test test-tsan
test-arm-arm64_proxy fb26940 link true /test test-arm-arm64
release-test_proxy fb26940 link true /test release-test
test-asan_proxy fb26940 link true /test test-asan
test_proxy fb26940 link true /test test
check-wasm_proxy fb26940 link true /test check-wasm
Details

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.

@hanxiaop
Copy link
Copy Markdown
Member

hanxiaop commented Aug 7, 2023

Should this be in the master branch as well and then do a cherry-pick to 1.18?

master is right, I'm not sure why this's not sync. see #4642

I see, thanks

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Aug 7, 2023

/test test-tsan

@istio-testing istio-testing merged commit 9e63498 into release-1.18 Aug 7, 2023
@istio-testing istio-testing deleted the grpcsub-filesystem branch August 7, 2023 12:40
@ericvn
Copy link
Copy Markdown

ericvn commented Aug 7, 2023

master is right, I'm not sure why this's not sync. see #4642

Looks like this merged in the main branch on May 6th, while the branch cut for 1.18 was on May 3rd. Would have needed a cherry-pick.

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Aug 7, 2023

need a better way to check to difference between upstream and proxy repo, in particular for ENVOY_EXTENSIONS.

@hanxiaop hanxiaop mentioned this pull request Aug 11, 2023
Closed
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hanxiaop hanxiaop hanxiaop approved these changes

+1 more reviewer

@mhyllander mhyllander mhyllander left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

istio-proxy is missing some envoy extensions since 1.18.0

6 participants

@zirain @pmerrison @istio-testing @hanxiaop @ericvn @mhyllander