-
Notifications
You must be signed in to change notification settings - Fork 220
[release-4.12] OCPBUGS-12464: Target metrics port by name in internal service #910
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
[release-4.12] OCPBUGS-12464: Target metrics port by name in internal service #910
Conversation
Reformat ensureInternalIngressControllerService to follow the standard pattern for "ensure" functions. * pkg/operator/controller/ingress/controller.go (ensureIngressController): Expect a Boolean return value from ensureInternalIngressControllerService, indicating whether the service exists. * pkg/operator/controller/ingress/internal_service.go (ensureInternalIngressControllerService): Add a Boolean return value. Use a switch statement, and add a to-do comment for handling updates. (currentInternalIngressControllerService): Add a Boolean return value.
Add logic to ensureInternalIngressControllerService to update the service when an update is required. * pkg/operator/controller/ingress/internal_service.go (ensureInternalIngressControllerService): Use the new updateInternalService method to update the service as needed. (updateInternalService): Check whether the given ClusterIP service needs to be updated, and update it if so, using the new internalServiceChanged function. (managedInternalServiceAnnotations): New variable with the set of annotation keys for annotations that the operator manages for its internal router services. (internalServiceChanged): New function. Check whether the current internal service needs to be updated, and update it if it does, using the new managedInternalServiceAnnotations variable. * pkg/operator/controller/ingress/internal_service_test.go: New file. (Test_desiredInternalIngressControllerService): New test. Verify that desiredInternalIngressControllerService returns the expected service. (Test_internalServiceChanged): New test. Verify that internalServiceChanged correctly detects changes and performs updates.
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.
|
@openshift-cherrypick-robot: Jira Issue OCPBUGS-4573 has been cloned as Jira Issue OCPBUGS-12464. Will retitle bug to link to clone. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@openshift-cherrypick-robot: This pull request references Jira Issue OCPBUGS-12464, which is valid. The bug has been moved to the POST state. 6 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@openshift-cherrypick-robot: No Bugzilla bug is referenced in the title of this pull request. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
e2e-gcp-ovn-serial failed because the "events should not repeat pathologically" test failed because of missing node annotations, which is being tracked as OCPBUGS-10841. e2e-aws-ovn-single-node failed because of multiple test failures:
/test e2e-aws-ovn-single-node e2e-azure-ovn failed because of the same test failures as e2e-aws-ovn-single-node. e2e-aws-ovn failed because of the same test failures as e2e-aws-ovn-single-node. |
|
e2e-aws-ovn-single-node failed because the same four tests failed. I found that these tests are failing for many other PRs, so I filed OCPBUGS-12746 to track those failures. e2e-azure-ovn failed with the same four failures. e2e-gcp-ovn-serial failed because of OCPBUGS-10841. e2e-aws-ovn failed because AWS is full: /test e2e-aws-ovn This backport is a clean cherry-pick, and it has been approved by PM. The changes are fairly well isolated, include good test coverage, and do not appear to pose significant risk. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Miciah The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/label cherry-pick-approved |
|
e2e-aws-ovn-single-node failed and e2e-azure-ovn failed because of the same failures as before; waiting for OCPBUGS-12746 and OCPBUGS-12688 to be verified. e2e-gcp-ovn-serial failed because of OCPBUGS-10841 "CI fails on "events should not repeat pathologically" because of missing node annotations". I have updated the bug report to communicate that it is affecting 4.12 (the original bug report indicated that only 4.14 and 4.13 were affected). |
|
/retest |
|
@openshift-cherrypick-robot: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@openshift-cherrypick-robot: Jira Issue OCPBUGS-12464: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-12464 has been moved to the MODIFIED state. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Fix included in accepted release 4.12.0-0.nightly-2023-05-15-222942 |
This is an automated cherry-pick of #864
/assign Miciah