OCPBUGS-38411: Not allow eipAllocations in CLB while changing the LBType of the Ingress Controller from NLB to Classic. #1130
Conversation
openshift-ci-robot
added
jira/severity-moderate
|
@miheer: This pull request references Jira Issue OCPBUGS-38411, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/invalid-bug
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
miheer
force-pushed
the
OCPBUGS-38411
branch
11 times, most recently
from
70c78c2
to
f3734ad
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
|
||
| if eipAllocationsAWSEnabled { | ||
| nlbParams := getAWSNetworkLoadBalancerParametersInSpec(ci) | ||
| if nlbParams != nil && awsEIPAllocationsExist(nlbParams.EIPAllocations) { | ||
| service.Annotations[awsEIPAllocationsAnnotation] = "" | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think this do what you want, the annotation isn't set by default anyways.
And this adds the annotation with a value of "", it's not clearing it. You should be able to just remove this.
There was a problem hiding this comment.
yes. i was already working on it. thanks for pointing out !
…ype of the Ingress Controller from NLB to Classic. After changing the LBType of the IngressController from `NLB` to `Classic`, the eipAllocations still were attached with controller and the LB service. Howver, eipAllocations are not supported in Classic. This commit fixes service annotation `service.beta.kubernetes.io/aws-load-balancer-eip-allocations` value to a blank value. The CCM then removes this annotation from the service. It also sets the IngressController in Progressing Status to True with a status message to delete the load balancer service so that a new lb service is recreated for Classic LB.
|
/assign @gcs278 |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
@miheer: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
DNS Propagation bug should be fixed, retesting gatewayapi: |
Miciah
added
the
priority/important-soon
After changing the LBType of the IngressController from
NLBtoClassic, the eipAllocations still were attached with controller and the LB service. Howver, eipAllocations are not supported in Classic.This commit fixes service annotation
service.beta.kubernetes.io/aws-load-balancer-eip-allocationsvalue to a blank value. The CCM then removes this annotation from the service. It also sets the IngressController in Progressing Status to True with a status message to delete the load balancer service so that a new lb service is recreated for Classic LB.