-
Notifications
You must be signed in to change notification settings - Fork 220
[release-4.6] Bug 1904594: Assume ingresscontroller is external absent status #503
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.6] Bug 1904594: Assume ingresscontroller is external absent status #503
Conversation
If an ingresscontroller's status.endpointPublishingStrategy.type field is "LoadBalancerService" but status.endpointPublishingStrategy.loadBalancer is nil, assume the ingresscontroller should have an external load balancer. Commit 8cd622f added support for internal scope for load balancers. In doing so, it changed the operator logic to initialize status.endpointPublishingStrategy.loadBalancer to indicate the scope when an ingresscontroller was created, using the corresponding fields in the ingresscontroller's spec to determine the scope. The same commit also changed operator logic to assume that the ingresscontroller has internal scope if status.endpointPublishingStrategy.loadBalancer is nil. However, if an ingresscontroller already exists, the operator does not update status.endpointPublishingStrategy.loadBalancer. This commit was made in OpenShift 4.2, which means that if a cluster was upgraded from 4.1, any ingresscontrollers that were created prior to the upgrade would not have status.endpointPublishingStrategy.loadBalancer set. Subsequently, commit 982682f made the ingresscontroller's scope mutable, meaning if an ingresscontroller has an external load balancer and status.endpointPublishingStrategy.loadBalancer indicates that the ingresscontroller should have an internal load balancer, then the operator changes the load balancer from external to internal. In combination, those two commits cause the operator to change an ingresscontroller's load balancer from external to internal if the cluster has been upgraded from 4.1 → 4.2 → ... → 4.6 and the ingresscontroller was originally created on OpenShift 4.1. This commit rectifies the situation by amending the logic that was added in commit 8cd622f to assume that a nil value for status.endpointPublishingStrategy.loadBalancer means that the load balancer should be external. This assumption is valid because status.endpointPublishingStrategy.loadBalancer is nil exactly when the ingresscontroller was created on an OpenShift 4.1 cluster, and external scope was the only supported option on OpenShift 4.1. * pkg/operator/controller/ingress/load_balancer_service.go (desiredLoadBalancerService): Assume nil status.endpointPublishingStrategy.loadBalancer means external scope.
|
@openshift-cherrypick-robot: Bugzilla bug 1904582 has been cloned as Bugzilla bug 1904594. Retitling PR to link against new bug. 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 Bugzilla bug 1904594, which is invalid:
Comment 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Miciah, openshift-cherrypick-robot 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 |
|
/bugzilla refresh Recalculating validity in case the underlying Bugzilla bug has changed. |
|
@openshift-bot: This pull request references Bugzilla bug 1904594, which is invalid:
Comment 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. |
|
/bugzilla refresh Recalculating validity in case the underlying Bugzilla bug has changed. |
|
@openshift-bot: This pull request references Bugzilla bug 1904594, which is invalid:
Comment 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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/bugzilla refresh Recalculating validity in case the underlying Bugzilla bug has changed. |
|
@openshift-bot: This pull request references Bugzilla bug 1904594, which is invalid:
Comment 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. |
|
Because we agree that we won't ship another 4.6.z without this and that forcing testing happen in 4.7 will delay that I've overridden bugzilla/valid-bug and applied cherry-pick-approved. |
|
/retest |
/test e2e-aws-operator |
|
/retest |
|
/override ci/prow/e2e-operator |
|
@sdodson: /override requires a failed status context to operate on.
Only the following contexts were expected:
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. |
|
/override ci/prow/e2e-aws-operator |
|
@sdodson: Overrode contexts on behalf of sdodson: ci/prow/e2e-aws-operator 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. |
|
/override ci/prow/e2e-aws |
|
@sdodson: Overrode contexts on behalf of sdodson: ci/prow/e2e-aws 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: All pull requests linked via external trackers have merged: Bugzilla bug 1904594 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. |
This is an automated cherry-pick of #502
/assign Miciah