xds: add Outlier Detection Balancer #5413
Conversation
dfawley
changed the title
| @@ -106,3 +107,43 @@ func (s) TestClientSideXDS(t *testing.T) { | |||
| t.Fatalf("rpc EmptyCall() failed: %v", err) | |||
| } | |||
| } | |||
|
|
|||
| func (s) TestOutlierDetection(t *testing.T) { | |||
There was a problem hiding this comment.
Could we please move this test to be in a file on its own. Otherwise over time, this file is going to become a dumping ground for tests. Thanks.
| resources := e2e.DefaultClientResources(e2e.ResourceParams{ | ||
| DialTarget: serviceName, | ||
| NodeID: nodeID, | ||
| Host: "localhost", | ||
| Port: port, | ||
| SecLevel: e2e.SecurityLevelNone, | ||
| }) |
There was a problem hiding this comment.
Do these resource even contain appropriate outlier detection configuration?
There was a problem hiding this comment.
It does. xdsclient will emit a nil Outlier Detection struct, which will cause the cds balancer to prepare a "no-op" Outlier Detection config. I verified it was present in the system manually through logs. Let me know if you want an explicit check.
There was a problem hiding this comment.
Will the test fail if the CDS balancer suddenly decides to not prepare a "no-op" Outlier Detection config? If so, how?
| if _, err := client.EmptyCall(ctx, &testpb.Empty{}, grpc.WaitForReady(true)); err != nil { | ||
| t.Fatalf("rpc EmptyCall() failed: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
So, what really are we testing here with regards to outlier detection?
There was a problem hiding this comment.
Doug mentioned to me he wanted a simple e2e sanity test with Outlier Detection as part of the system. More specific e2e functionality will come as part of the interop tests, which Michael is currently writing.
There was a problem hiding this comment.
But what sanity is this test testing? What change can cause this test to fail? And when it does fail, how does it help with identifying the issue that caused the failure?
|
Not sure if all changes suggested on the other PR have made it here. For example, I don't see the balancer registration being protected with an environment variable. So, I'll probably hold off reviewing this until you let me know that this is ready. |
| resources := e2e.DefaultClientResources(e2e.ResourceParams{ | ||
| DialTarget: serviceName, | ||
| NodeID: nodeID, | ||
| Host: "localhost", | ||
| Port: port, | ||
| SecLevel: e2e.SecurityLevelNone, | ||
| }) |
There was a problem hiding this comment.
It does. xdsclient will emit a nil Outlier Detection struct, which will cause the cds balancer to prepare a "no-op" Outlier Detection config. I verified it was present in the system manually through logs. Let me know if you want an explicit check.
| if _, err := client.EmptyCall(ctx, &testpb.Empty{}, grpc.WaitForReady(true)); err != nil { | ||
| t.Fatalf("rpc EmptyCall() failed: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Doug mentioned to me he wanted a simple e2e sanity test with Outlier Detection as part of the system. More specific e2e functionality will come as part of the interop tests, which Michael is currently writing.
| @@ -106,3 +107,43 @@ func (s) TestClientSideXDS(t *testing.T) { | |||
| t.Fatalf("rpc EmptyCall() failed: %v", err) | |||
| } | |||
| } | |||
|
|
|||
| func (s) TestOutlierDetection(t *testing.T) { | |||
|
Rebased and got to your comments in a new PR, #5435. |
This PR adds the Outlier Detection Balancer as part of the xDS configured tree of balancers (see https://github.com/grpc/proposal/blob/master/A50-xds-outlier-detection.md).
Contains #5371.
RELEASE NOTES: