Revert "Support api config source without cluster names (#2999)"#3018
Merged
mattklein123 merged 1 commit intoenvoyproxy:masterfrom Apr 6, 2018
Merged
Conversation
)" This reverts commit 01e75f9. Discuession on #envoy-users revealed that this is going to break operationally for folks who are trying to roll forward binaries with the same config, there's no config that can work both before/after. Signed-off-by: Harvey Tuch <htuch@google.com>
mattklein123
approved these changes
Apr 6, 2018
Member
Author
|
@ambuc FYI; we can discuss Monday, but we need to weaken one of the checks here to avoid folks breaking when they are rolling forward binaries without config. Most of this PR can be re-reverted as is. |
ambuc
added a commit
to ambuc/envoy
that referenced
this pull request
Apr 9, 2018
…yproxy#2999)" (envoyproxy#3018)" This reverts commit fdcbe10. Signed-off-by: James Buckland <jbuckland@google.com>
htuch
pushed a commit
that referenced
this pull request
Apr 11, 2018
This reverts #3018, which reverted #2999. Therefore this PR is similar to #2999, except that 77b292c allows REST/gRPC config mismatch errors to warn instead of throwing an error. Hopefully this solves the issue for users who were unable to roll forward their binaries while keeping the same config. Changelist for #2999: To support the deprecation of cluster_names in the api_config_source (#2860), I added a more verbose error in utility.cc for GRPC api_config_sources with any named clusters hunted down places in tests where api_config_source.cluster_names()[0] was implicitly used (assuming that API would be GRPC and would have cluster names). renamed factoryForApiConfigSource to factoryForGrpcApiConfigSource, as it already implicitly returns a Grpc::AsyncClientFactoryPtr, and gave a more verbose error for when the user passes a config with cluster_names set. separated out checkApiConfigSourceSubscriptionBackingCluster into checkApiConfigSourceNames, which does the gRPC services v. cluster names validation, and - checkApiConfigSourceSubscriptionBackingCluster, which actually validates the cluster name against the clusters map. more completely covered the tests for the branching cases for non-gRPC API configs which happened to have grpc services defined. Risk Level: Low Testing: bazel test test/... all passed. Fixed #2680 Fixed #2902 Signed-off-by: James Buckland <jbuckland@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts commit 01e75f9.
Discussion on #envoy-users revealed that this is going to break operationally for folks who are
trying to roll forward binaries with the same config, there's no config that can work both
before/after.
Signed-off-by: Harvey Tuch htuch@google.com