Skip to content

Add a test client for certain grpclb fallback scenarios#19623

Merged
apolcyn merged 2 commits intogrpc:masterfrom
apolcyn:grpclb_fallback_test
Jul 12, 2019
Merged

Add a test client for certain grpclb fallback scenarios#19623
apolcyn merged 2 commits intogrpc:masterfrom
apolcyn:grpclb_fallback_test

Conversation

@apolcyn
Copy link
Copy Markdown
Contributor

@apolcyn apolcyn commented Jul 12, 2019

This synchronizes upstream test protos from grpc/grpc-proto#55 and grpc/grpc-proto#56, and then adds a new one-off test client.

Checking this test into this repo is mostly done to simply the build process - the runner for this test will live outside of this repo. Also note this has been manually tested.

@apolcyn apolcyn added lang/c++ area/test release notes: no Indicates if PR should not be in release notes labels Jul 12, 2019
@apolcyn apolcyn force-pushed the grpclb_fallback_test branch from 867fbd2 to 4999420 Compare July 12, 2019 01:31
Copy link
Copy Markdown
Contributor

@yang-g yang-g left a comment

Choose a reason for hiding this comment

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

Mostly some nits.

}

void RunFallbackAfterStartupTest(
const std::string break_lb_and_backend_conns_cmd) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

string& ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

}

void RunFallbackBeforeStartupTest(
const std::string break_lb_and_backend_conns_cmd,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

string& ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

socklen_t len = sizeof(newval);
if (0 != getsockopt(fd, IPPROTO_TCP, TCP_USER_TIMEOUT, &newval, &len) ||
newval != timeout) {
gpr_log(GPR_ERROR, "Failed to set socket option TCP_USER_TIMEOUT");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/set/get/ ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done, slightly reworded

grpc::CreateCustomChannel(FLAGS_server_uri, channel_creds, channel_args));
}

void RunCommand(const std::string command) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

string& ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

"linux",
"mac",
"posix",
"windows"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure it matters but I suspect this will not run under windows?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good point, restricted to linux only - this test is indeed only meant for linux

@apolcyn
Copy link
Copy Markdown
Contributor Author

apolcyn commented Jul 12, 2019

Comments addressed, thanks for the quick review!

@apolcyn
Copy link
Copy Markdown
Contributor Author

apolcyn commented Jul 12, 2019

C/C++ macos: #19614
Node Macos: #19422
Objc Macos: https://github.com/grpc/grpc/issues
Interop cloud-to-cloud: #19626

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area/test lang/c++ 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