Skip to content

Commit

Permalink
service-internal.yaml: Target metrics port by name
Browse files Browse the repository at this point in the history
Use the "metrics" port name instead of port number 1936 for the router
internal service's metrics port's target.

Before this commit, the router's internal service's metrics port always
targeted port 1936 on the router pod.  However, the router pod's metrics
port can be customized, and so it is not necessarily port 1936.  As a
consequence, the service's metrics port didn't work when the router pod's
metrics port was customized.  The router pod's metrics port always has the
name "metrics", so this commit changes the service to reference the port by
name to avoid breaking when the port number changes.

This commit fixes OCPBUGS-4573.

https://issues.redhat.com/browse/OCPBUGS-4573

Follow-up to commit af653f9.

* assets/router/service-internal.yaml: Use the "metrics" port name instead
of port 1936 for the metrics port's target.
* pkg/manifests/bindata.go: Regenerate.
* pkg/operator/controller/ingress/internal_service_test.go
(Test_desiredInternalIngressControllerService): Expect the metrics port to
reference the "metrics" target port by name.
(Test_internalServiceChanged): Add a test case for changing the "metrics"
port's target port from an integer to a string.
  • Loading branch information
Miciah committed Jan 10, 2023
1 parent 44a60f5 commit 9c6e368
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 7 deletions.
2 changes: 1 addition & 1 deletion assets/router/service-internal.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ spec:
- name: metrics
port: 1936
protocol: TCP
targetPort: 1936
targetPort: metrics
8 changes: 4 additions & 4 deletions pkg/manifests/bindata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pkg/operator/controller/ingress/internal_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ const (
)

// ensureInternalRouterServiceForIngress ensures that an internal service exists
// for a given IngressController.
// for a given IngressController. Returns a Boolean indicating whether the
// service exists, the current service if it does exist, and an error value.
func (r *reconciler) ensureInternalIngressControllerService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) (bool, *corev1.Service, error) {
desired := desiredInternalIngressControllerService(ic, deploymentRef)
have, current, err := r.currentInternalIngressControllerService(ic)
Expand Down
15 changes: 14 additions & 1 deletion pkg/operator/controller/ingress/internal_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func Test_desiredInternalIngressControllerService(t *testing.T) {
Name: "metrics",
Protocol: "TCP",
Port: int32(1936),
TargetPort: intstr.FromInt(1936),
TargetPort: intstr.FromString("metrics"),
}}, svc.Spec.Ports)
}

Expand Down Expand Up @@ -116,6 +116,19 @@ func Test_internalServiceChanged(t *testing.T) {
},
expect: true,
},
{
description: "if .spec.ports changes by changing the metrics port's target port from an integer to a string",
mutate: func(svc *corev1.Service) {
for i := range svc.Spec.Ports {
if svc.Spec.Ports[i].Name == "metrics" {
svc.Spec.Ports[i].TargetPort = intstr.FromString("metrics")
return
}
}
panic("no metrics port found!")
},
expect: true,
},
{
description: "if .spec.selector changes",
mutate: func(svc *corev1.Service) {
Expand Down

0 comments on commit 9c6e368

Please sign in to comment.