From 815f2a8a4c4e67e8dfe2e0877f6574656027fdef Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 19 Nov 2024 19:17:22 +0530 Subject: [PATCH] separate test for new watcher resource remove --- .../xdsclient/tests/lds_watchers_test.go | 122 +++++++++++++++--- 1 file changed, 103 insertions(+), 19 deletions(-) diff --git a/xds/internal/xdsclient/tests/lds_watchers_test.go b/xds/internal/xdsclient/tests/lds_watchers_test.go index 482312aa4ed5..23520a34c27c 100644 --- a/xds/internal/xdsclient/tests/lds_watchers_test.go +++ b/xds/internal/xdsclient/tests/lds_watchers_test.go @@ -742,9 +742,7 @@ func (s) TestLDSWatch_ValidResponseCancelsExpiryTimerBehavior(t *testing.T) { // scenarios: // 1. Removing a resource should trigger the watch callback with a resource // removed error. It should not trigger the watch callback for an unrelated -// resource. If any new watcher try to register watcher for removed -// resource, it should trigger watch callback to it with a resource removed -// error as well. +// resource. // 2. An update to another resource should result in the invocation of the watch // callback associated with that resource. It should not result in the // invocation of the watch callback associated with the deleted resource. @@ -849,20 +847,8 @@ func (s) TestLDSWatch_ResourceRemoved(t *testing.T) { t.Fatal(err) } - // Any new watcher trying to register for removed resource, should get - // resource removed error. - lw3 := newListenerWatcher() - ldsCancel3 := xdsresource.WatchListener(client, resourceName1, lw3) - defer ldsCancel3() - if err := verifyListenerUpdate(ctx, lw3.updateCh, listenerUpdateErrTuple{ - err: xdsresource.NewErrorf(xdsresource.ErrorTypeResourceNotFound, ""), - }); err != nil { - t.Fatal(err) - } - // Update the second listener resource on the management server. The first - // and third watcher should not see an update, while the second watcher - // should. + // watcher should not see an update, while the second watcher should. resources = e2e.UpdateOptions{ NodeID: nodeID, Listeners: []*v3listenerpb.Listener{e2e.DefaultClientListener(resourceName2, "new-rds-resource")}, @@ -874,9 +860,6 @@ func (s) TestLDSWatch_ResourceRemoved(t *testing.T) { if err := verifyNoListenerUpdate(ctx, lw1.updateCh); err != nil { t.Fatal(err) } - if err := verifyNoListenerUpdate(ctx, lw3.updateCh); err != nil { - t.Fatal(err) - } wantUpdate = listenerUpdateErrTuple{ update: xdsresource.ListenerUpdate{ RouteConfigName: "new-rds-resource", @@ -888,6 +871,107 @@ func (s) TestLDSWatch_ResourceRemoved(t *testing.T) { } } +// TestLDSWatch_NewWatcherForRemovedResource covers the case where a new +// watcher registers for the resource which is removed. The test verifies +// the following scenarios: +// 1. Removing a resource should trigger the watch callback with a resource +// removed error to existing watcher. +// 2. If any new watcher try to register for removed resource, it should +// trigger watch callback to it with a resource removed error as well. +func TestLDSWatch_NewWatcherForRemovedResource(t *testing.T) { + mgmtServer := e2e.StartManagementServer(t, e2e.ManagementServerOptions{}) + + nodeID := uuid.New().String() + authority := makeAuthorityName(t.Name()) + bc, err := bootstrap.NewContentsForTesting(bootstrap.ConfigOptionsForTesting{ + Servers: []byte(fmt.Sprintf(`[{ + "server_uri": %q, + "channel_creds": [{"type": "insecure"}] + }]`, mgmtServer.Address)), + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), + Authorities: map[string]json.RawMessage{ + // Xdstp style resource names used in this test use a slash removed + // version of t.Name as their authority, and the empty config + // results in the top-level xds server configuration being used for + // this authority. + authority: []byte(`{}`), + }, + }) + if err != nil { + t.Fatalf("Failed to create bootstrap configuration: %v", err) + } + testutils.CreateBootstrapFileForTesting(t, bc) + + // Create an xDS client with the above bootstrap contents. + client, close, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + Name: t.Name(), + Contents: bc, + }) + if err != nil { + t.Fatalf("Failed to create xDS client: %v", err) + } + defer close() + + // Register watch for the listener resources and have the + // callbacks push the received updates on to a channel. + resourceName := ldsName + lw1 := newListenerWatcher() + ldsCancel1 := xdsresource.WatchListener(client, resourceName, lw1) + defer ldsCancel1() + + // Configure the management server to return listener resource, + // corresponding to the registered watch. + resource := e2e.UpdateOptions{ + NodeID: nodeID, + Listeners: []*v3listenerpb.Listener{e2e.DefaultClientListener(resourceName, rdsName)}, + SkipValidation: true, + } + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := mgmtServer.Update(ctx, resource); err != nil { + t.Fatalf("Failed to update management server with resource: %v, err: %v", resource, err) + } + + // Verify the contents of the received update for existing watch. + wantUpdate := listenerUpdateErrTuple{ + update: xdsresource.ListenerUpdate{ + RouteConfigName: rdsName, + HTTPFilters: []xdsresource.HTTPFilter{{Name: "router"}}, + }, + } + if err := verifyListenerUpdate(ctx, lw1.updateCh, wantUpdate); err != nil { + t.Fatal(err) + } + + // Remove the listener resource on the management server. + resource = e2e.UpdateOptions{ + NodeID: nodeID, + Listeners: []*v3listenerpb.Listener{}, + SkipValidation: true, + } + if err := mgmtServer.Update(ctx, resource); err != nil { + t.Fatalf("Failed to update management server with resource: %v, err: %v", resource, err) + } + + // The existing watcher should receive a resource removed error. + if err := verifyListenerUpdate(ctx, lw1.updateCh, listenerUpdateErrTuple{ + err: xdsresource.NewErrorf(xdsresource.ErrorTypeResourceNotFound, ""), + }); err != nil { + t.Fatal(err) + } + + // Any new watcher trying to register for removed resource, should get + // resource removed error as well. + lw2 := newListenerWatcher() + ldsCancel2 := xdsresource.WatchListener(client, resourceName, lw2) + defer ldsCancel2() + if err := verifyListenerUpdate(ctx, lw2.updateCh, listenerUpdateErrTuple{ + err: xdsresource.NewErrorf(xdsresource.ErrorTypeResourceNotFound, ""), + }); err != nil { + t.Fatal(err) + } +} + // TestLDSWatch_NACKError covers the case where an update from the management // server is NACK'ed by the xdsclient. The test verifies that the error is // propagated to the watcher.