Skip to content
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

Added name to service spec #58

Merged
merged 3 commits into from
Jun 15, 2022
Merged

Added name to service spec #58

merged 3 commits into from
Jun 15, 2022

Conversation

Krithika3
Copy link
Contributor

@Krithika3 Krithika3 commented Jun 15, 2022

I was working on a document for metrics and there was an ask to figure out how to scrape the metrics using the prometheus operator. As part of the exercise I had created some k8s serviceMonitors to monitor services(based on labels) and scrape the corresponding metric. While doing so I figured that the serviceMonitor expected a name field on the service spec Port object. Added the same.

This has been tested by running a sample pipeline in my local k3d cluster

Signed-off-by: Krithika Vijayakumar <krithika_vijayakumar@intuit.com>
@@ -81,6 +81,14 @@ func (v Vertex) GetServiceObjs() []*corev1.Service {
}

func (v Vertex) getServiceObj(name string, headless bool, port int) *corev1.Service {
var servicePortName string
Copy link
Member

Choose a reason for hiding this comment

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

Pass the port name as a parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Signed-off-by: Krithika Vijayakumar <krithika_vijayakumar@intuit.com>
@Krithika3 Krithika3 requested a review from whynowy June 15, 2022 20:24
Signed-off-by: Krithika Vijayakumar <krithika_vijayakumar@intuit.com>
Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

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

LGTM

@Krithika3 Krithika3 marked this pull request as ready for review June 15, 2022 21:14
@Krithika3 Krithika3 requested a review from vigith as a code owner June 15, 2022 21:14
@whynowy whynowy merged commit fd5b374 into numaproj:main Jun 15, 2022
whynowy pushed a commit that referenced this pull request Jul 8, 2022
* Added name to service spec

Signed-off-by: Krithika Vijayakumar <krithika_vijayakumar@intuit.com>

* Added vertexHTTPSPortName

Signed-off-by: Krithika Vijayakumar <krithika_vijayakumar@intuit.com>
yhl25 pushed a commit to yhl25/numaflow that referenced this pull request Sep 1, 2022
* Added name to service spec

Signed-off-by: Krithika Vijayakumar <krithika_vijayakumar@intuit.com>

* Added vertexHTTPSPortName

Signed-off-by: Krithika Vijayakumar <krithika_vijayakumar@intuit.com>

Signed-off-by: Yashash H L <yashash_hl@intuit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants