OCPBUGS-16728: Require Service Deletion for LB Type Updates #1142

gcs278 · 2024-09-10T21:17:48Z

Due to the AWS CCM leaking resources when the load balancer type is changed on a service, the cloud team is now blocking updates to the load balancer type. As a result, the Ingress Operator will require the service to be deleted and recreated when the Ingress Controller's load balancer type changes.

This change introduces a Progressing=True status message when the load balancer type is modified, instructing the user on how to effectuate the change. Additionally, the service.beta.kubernetes.io/aws-load-balancer-type annotation is now added to the managedAtServiceCreationLBServiceAnnotations map along with other annotations that require service deletion.

This commit does not alter the function of the LB type in the status, which has always (incorrectly) represented the desired state. However, with this change, a new scenario is introduced where the desired LB type may differ from the actual state. Consequently, the LB type status may no longer reliably reflect the actual state as it previously did, potentially surprising users who mistakenly assumed it always indicated the current state.

openshift-ci-robot · 2024-09-10T21:17:53Z

@gcs278: This pull request references Jira Issue OCPBUGS-16728, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.18.0) matches configured target version for branch (4.18.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Due to the AWS CCM leaking resources when the load balancer type is changed on a service, the cloud team is now blocking updates to the load balancer type. As a result, the Ingress Operator will require the service to be deleted and recreated when the Ingress Controller's load balancer type changes.

This change introduces a Progressing=True status message when the load balancer type is modified, instructing the user on how to effectuate the change. Additionally, the
service.beta.kubernetes.io/aws-load-balancer-type annotation is now unmanaged.

WIP:

I'd like to confirm interactions with also changing Subnets, or EIPAllocations, or Scope at the same time.

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.

lihongan · 2024-09-12T05:31:28Z

@gcs278 tested and found one issue:
After switching we got message

ingresscontroller "nlb" is progressing: IngressControllerProgressing: One or more status conditions indicate progressing: LoadBalancerProgressing=True (OperandsProgressing: One or more managed resources are progressing: The IngressController load balancer type was changed from "NLB" to "Classic".  
To effectuate this change, you must delete the service: `oc -n openshift-ingress delete svc/router-nlb`; the service load-balancer will then be deprovisioned and a new one created. This will most likely cause the new load-balancer to have a different host name and IP address and cause disruption. 
To return to the previous state, you can revert the change to the IngressController: `oc -n openshift-ingress-operator patch ingresscontrollers/nlb --type=merge --patch='{"spec":{"endpointPublishingStrategy":{"type":"LoadBalancerService","loadBalancer":{"providerParameters":{"type":"AWS","aws":{"type":"Classic"}}}}}}'`), DeploymentRollingOut=True (DeploymentRollingOut: Waiting for router deployment rollout to finish: 1 old replica(s) are pending termination......

The command to revert the change is wrong, it should be {"type":"NLB"} because the LB type was changed from "NLB" to "Classic".

lihongan · 2024-09-12T05:38:21Z

Another concern is the ingresscontroller status, should it always show the actual status? If yes, then the spec and status might be different before deleting the svc to effectuate the change.
And seeing same thing when changing the scope (Internal<>External)

gcs278 · 2024-09-12T14:26:23Z

@gcs278 tested and found one issue: After switching we got message

@lihongan Thanks, good catch! Fixed.

Another concern is the ingresscontroller status, should it always show the actual status? If yes, then the spec and status might be different before deleting the svc to effectuate the change. And seeing same thing when changing the scope (Internal<>External)

Yea it's a great observation, I just talked about this with @Miciah yesterday. This PR won't address having the LB Type in the status reflect the actual LB Type status. It's complicated, and will require much more refactoring than I'm willing to bite off with this PR. I think we should consider this a technical debt item that we should try to solve later.

The LB Type in the status will mostly be equivalent to the LB Type in the spec (for now).

gcs278 · 2024-09-12T21:20:53Z

I've taken the WIP off the PR. I'm pretty content with the approach taken from some ideas of @Miciah (i.e. two disjoint "managed" annotation lists). Some parts of the code actually were simplified and code was removed as a result.

I've also dropped the usage of "auto effectuation" or "manual effectuation" as I don't think this aptly conveys the idea that you need to delete/recreate the service (based on conversations with other team members). I went for managedAtServiceCreationLBServiceAnnotations to explicitly describe that the annotations are only managed when the service is created, implying the service will need to be recreated for updates to take affect. I'm open to ideas still though.

gcs278 · 2024-09-13T01:48:46Z

e2e-aws-operator unrelated failure:

=== RUN   TestAll/serial/TestUpdateDefaultIngressControllerSecret
    util_test.go:540: conflict when updating ingress controller openshift-ingress-operator/default: Operation cannot be fulfilled on ingresscontrollers.operator.openshift.io "default": the object has been modified; please apply your changes to the latest version and try again, retrying...

/test e2e-aws-operator

Install failure
/test e2e-aws-operator-techpreview

unrelated
/test e2e-aws-ovn-upgrade
/test e2e-gcp-ovn

lihongan · 2024-09-13T04:38:36Z

@gcs278 Another issue is observed. Because PROXY proxy is enabled on Classic but disabled on NLB, so before deleting the LB service to effectuate the change, I can see the router deployment is rolled out with ENV ROUTER_USE_PROXY_PROTOCOL updated and service annotation service.beta.kubernetes.io/aws-load-balancer-proxy-protocol is changed, that might be potential problem.

gcs278 · 2024-09-13T13:53:39Z

@gcs278 Another issue is observed. Because PROXY proxy is enabled on Classic but disabled on NLB, so before deleting the LB service to effectuate the change, I can see the router deployment is rolled out with ENV ROUTER_USE_PROXY_PROTOCOL updated and service annotation service.beta.kubernetes.io/aws-load-balancer-proxy-protocol is changed, that might be potential problem.

Ah good catch. I forgot that somethings rely on the status LB Type to be accurate. I need to think about how to fix this.
/wip

Miciah · 2024-10-02T15:01:34Z

/assign

alebedev87

Sorry for the long delay.

alebedev87 · 2024-11-27T14:54:15Z

pkg/operator/controller/ingress/load_balancer_service.go

+	managedLBServiceAnnotations = func() []string {
+		result := []string{


Should we use the generic sets.Set instead of the deprecated sets.Strings? It seems to me that the idea for using a set was to prevent accidental duplicates which are possible with a slice.

Yea that's a fair point. Updated to use sets.Set

alebedev87 · 2024-11-27T15:01:26Z

pkg/operator/controller/ingress/load_balancer_service.go

 		currentVal, have := current.Annotations[annotation]
 		expectedVal, want := expected.Annotations[annotation]
 		if (want && (!have || currentVal != expectedVal)) || (have && !want) {
-			changed = true
+			changedAnnotations = append(changedAnnotations, annotation)
 			break


Didn't you mean to collect all changed annotations? We still break once a change is found.

Whoops good catch. The changed list is just for logging at the moment, but still important.

alebedev87 · 2024-11-27T15:36:02Z

pkg/operator/controller/ingress/load_balancer_service.go

 func loadBalancerServiceIsProgressing(ic *operatorv1.IngressController, service *corev1.Service, platform *configv1.PlatformStatus, subnetsAWSEnabled bool, eipAllocationsAWSEnabled bool) error {
 	var errs []error
+	// Tricky: Scope in the status indicates the *desired* scope.


Will getting the desired state from ic.Spec be a step towards the right direction (reading the desired state from the spec and actual from the service)?

Are you asking hypothetically? Or are you suggesting to change how LB scope spec/status work?

Generally speaking, @Miciah and I have mostly agreed that we should push that new API fields in the direction of spec == desired state and status == actual state. We did EIPAllocations, Subnets, AllowedSourceRanges, and OpenStack Floating IP in this manner (status == actual state).

We aren't going to be able to change the behavior of existing spec/status fields, but we can at least implement new ones in this way. For scope, the spec won't indicate the full desire (user desire + defaulting), so we can't change this without changing the behavior.

So yea, I think it's the right direction.

For scope, the spec won't indicate the full desire (user desire + defaulting), so we can't change this without changing the behavior.

Right, I forgot about the defaulting. Then should the comment say something like:

Suggested change

// Tricky: Scope in the status indicates the *desired* scope.

// Tricky: Scope in the status indicates the *desired* scope with the defaulting taken into account.

?

Yea sure it's tricky language.

I'll clarify.

alebedev87 · 2024-11-27T17:03:05Z

pkg/operator/controller/ingress/load_balancer_service.go

 	if platform.Type == configv1.AWSPlatformType && subnetsAWSEnabled {
 		var (
+			// Tricky: Subnets in the status indicates the *actual* scope.


Similar question. Can we read subnets from the service annotation directly?

Subnets and EIPs status do reflect the actual state, but yes, we can also read it from the service annotation directly.

It doesn't really matter either way. I implemented this way because I liked the idea of putting the annotation into the IC status value, then using the status as a single point to access the actual value.

alebedev87 · 2024-11-27T17:35:49Z

pkg/operator/controller/ingress/status.go

+// clearIngressControllerInactiveAWSLBTypeParametersStatus clears AWS parameters for the inactive load balancer type,
+// unless there is a load balancer type change is pending. When a change is pending, setDefaultPublishingStrategy
+// sets the desired parameters for the new LB type. To avoid removing the desired state, we allow


The logic of setDefaultPublishingStategy puzzled me in other reviews too. Do I understand correctly that this function reverts the changes done by setDefaultPublishingStategy syncing the status with the actual
state of the service (good direction)?

Not for this PR but why can't we just stop updating the status in setDefaultPublishingStategy and let syncIngressControllerStatus take care of the status exclusively?

If the type of the LB is changed by the user:

we save the desired state in spec

update the status to instruct the user to recreate the service

keep the LB type in status the same as in the service's annotation

Once the service is manually recreated the reconciliation will be triggered and we will reset the status to the new type taken from the service annotation.

Am I missing some hidden value in what setDefaultPublishingStategy is doing w.r.t. to the status?

Right, I originally did this status NLB/CLB parameters clearing in setDefaultPublishingStategy, then I noticed during pending Scope changes that setDefaultPublishingStategy was only keeping the desired parameters and deleting the actual parameter's status (it doesn't know or care about pending changes, it just know and cares about desired state).

So, we can't just clear the NLB/CLB parameters based on desired state, we need to know the actual state (if a change is pending), so we don't prematurely clear status that represents the actual state.

Not for this PR but why can't we just stop updating the status in setDefaultPublishingStategy and let syncIngressControllerStatus take care of the status exclusively?

Most of the time yes. The only exception is maybe some defaulting logic, especially in setDefaultProviderParameters that may need to be added to or maintained. I wanted to put a comment in setDefaultPublishingStategy to prevent future developers from adding logic to it accidentally (like what happened with Floating IP). I might go ahead and do that.

If the type of the LB is changed by the user:
we save the desired state in spec
update the status to instruct the user to recreate the service
keep the LB type in status the same as in the service's annotation

Yes, in a perfect world, but to be clear, this PR doesn't keep the LB type in status the same as in the service's annotation, it actually immediately updates the LB type in the status to the desired state while it's pending. Hongan was asking the same question. API Team said that changing the behavior of a status field is incompatible, and we also use the LB Type status field for defaulting, which cause us to loose our defaulting. So this was a dead end.

Am I missing some hidden value in what setDefaultPublishingStategy is doing w.r.t. to the status?

Not really, but maybe some in memory defaulting to avoid nil pointer exceptions like I mentioned earlier.

gcs278

Thanks for the review. I still need to rebase due to the fact that #1147 introduced a spec field that effectively a managedAtServiceCreationLBServiceAnnotations, but not an annotation. So I will need to refactor.

gcs278 · 2024-11-28T01:28:49Z

pkg/operator/controller/ingress/load_balancer_service.go

+	managedLBServiceAnnotations = func() []string {
+		result := []string{


Yea that's a fair point. Updated to use sets.Set

gcs278 · 2024-11-28T01:31:07Z

pkg/operator/controller/ingress/load_balancer_service.go

 		currentVal, have := current.Annotations[annotation]
 		expectedVal, want := expected.Annotations[annotation]
 		if (want && (!have || currentVal != expectedVal)) || (have && !want) {
-			changed = true
+			changedAnnotations = append(changedAnnotations, annotation)
 			break


Whoops good catch. The changed list is just for logging at the moment, but still important.

gcs278 · 2024-11-28T01:42:41Z

pkg/operator/controller/ingress/load_balancer_service.go

 func loadBalancerServiceIsProgressing(ic *operatorv1.IngressController, service *corev1.Service, platform *configv1.PlatformStatus, subnetsAWSEnabled bool, eipAllocationsAWSEnabled bool) error {
 	var errs []error
+	// Tricky: Scope in the status indicates the *desired* scope.


Are you asking hypothetically? Or are you suggesting to change how LB scope spec/status work?

Generally speaking, @Miciah and I have mostly agreed that we should push that new API fields in the direction of spec == desired state and status == actual state. We did EIPAllocations, Subnets, AllowedSourceRanges, and OpenStack Floating IP in this manner (status == actual state).

We aren't going to be able to change the behavior of existing spec/status fields, but we can at least implement new ones in this way. For scope, the spec won't indicate the full desire (user desire + defaulting), so we can't change this without changing the behavior.

So yea, I think it's the right direction.

gcs278 · 2024-11-28T01:47:54Z

pkg/operator/controller/ingress/load_balancer_service.go

 	if platform.Type == configv1.AWSPlatformType && subnetsAWSEnabled {
 		var (
+			// Tricky: Subnets in the status indicates the *actual* scope.


Subnets and EIPs status do reflect the actual state, but yes, we can also read it from the service annotation directly.

It doesn't really matter either way. I implemented this way because I liked the idea of putting the annotation into the IC status value, then using the status as a single point to access the actual value.

gcs278 · 2024-11-28T02:08:14Z

pkg/operator/controller/ingress/status.go

+// clearIngressControllerInactiveAWSLBTypeParametersStatus clears AWS parameters for the inactive load balancer type,
+// unless there is a load balancer type change is pending. When a change is pending, setDefaultPublishingStrategy
+// sets the desired parameters for the new LB type. To avoid removing the desired state, we allow


Right, I originally did this status NLB/CLB parameters clearing in setDefaultPublishingStategy, then I noticed during pending Scope changes that setDefaultPublishingStategy was only keeping the desired parameters and deleting the actual parameter's status (it doesn't know or care about pending changes, it just know and cares about desired state).

So, we can't just clear the NLB/CLB parameters based on desired state, we need to know the actual state (if a change is pending), so we don't prematurely clear status that represents the actual state.

Not for this PR but why can't we just stop updating the status in setDefaultPublishingStategy and let syncIngressControllerStatus take care of the status exclusively?

Most of the time yes. The only exception is maybe some defaulting logic, especially in setDefaultProviderParameters that may need to be added to or maintained. I wanted to put a comment in setDefaultPublishingStategy to prevent future developers from adding logic to it accidentally (like what happened with Floating IP). I might go ahead and do that.

If the type of the LB is changed by the user:
we save the desired state in spec
update the status to instruct the user to recreate the service
keep the LB type in status the same as in the service's annotation

Yes, in a perfect world, but to be clear, this PR doesn't keep the LB type in status the same as in the service's annotation, it actually immediately updates the LB type in the status to the desired state while it's pending. Hongan was asking the same question. API Team said that changing the behavior of a status field is incompatible, and we also use the LB Type status field for defaulting, which cause us to loose our defaulting. So this was a dead end.

Am I missing some hidden value in what setDefaultPublishingStategy is doing w.r.t. to the status?

Not really, but maybe some in memory defaulting to avoid nil pointer exceptions like I mentioned earlier.

Some links in `load_balancer_service.go` were invalid due to K8S documentation URL changes.

alebedev87 · 2024-12-10T12:27:43Z

/lgtm
/approve
/hold

LGTM, holding for @Miciah to have a look.

openshift-ci · 2024-12-10T12:34:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alebedev87

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:

~~OWNERS~~ [alebedev87]

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

pkg/operator/controller/ingress/load_balancer_service_test.go

Refactor to introduce a new map, `managedAtServiceCreationLBServiceAnnotations`, which tracks annotations that are managed by the operator but are only applied to the load balancer at the time of service creation. These annotations are also responsible for triggering the `Progressing: True` status when modified. This addition not only improves documentation of these annotations, but also simplifies the code. The `loadBalancerServiceAnnotationsChanged` function has been adapted to replace existing logic in `shouldRecreateLoadBalancer`, allowing the removal of the `scopeEqual` function as well.

Due to the AWS CCM leaking resources when the load balancer type is changed on a service, the cloud team is now blocking updates to the load balancer type. As a result, the Ingress Operator will require the service to be deleted and recreated when the Ingress Controller's load balancer type changes. This change introduces a `Progressing: True` status message when the load balancer type is modified, instructing the user on how to effectuate the change. This commit does not alter the function of the LB type in the status, which has always (incorrectly) represented the desired state. However, with this change, a new scenario is introduced where the desired LB type may differ from the actual state. Consequently, the LB type status may no longer reliably reflect the actual state as it previously did, potentially surprising users who mistakenly assumed it always indicated the current state.

IsProxyProtocolNeeded must get the LB Type from the service, as the status now may not accurately reflect the actual LB Type during an update. However, we can't rely only on the service because it doesn’t exist during the initial bootstrap (and potentially when it gets deleted). In that case, it's safe to use the status to determine the anticipated LB Type. Without this commit results in a misalignment of the proxy protocol value between the load balancer and the router deployment when an LB type change is pending. This commit will be squashed into the previous one, as both are dependent on each other. I've kept them separate for easier review.

Previously, the fix for OCPBUGS-38217 cleared inactive LB parameters in the status in setDefaultPublishingStrategy. This was valid at the time, as the desired LB type would always become the actual LB type. However, with the introduction of pending LB type changes, setDefaultPublishingStrategy was now prematurely clearing parameters for the current (but soon-to-be inactive) LB type. This caused issues because certain LB parameters, such as classicLoadBalancer.subnets or networkLoadBalancer.eipAllocations, reflect the actual state. As a result, setDefaultPublishingStrategy was clearing them too early, without knowing a type change was pending. The solution is to move the clearing logic to syncIngressControllerStatus in status.go, where it can detect pending LB type changes and avoid clearing parameters for the still-active LB type. This means that during a pending type change, both networkLoadBalancer and classicLoadBalancer parameters are present. This commit will be squashed into the previous one, as both are dependent on each other. I've kept them separate for easier review.

gcs278 · 2024-12-10T14:59:12Z

@alebedev87 thanks for the review. Just added a unit test Test_clearIngressControllerInactiveAWSLBTypeParametersStatus and fixed your last comment.

alebedev87 · 2024-12-10T22:22:59Z

/lgtm

gcs278 · 2024-12-17T19:12:35Z

/retest

openshift-ci · 2024-12-17T22:52:01Z

@gcs278: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn-techpreview	`b27545b`	link	false	`/test e2e-aws-ovn-techpreview`
ci/prow/e2e-azure-ovn	`b27545b`	link	false	`/test e2e-azure-ovn`
ci/prow/okd-scos-e2e-aws-ovn	`b27545b`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/e2e-aws-ovn-serial	`b27545b`	link	true	`/test e2e-aws-ovn-serial`

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.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 10, 2024

openshift-ci-robot added jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Sep 10, 2024

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Sep 10, 2024

openshift-ci bot requested review from lihongan, candita and miheer September 10, 2024 21:18

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch from 65eea90 to 8a299a9 Compare September 11, 2024 22:25

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch from 8a299a9 to 26a1492 Compare September 12, 2024 14:21

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch 2 times, most recently from 9d3a208 to b02db5f Compare September 12, 2024 21:10

gcs278 changed the title ~~[WIP] OCPBUGS-16728: Require Service Deletion for LB Type Updates~~ OCPBUGS-16728: Require Service Deletion for LB Type Updates Sep 12, 2024

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 12, 2024

gcs278 changed the title ~~OCPBUGS-16728: Require Service Deletion for LB Type Updates~~ [WIP] OCPBUGS-16728: Require Service Deletion for LB Type Updates Sep 13, 2024

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 13, 2024

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch from b02db5f to 5e2d300 Compare September 18, 2024 23:25

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 18, 2024

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch from 5e2d300 to c6fd8b6 Compare September 19, 2024 01:27

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 19, 2024

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch from c6fd8b6 to 07b7cbb Compare September 27, 2024 17:47

openshift-ci bot assigned Miciah Oct 2, 2024

Miciah added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Nov 19, 2024

alebedev87 reviewed Nov 27, 2024

View reviewed changes

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch from 9722b17 to 39cf30c Compare November 28, 2024 02:24

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 28, 2024

gcs278 commented Nov 28, 2024

View reviewed changes

Update Broken Annotation Documentation Links

9b665ce

Some links in `load_balancer_service.go` were invalid due to K8S documentation URL changes.

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch from 39cf30c to cc22531 Compare December 9, 2024 21:26

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 9, 2024

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch 4 times, most recently from e424420 to 32fba0a Compare December 9, 2024 23:14

openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Dec 10, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 10, 2024

alebedev87 reviewed Dec 10, 2024

View reviewed changes

pkg/operator/controller/ingress/load_balancer_service_test.go Outdated Show resolved Hide resolved

gcs278 added 4 commits December 10, 2024 09:19

gcs278 force-pushed the OCPBUGS-16728-lb-type-change-effectuation branch from 32fba0a to b27545b Compare December 10, 2024 14:58

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2024

		managedLBServiceAnnotations = func() []string {
		result := []string{

	// Tricky: Scope in the status indicates the desired scope.
	// Tricky: Scope in the status indicates the desired scope with the defaulting taken into account.

OCPBUGS-16728: Require Service Deletion for LB Type Updates #1142

Are you sure you want to change the base?

OCPBUGS-16728: Require Service Deletion for LB Type Updates #1142

Conversation

gcs278 commented Sep 10, 2024 • edited Loading

openshift-ci-robot commented Sep 10, 2024

lihongan commented Sep 12, 2024

lihongan commented Sep 12, 2024

gcs278 commented Sep 12, 2024 • edited Loading

gcs278 commented Sep 12, 2024

gcs278 commented Sep 13, 2024

lihongan commented Sep 13, 2024

gcs278 commented Sep 13, 2024

Miciah commented Oct 2, 2024

alebedev87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcs278 Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcs278 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alebedev87 commented Dec 10, 2024

openshift-ci bot commented Dec 10, 2024

gcs278 commented Dec 10, 2024

alebedev87 commented Dec 10, 2024

gcs278 commented Dec 17, 2024

openshift-ci bot commented Dec 17, 2024

gcs278 commented Sep 10, 2024 •

edited

Loading

gcs278 commented Sep 12, 2024 •

edited

Loading

gcs278 Dec 9, 2024 •

edited

Loading