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

SECURESIGN-1250 | Targets are not coming up with monitoring enabled. #518

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

JasonPowr
Copy link
Contributor

Pr to fix the issue where service monitor targets are not coming up, caused by a change in the naming for the service ports.

@JasonPowr JasonPowr force-pushed the fix-service-monitors branch from db4bc8b to 80d5f9a Compare July 22, 2024 09:14
@JasonPowr JasonPowr requested a review from osmman July 22, 2024 09:15
@@ -44,7 +40,7 @@ func (i createServiceAction) Handle(ctx context.Context, instance *rhtasv1alpha1
)

labels := constants.LabelsFor(actions.LogSignerComponentName, actions.LogsignerDeploymentName, instance.Name)
logsignerService := k8sutils.CreateService(instance.Namespace, actions.LogsignerDeploymentName, actions.MonitoringPortName, actions.MonitoringPort, labels)
logsignerService := k8sutils.CreateService(instance.Namespace, actions.LogsignerDeploymentName, actions.MonitoringPortName, actions.MonitoringPort, actions.MonitoringPort, labels)
Copy link
Contributor

Choose a reason for hiding this comment

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

trillian-logsigner should provide 2 service ports. GRPC 8091 which is missing and 8090 for HTTP metrics

Copy link
Contributor

Choose a reason for hiding this comment

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

currently there is only 8090 provided

@osmman
Copy link
Contributor

osmman commented Jul 22, 2024

maybe we should rename 80-tcp to be consistent across services to http

@JasonPowr JasonPowr force-pushed the fix-service-monitors branch from 80d5f9a to 9c8df05 Compare July 22, 2024 13:36
@JasonPowr JasonPowr requested a review from osmman July 22, 2024 13:37
Copy link

openshift-ci bot commented Jul 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JasonPowr, osmman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@JasonPowr
Copy link
Contributor Author

/test tas-operator-e2e

@openshift-merge-bot openshift-merge-bot bot merged commit dd2828c into main Jul 22, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants