Remove obsolete finalizer from KnativeServing resources. #35
Conversation
The genreconciler machinery adds a finalizer automatically. There's no need to define our own and do our own management of it. We need to make sure to remove it from old resources though.
knative-prow-robot
added
the
size/M
There was a problem hiding this comment.
@markusthoemmes: 0 warnings.
In response to this:
Proposed Changes
The genreconciler machinery adds a finalizer automatically. There's no need to define our own and do our own management of it. We need to make sure to remove it from old resources though.
Release Note
NONE/assign @houshengbo @jcrossley3
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.
| if finalizer == finalizerName { | ||
| return nil | ||
| } | ||
| func (r *Reconciler) ensureFinalizerRemoval(_ context.Context, _ *mf.Manifest, instance *servingv1alpha1.KnativeServing) error { |
There was a problem hiding this comment.
If I understand correctly, the comment of this function should be changed into
ensureFinalizerRemovel ensures that Finalizers in the operator Serving CR is consistent with the ones generated by genreconciler.
The oldFinalizerName = "delete-knative-serving-manifest" is removed, if it exists in old operator serving CR.
In the delete function, we do not even check the Finalizers any more.
There was a problem hiding this comment.
Good catch, changed the comment.
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: houshengbo, markusthoemmes 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:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
Proposed Changes
The genreconciler machinery adds a finalizer automatically. There's no need to define our own and do our own management of it. We need to make sure to remove it from old resources though.
Release Note
/assign @houshengbo @jcrossley3