-
Notifications
You must be signed in to change notification settings - Fork 100
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
Improve KnativeEventing finalization for TLS resources #1590
Improve KnativeEventing finalization for TLS resources #1590
Conversation
For optional resources like cert-manager's Certificates and Issuers, we don't want to fail finalization when such operators are not installed, so during finalization, we split the resources in - optional resources (TLS resources, etc) - resources (core k8s resources) Then, we delete `resources` first and after we delete optional resources while also ignoring errors returned when such operators are not instaled. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
2584610
to
6fd2b9c
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1590 +/- ##
=======================================
Coverage 79.40% 79.40%
=======================================
Files 41 41
Lines 1845 1845
=======================================
Hits 1465 1465
Misses 277 277
Partials 103 103 ☔ View full report in Codecov by Sentry. |
/cherry-pick release-1.11 |
@pierDipi: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you. 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 kubernetes/test-infra repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: creydr, pierDipi 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 |
@pierDipi: failed to push cherry-picked changes in GitHub: pushToNamedFork is not implemented in the v2 client 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 kubernetes/test-infra repository. |
/cherry-pick release-1.11 |
@pierDipi: failed to push cherry-picked changes in GitHub: pushToNamedFork is not implemented in the v2 client 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 kubernetes/test-infra repository. |
For optional resources like cert-manager's Certificates and Issuers, we don't want to fail finalization when such operators are not installed, so during finalization, we split the resources in - optional resources (TLS resources, etc) - resources (core k8s resources) Then, we delete `resources` first and after we delete optional resources while also ignoring errors returned when such operators are not instaled. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Done manually #1591, bot is broken kubernetes/test-infra#30937 |
…#1590) (#1591) * Improve KnativeEventing finalization for TLS resources (#1590) For optional resources like cert-manager's Certificates and Issuers, we don't want to fail finalization when such operators are not installed, so during finalization, we split the resources in - optional resources (TLS resources, etc) - resources (core k8s resources) Then, we delete `resources` first and after we delete optional resources while also ignoring errors returned when such operators are not instaled. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Improve manifest deletion error handling It might happen that not all resources were created successfully so we should not fail finalization when resources are not found, we can just ignore not found errors. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
For optional resources like cert-manager's Certificates and Issuers, we don't
want to fail finalization when such operators are not installed, so during
finalization, we split the resources in
Then, we delete
resources
first and after we delete optional resourceswhile also ignoring errors returned when such operators are not instaled.