Skip to content

[PSM Interop] Enable xDS custom LB test for Node#34146

Merged
murgatroid99 merged 1 commit intogrpc:masterfrom
murgatroid99:xds_test_node_enable_custom_lb
Aug 28, 2023
Merged

[PSM Interop] Enable xDS custom LB test for Node#34146
murgatroid99 merged 1 commit intogrpc:masterfrom
murgatroid99:xds_test_node_enable_custom_lb

Conversation

@murgatroid99
Copy link
Copy Markdown
Member

v1.10.x is the next version, so this will only run on master for now. This won't work until grpc/grpc-node#2555 is merged.

@murgatroid99 murgatroid99 requested a review from sergiitk August 23, 2023 22:47
@murgatroid99 murgatroid99 added the release notes: no Indicates if PR should not be in release notes label Aug 23, 2023
@murgatroid99 murgatroid99 changed the title Enable xDS custom LB interop tests for Node [PSM Interop] Enable xDS custom LB test for Node Aug 23, 2023
@sergiitk
Copy link
Copy Markdown
Member

sergiitk commented Aug 24, 2023

Running a lb tests using framework from this branch, and grpc-node code from grpc/grpc-node#2555.

@murgatroid99
Copy link
Copy Markdown
Member Author

It looks like the custom LB test didn't run in that test job, but I merged the Node repo PR, so running it against master should work now.

@sergiitk
Copy link
Copy Markdown
Member

@murgatroid99 I see - it didn't run because it's not in https://github.com/grpc/grpc-node/blob/354bd2d5c3c88ab736be669fcc245e64f4f352dd/packages/grpc-js-xds/scripts/xds_k8s_lb.sh#L168.

Could you please add it to the list, but also format and sort this list as we did here?

test_suites=(
"app_net_test"
"affinity_test"
"api_listener_test"
"change_backend_service_test"
"custom_lb_test"
"failover_test"
"outlier_detection_test"
"remove_neg_test"
"round_robin_test"
)

Let me know when the PR is ready - I'll run tests against it.

@murgatroid99
Copy link
Copy Markdown
Member Author

I filed that PR in grpc/grpc-node#2557.

@sergiitk
Copy link
Copy Markdown
Member

@sergiitk
Copy link
Copy Markdown
Member

sergiitk commented Aug 25, 2023

Failed:

Traceback (most recent call last):
  File "/tmp/tmp.CXtkTaZSEp/grpc/tools/run_tests/xds_k8s_test_driver/tests/custom_lb_test.py", line 120, in test_custom_lb_config
    self.assertRpcStatusCodes(
  File "/tmp/tmp.CXtkTaZSEp/grpc/tools/run_tests/xds_k8s_test_driver/framework/xds_k8s_testcase.py", line 410, in assertRpcStatusCodes
    self.fail(
AssertionError: Expected only status (15, DATA_LOSS), but found status (0, OK) for method UNARY_CALL.
Diff stats:
- method: UNARY_CALL
  rpcs_started: 247
  result:
    (0, OK): 247

@murgatroid99
Copy link
Copy Markdown
Member Author

murgatroid99 commented Aug 25, 2023

@murgatroid99
Copy link
Copy Markdown
Member Author

The test passed. The bugfix is in the same PR that enabled the test.

Copy link
Copy Markdown
Member

@sergiitk sergiitk left a comment

Choose a reason for hiding this comment

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

LGTM, but please make sure to merge the node change to master first (needed to not break staging)

@murgatroid99 murgatroid99 merged commit caa176c into grpc:master Aug 28, 2023
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Aug 28, 2023
murgatroid99 added a commit that referenced this pull request Sep 15, 2023
Similar to #34146, this will only run on master for now. This will work
after grpc/grpc-node#2568 is merged.
sergiitk pushed a commit to sergiitk/grpc that referenced this pull request Dec 8, 2023
v1.10.x is the next version, so this will only run on master for now.
This won't work until grpc/grpc-node#2555 is merged.
sergiitk pushed a commit to sergiitk/grpc that referenced this pull request Dec 8, 2023
Similar to grpc#34146, this will only run on master for now. This will work
after grpc/grpc-node#2568 is merged.
@ti-chi-bot ti-chi-bot bot mentioned this pull request Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/psm interop imported Specifies if the PR has been imported to the internal repository lang/node release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants