-
Notifications
You must be signed in to change notification settings - Fork 349
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 serviceType for the collector service #1286
add serviceType for the collector service #1286
Conversation
Signed-off-by: Simon Schneider <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1286 +/- ##
=======================================
Coverage 87.32% 87.33%
=======================================
Files 89 89
Lines 4947 4957 +10
=======================================
+ Hits 4320 4329 +9
- Misses 464 465 +1
Partials 163 163
Continue to review full report at Codecov.
|
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.
Looks good! Could we have an example under /examples
, showing how to use it? People ask about this quite often. It would also be good to have this new example as part of the e2e tests, so that we can document whether this works in non-vanilla Kubernetes (OpenShift, for instance).
Signed-off-by: Simon Schneider <[email protected]>
7583918
to
0867f39
Compare
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.
LGTM. @kevinearls, would you please check whether the example test looks sane to you?
By the way: I just confirmed that the new test is being executed and is passing:
|
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.
The test changes LGTM.
Is there anything missing for this PR? |
No, I was expecting mergify to merge it, but for some reason, it didn't (codecov checks missing?). I merged this manually. |
Adds serviceType option for the collector service.
Closes: #1263
Signed-off-by: Simon Schneider [email protected]