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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add ability to start container tests in debug mode and attach a debugger #16887
add ability to start container tests in debug mode and attach a debugger #16887
Changes from 6 commits
464c4be
9bd2582
f85924c
f9aa773
6c9ad62
94358e9
c67dac8
aa4d214
8fb470d
be7569b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we always expect the
--config-file
arg to be/consul/config/config.json
? I'm surprised we didn't need this in the args before?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes at least for the base image. I think without dlv it's passed through env vars which dlv don't pass along to the sub process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can replace this with
retry-go
, which is imported to the integ test:consul/test/integration/consul-container/libs/cluster/cluster.go
Lines 141 to 157 in 44b3924
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review @huikang ! I changed that and also added few changes to print some data about the node when printing the debug port to help identify which container we would like to debug in a multi Consul test.