-
Notifications
You must be signed in to change notification settings - Fork 325
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 probes now that expose paths are supported #3096
Conversation
@@ -35,6 +35,7 @@ func TestMeshInject_MultiportService(t *testing.T) { | |||
|
|||
helmValues := map[string]string{ | |||
"global.experiments[0]": "resource-apis", | |||
"global.image": "ndhanushkodi/consul-dev:expose2", |
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 can remove this once the change is in the consul image being used here
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.
Just to save you the time checking: hashicorppreview/consul:1.17-dev
and hashicorppreview/consul:1.17.0
were published this AM, so I think you're good!
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.
Probably a little sus we don't need to make any test changes but the change looks good for now.
@DanStough oh yeah, so before making fixes in consul, it the multiport pod wouldn't come up with the probes, so I saw it crashloop, and the fact that it came up and the rest of the test could run means probes are working |
I was thinking of the webhook unit test, but I don't think it's a big deal. |
@DanStough ahhh got it |
7a279d9
to
5fa62b8
Compare
Co-authored-by: Nitya Dhanushkodi <[email protected]>
Changes proposed in this PR:
How I've tested this PR:
How I expect reviewers to test this PR:
Checklist: