Skip to content

Conversation

@openshift-cherrypick-robot

This is an automated cherry-pick of #320

/assign danehans

@openshift-ci-robot
Copy link
Contributor

@openshift-cherrypick-robot: This pull request references Bugzilla bug 1766141, which is invalid:

  • expected the bug to target the "4.2.z" release, but it targets "4.3.0" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, ON_DEV, POST, POST, but it is VERIFIED instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

[release-4.2] Bug 1766141: Ensures LB service finalizer is removed

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-ci-robot openshift-ci-robot added bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 7, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: openshift-cherrypick-robot
To complete the pull request process, please assign miciah
You can assign the PR to them by writing /assign @miciah in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

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

@danehans
Copy link
Contributor

danehans commented Nov 7, 2019

/retitle [release-4.2] Bug 1769882: Ensures LB service finalizer is removed

@openshift-ci-robot openshift-ci-robot changed the title [release-4.2] Bug 1766141: Ensures LB service finalizer is removed [release-4.2] Bug 1769882: Ensures LB service finalizer is removed Nov 7, 2019
@openshift-ci-robot
Copy link
Contributor

@openshift-cherrypick-robot: This pull request references Bugzilla bug 1769882, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

[release-4.2] Bug 1769882: Ensures LB service finalizer is removed

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-ci-robot openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Nov 7, 2019
// lbServiceCleanUpFinalizer is a finalizer attached to any service that has type=LoadBalancer.
// Upon deletion of the service, the actual deletion of the resource will be blocked until
// this finalizer is removed.
lbServiceCleanUpFinalizer = "service.kubernetes.io/load-balancer-cleanup"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot use this in OpenShift 4.2 or earlier because the finalizer is new in Kubernetes 1.16:

Specifically, if a Service has type LoadBalancer, the service controller will attach a finalizer named service.kubernetes.io/load-balancer-cleanup. The finalizer will only be removed after the load balancer resource is cleaned up. This prevents dangling load balancer resources even in corner cases such as the service controller crashing.

This feature is beta and enabled by default since Kubernetes v1.16. You can also enable it in v1.15 (alpha) via the feature gate ServiceLoadBalancerFinalizer.

https://kubernetes.io/docs/tasks/access-application-cluster/create-external-load-balancer/#garbage-collecting-load-balancers

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we refactor master to use existence checks before back porting?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we refactor master to use existence checks before back porting?

@danehans although I phrased this as a question, I guess it's more of a statement that the refactor is necessary because the information to support the current approach simply does not exist in 4.1 and 4.2 clusters

@Miciah
Copy link
Contributor

Miciah commented Dec 16, 2019

The backport needs to include both #320 and #330. You will need to open a new PR that incorporates the commits from both of those PRs.

@ironcladlou
Copy link
Contributor

This can't be automatically backported.

/close

@openshift-ci-robot
Copy link
Contributor

@ironcladlou: Closed this PR.

Details

In response to this:

This can't be automatically backported.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants