OCPBUGS-33958: secret re-creation scenario for externalCertificate with active informer #614

chiragkyal · 2024-07-15T07:39:23Z

This PR:

Handles secret re-creation scenarios for externalCertificate where a previously deleted secret is added again.
Introduces a deletedSecrets map in RouteSecretManager to track routes for which the associated secret was deleted. This helps differentiate between new secret creation and recreation of a previously deleted secret.
Refactors secret handling logic in generateSecretHandler to address these changes.
- AddFunc handles the secret recreations. Upon detecting a recreated secret, it revalidates the route, updates its TLS configuration, and triggers a Modified event for the next plugins.
- DeleteFunc does not call UnregisterRoute(), instead tracks deleted secrets.
Improves the logic for handling Modified events for routes with externalCertificate to avoid blindly unregistering and re-registrations with the secret manager.
- This change is also required to avoid always calling UnregisterRoute(), which leads to the stopping of the informer.
- Depends on OCPBUGS-33958: retrieve registered secret name from secretManager library-go#1740
Makes stopping of the informer self-contained to the RouteSecretManager plugin.
- RemoveRoute (templateRouter) should not call UnregisterRoute() to avoid removal of informer when RouteSecretManager calls the next plugin chain with watch.Deleted event.
- Informer will be stopped only during the following cases
  - Route is deleted
  - Route is updated and externalCertificate field is removed
  - Route is updated and externalCertificate field is changed to a new secret name. [Old secret name is cached inside SecretManager's registeredHandlers map.
    - OCPBUGS-33958: retrieve registered secret name from secretManager library-go#1740
Adds unit tests to cover the new logic and scenarios.

New Helper Functions:
validate() to encapsulate the externalCertificate validation logic.
populateRouteTLSFromSecret() to update a route's in-memory TLS configuration from the referenced secret.
unregister() to remove the route's registration with the secret manager and clean up references in deletedSecrets.

Part of : https://issues.redhat.com//browse/OCPBUGS-33958

openshift-ci · 2024-07-15T07:39:27Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci-robot · 2024-07-15T07:39:28Z

@chiragkyal: This pull request references Jira Issue OCPBUGS-33958, which is invalid:

expected the bug to target the "4.17.0" version, but no target version was set

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

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.

chiragkyal · 2024-07-25T09:09:01Z

/jira refresh

openshift-ci-robot · 2024-07-25T09:09:06Z

@chiragkyal: This pull request references Jira Issue OCPBUGS-33958, which is valid.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @lihongan

In response to this:

/jira refresh

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 · 2024-08-28T09:03:23Z

@chiragkyal: This pull request references Jira Issue OCPBUGS-33958, which is invalid:

expected the bug to target either version "4.18." or "openshift-4.18.", but it targets "4.17.0" instead

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

In response to this:

This PR:

Handles secret re-creation scenarios for externalCertificate where a previously deleted secret is added again.

Introduces a deletedSecrets map in RouteSecretManager to track routes for which the associated secret was deleted. This helps differentiate between new secret creation and recreation of a previously deleted secret.

Refactors secret handling logic in generateSecretHandler to address these changes.

AddFunc handles the secret recreations. Upon detecting a recreated secret, it revalidates the route, updates its TLS configuration, and triggers a Modified event for the next plugins.

DeleteFunc does not call UnregisterRoute(), instead tracks deleted secrets.

Improves the logic for handling Modified events for routes with externalCertificate to avoid blindly unregistering and re-registrations with the secret manager.

This change is also required to avoid always calling UnregisterRoute(), which leads to the stopping of the informer.

Depends on OCPBUGS-33958: retrieve registered secret name from secretManager library-go#1740

Makes stopping of the informer self-contained to the RouteSecretManager plugin.

RemoveRoute (templateRouter) should not call UnregisterRoute() to avoid removal of informer when RouteSecretManager calls the next plugin chain with watch.Deleted event.

Informer will be stopped only during the following cases

Route is deleted

Route is updated and externalCertificate field is removed

Route is updated and externalCertificate field is changed to a new secret name. [Old secret name is cached inside SecretManager's registeredHandlers map.

OCPBUGS-33958: retrieve registered secret name from secretManager library-go#1740

Adds unit tests to cover the new logic and scenarios.

New Helper Functions:

validate() to encapsulate the externalCertificate validation logic.

populateRouteTLSFromSecret() to update a route's in-memory TLS configuration from the referenced secret.

unregister() to remove the route's registration with the secret manager and clean up references in deletedSecrets.

Part of : https://issues.redhat.com//browse/OCPBUGS-33958

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 · 2024-11-25T19:59:04Z

@chiragkyal: This pull request references Jira Issue OCPBUGS-33958, which is invalid:

expected the bug to target either version "4.19." or "openshift-4.19.", but it targets "4.18.0" instead

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

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

In response to this:

This PR:

Handles secret re-creation scenarios for externalCertificate where a previously deleted secret is added again.

Introduces a deletedSecrets map in RouteSecretManager to track routes for which the associated secret was deleted. This helps differentiate between new secret creation and recreation of a previously deleted secret.

Refactors secret handling logic in generateSecretHandler to address these changes.

AddFunc handles the secret recreations. Upon detecting a recreated secret, it revalidates the route, updates its TLS configuration, and triggers a Modified event for the next plugins.

DeleteFunc does not call UnregisterRoute(), instead tracks deleted secrets.

Improves the logic for handling Modified events for routes with externalCertificate to avoid blindly unregistering and re-registrations with the secret manager.

This change is also required to avoid always calling UnregisterRoute(), which leads to the stopping of the informer.

Depends on OCPBUGS-33958: retrieve registered secret name from secretManager library-go#1740

Makes stopping of the informer self-contained to the RouteSecretManager plugin.

RemoveRoute (templateRouter) should not call UnregisterRoute() to avoid removal of informer when RouteSecretManager calls the next plugin chain with watch.Deleted event.

Informer will be stopped only during the following cases

Route is deleted

Route is updated and externalCertificate field is removed

Route is updated and externalCertificate field is changed to a new secret name. [Old secret name is cached inside SecretManager's registeredHandlers map.

OCPBUGS-33958: retrieve registered secret name from secretManager library-go#1740

Adds unit tests to cover the new logic and scenarios.

New Helper Functions:

validate() to encapsulate the externalCertificate validation logic.

populateRouteTLSFromSecret() to update a route's in-memory TLS configuration from the referenced secret.

unregister() to remove the route's registration with the secret manager and clean up references in deletedSecrets.

Part of : https://issues.redhat.com//browse/OCPBUGS-33958

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.

chiragkyal · 2024-11-25T20:01:06Z

/jira refresh

openshift-ci-robot · 2024-11-25T20:01:13Z

@chiragkyal: This pull request references Jira Issue OCPBUGS-33958, which is valid.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @lihongan

In response to this:

/jira refresh

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.

Signed-off-by: chiragkyal <[email protected]>

alebedev87 · 2024-11-29T17:28:05Z

I played with the fix on a live cluster:

$ oc version
Server Version: 4.18.0-0.test-2024-11-29-144149-ci-ln-42q6vwk-latest

Two things I noticed about the status of the route:

After I recreated the secret for the second time the route's status got stuck with ExternalCertificateSecretRecreated reason not progressing to Admitted=true. The third recreation did the right thing: moved from ExternalCertificateSecretRecreated to Admitted=true. Any subsequent recreation didn't fall into sticking on ExternalCertificateSecretRecreated.
When using 2 routes (with the same external certificate) the statuses differ after the secret deletion. The route which already experienced the recreation and the route which just got its first one: ExternalCertificateSecretDeleted (secret "selfsigned" deleted for route "test/httpd-24") vs ExternalCertificateValidationFailed ('spec.tls.externalCertificate: Not found: "secrets \"selfsigned\" not). Also after the secret is recreated the status differ too: Admitted=true, ExternalCertificateValidationFailed (spec.tls.externalCertificate: Not found: "secrets \"selfsigned\).

I think that we need to test again those cases and try to find out whether there is any race or a glitch.

chiragkyal · 2024-12-09T15:44:28Z

Thanks for doing the test. I also tried on a live cluster.

I think there is a race condition happening while updating the status of the route. The secret handler status update is racing with the status update of the plugin re-trigger.

The top level plugin has a lock :

router/pkg/router/controller/router_controller.go

Lines 210 to 217 in be831fa

    
           func (c *RouterController) HandleRoute(eventType watch.EventType, obj interface{}) { 
        
           	route := obj.(*routev1.Route) 
        
           	c.lock.Lock() 
        
           	defer c.lock.Unlock() 
        
           	c.processRoute(eventType, route) 
        
           	c.Commit() 
        
           }

which means all routes are getting processed serially, unlike secret handler.

As a solution, I think the StatusAdmitter plugin can have its own Mutex to lock the methods like RecordRouteUpdate, RecordRouteRejection, HandleRoute, etc., so that we can avoid race conditions.

Signed-off-by: chiragkyal <[email protected]>

chiragkyal · 2024-12-17T12:43:33Z

@alebedev87 @Miciah As per our discussion over sync call, I've updated the contention tracker to ignore the status update done by the secret manager. Please take a final look at the PR, and let's try to get this merged. Thanks!

chiragkyal · 2024-12-18T19:42:54Z

I did some testing on a real cluster with multiple routes using the same secret, and after the latest changes, the transition of the status were as expected and consistent through out the routes.

$ oc get route -n sandbox -w
NAME        HOST/PORT                                                                 PATH   SERVICES          PORT    TERMINATION   WILDCARD
example     example-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com            hello-openshift   <all>   edge          None
example1    example1-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example10   example10-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com          hello-openshift   <all>   edge          None
example11   example11-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com          hello-openshift   <all>   edge          None
example2    example2-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example3    example3-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example4    example4-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example5    example5-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example6    example6-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example7    example7-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example8    example8-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None


// Secret Deleted


example5    ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example2    ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example6    ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example7    ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example3    ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example8    ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example     ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example11   ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example1    ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example10   ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example4    ExternalCertificateSecretDeleted                                                 hello-openshift   <all>   edge          None
example5    ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example2    ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example6    ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example7    ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example3    ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example8    ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example     ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example11   ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example1    ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example10   ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None
example4    ExternalCertificateValidationFailed                                              hello-openshift   <all>   edge          None

// Secret Re-Created

example5    ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example10   ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example11   ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example     ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example4    ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example1    ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example7    ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example2    ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example8    ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example6    ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example3    ExternalCertificateSecretRecreated                                               hello-openshift   <all>   edge          None
example5    example5-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example10   example10-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com          hello-openshift   <all>   edge          None
example11   example11-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com          hello-openshift   <all>   edge          None
example     example-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com            hello-openshift   <all>   edge          None
example4    example4-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example1    example1-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example7    example7-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example2    example2-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example8    example8-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example6    example6-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None
example3    example3-sandbox.apps.ckyal-20241216-486d8f62.devcluster.openshift.com           hello-openshift   <all>   edge          None

alebedev87 · 2024-12-19T12:55:39Z

/approve

Thanks for the testing, we should be good now.

openshift-ci · 2024-12-19T12:56:22Z

[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

chiragkyal · 2024-12-19T12:56:31Z

tide/merge-method-squash

chiragkyal · 2024-12-19T12:57:52Z

/label tide/merge-method-squash

pkg/router/controller/contention.go

Miciah · 2024-12-19T13:55:31Z

pkg/router/controller/contention.go

+	if ignoreIngressConditionReason.Has(a.Reason) || ignoreIngressConditionReason.Has(b.Reason) {
+		return true
+	}


We discussed today that this condition may be broader than necessary, but we couldn't think of a realistic scenario in which it would cause problems. However, we may need to revisit this condition if we ever observe any strange behavior with respect to status updates for routes with external certificates during, say, a rolling update of a router deployment.

pkg/router/controller/route_secret_manager.go

Miciah · 2024-12-19T14:02:50Z

pkg/router/controller/route_secret_manager.go

+//     - If the external certificate has changed, it unregisters the old one and registers the new one.
+//     - If the external certificate has not changed, it revalidates and updates the in-memory TLS certificate and key.


The comment is a little unclear. Presumably by "the external certificate has changed" or "the external certificate has not changed", you mean that the external certificate secret reference has or has not changed. That is, if the secret reference changes, you need to track the new referent, but if the reference is the same, you only need to check whether the referent secret's contents have changed.

you mean that the external certificate secret reference has or has not changed

Yes exactly, if the secret reference is changed to a new secret, we will track the new secret. And if the secret name is unchanged, we will check the contents of the secret.

Miciah · 2024-12-19T14:42:13Z

pkg/router/controller/route_secret_manager.go

+			// Update the route status to notify plugins, including this plugin, for re-evaluation.
+			// - If the route is admitted (Admitted=True), record an update event.
+			// - If the route is not admitted, record a rejection event (keep it rejected).
+			if isRouteAdmittedTrue(route.DeepCopy()) {
+				p.recorder.RecordRouteUpdate(route, ExtCrtStatusReasonSecretUpdated, msg)


isRouteAdmittedTrue checks whether any router has admitted the route:

router/pkg/router/controller/route_secret_manager.go

Lines 396 to 407 in 3966d00

// isRouteAdmittedTrue returns true if the given route has been admitted

// by any of its ingress controllers, otherwise false.

func isRouteAdmittedTrue(route *routev1.Route) bool {

for _, ingress := range route.Status.Ingress {

for _, condition := range ingress.Conditions {

if condition.Type == routev1.RouteAdmitted && condition.Status == kapi.ConditionTrue {

return true

}

}

}

return false

}

Suppose that the current router has previously rejected the route, but another router admitted it. Then will this logic cause the current router to admit the route erroneously?

The route status could have a stale status entry from a router that is no longer running, or from a router that is using an old image, or from a router that is configured differently (for example, it might allow wildcards while the current one does not). Why wouldn't you check only whether this router has admitted the route?

Why wouldn't you check only whether this router has admitted the route?

Thanks for the suggestion. Yeah, it makes sense to me. I have updated the logic in 62daef4 commit to pass the routerName to RouteSecretManager and check the status change for only this specific router.

Miciah · 2024-12-19T14:49:12Z

pkg/router/controller/route_secret_manager.go

+//   - Handles secret recreation: If a secret is recreated (created after being
+//     deleted), it fetches the latest route object associated with it using the
+//     RouteLister and updates the route's status to trigger a full
+//     re-evaluation of the route by the entire plugin chain.


I think there is a gap here. Consider the following scenario:

Start with a router deployment.

Create a route with an external certificate reference to secret.

Delete the secret.

Restart the router deployment's pods (for example, for a rolling update).

Recreate the secret.

The new router pods will reject the route at step 4 and will miss the secret recreate event at step 5, right?

It might not be feasible to close this gap, but I want to raise this concern to make sure I understand correctly and to note that it is a potential issue.

I think this is likely possible because the route pod is managing the lifecycle of the secret manager and the secret informers. Restarting the router deployment's pods will kill the previously stored watch, and new pods will not register the new watch because the secret won't be there. So, yeah, in this scenario, we will miss the secret recreate event.

It might not be feasible to close this gap, but I want to raise this concern to make sure I understand correctly and to note that it is a potential issue.

I agree on this, given the secret manager's dependency on the router pods.

I can see something related to this in the EP as well.

The router will bootstrap the secret monitor to ensure it can keep the certificate up-to-date. This means the router pod will maintain active watches for every secret that is referenced by a route.

xRef: https://github.com/openshift/enhancements/blob/master/enhancements/ingress/route-secret-injection-for-external-certificate-management.md#implementation-detailsnotesconstraints-optional

Signed-off-by: chiragkyal <[email protected]>

openshift-ci · 2024-12-19T20:12:08Z

@chiragkyal: The following test 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/okd-scos-e2e-aws-ovn	`62daef4`	link	false	`/test okd-scos-e2e-aws-ovn`

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 Jul 15, 2024

openshift-ci-robot added the jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. label Jul 15, 2024

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 15, 2024

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

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

chiragkyal force-pushed the active-informer branch 6 times, most recently from 41a1750 to f51a3f2 Compare July 23, 2024 07:48

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jul 25, 2024

openshift-ci bot requested a review from lihongan July 25, 2024 09:09

chiragkyal mentioned this pull request Jul 31, 2024

OCPBUGS-33958: retrieve registered secret name from secretManager openshift/library-go#1740

Merged

chiragkyal force-pushed the active-informer branch 4 times, most recently from 70121a3 to f655bd2 Compare August 28, 2024 07:48

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

chiragkyal marked this pull request as ready for review August 28, 2024 07:50

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 Aug 28, 2024

openshift-ci bot requested review from frobware and Miciah August 28, 2024 07:50

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

chiragkyal reopened this Nov 25, 2024

openshift-ci-robot added jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. and removed jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Nov 25, 2024

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Nov 25, 2024

Address review comments

265240f

Signed-off-by: chiragkyal <[email protected]>

chiragkyal added 2 commits December 9, 2024 21:20

StatusAdmitter will have its own lock

466a909

Signed-off-by: chiragkyal <[email protected]>

ignore contention for secret events

3966d00

Signed-off-by: chiragkyal <[email protected]>

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

openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 19, 2024

Miciah reviewed Dec 19, 2024

View reviewed changes

pkg/router/controller/contention.go Outdated Show resolved Hide resolved

Miciah reviewed Dec 19, 2024

View reviewed changes

pkg/router/controller/route_secret_manager.go Outdated Show resolved Hide resolved

Miciah reviewed Dec 19, 2024

View reviewed changes

pkg/router/controller/route_secret_manager.go Outdated Show resolved Hide resolved

Miciah reviewed Dec 19, 2024

View reviewed changes

isRouteAdmittedTrue should only care about the current router

62daef4

Signed-off-by: chiragkyal <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-33958: secret re-creation scenario for externalCertificate with active informer #614

OCPBUGS-33958: secret re-creation scenario for externalCertificate with active informer #614

chiragkyal commented Jul 15, 2024 •

edited

Loading

openshift-ci bot commented Jul 15, 2024

openshift-ci-robot commented Jul 15, 2024

chiragkyal commented Jul 25, 2024

openshift-ci-robot commented Jul 25, 2024

openshift-ci-robot commented Aug 28, 2024

openshift-ci-robot commented Nov 25, 2024

chiragkyal commented Nov 25, 2024

openshift-ci-robot commented Nov 25, 2024

alebedev87 commented Nov 29, 2024 •

edited

Loading

chiragkyal commented Dec 9, 2024

chiragkyal commented Dec 17, 2024

chiragkyal commented Dec 18, 2024

alebedev87 commented Dec 19, 2024

openshift-ci bot commented Dec 19, 2024

chiragkyal commented Dec 19, 2024

chiragkyal commented Dec 19, 2024

Miciah Dec 19, 2024

Miciah Dec 19, 2024

chiragkyal Dec 19, 2024

Miciah Dec 19, 2024

chiragkyal Dec 19, 2024

Miciah Dec 19, 2024

chiragkyal Dec 19, 2024

chiragkyal Dec 19, 2024

openshift-ci bot commented Dec 19, 2024

		// - If the external certificate has changed, it unregisters the old one and registers the new one.
		// - If the external certificate has not changed, it revalidates and updates the in-memory TLS certificate and key.

	// isRouteAdmittedTrue returns true if the given route has been admitted
	// by any of its ingress controllers, otherwise false.
	func isRouteAdmittedTrue(route *routev1.Route) bool {
	for _, ingress := range route.Status.Ingress {
	for _, condition := range ingress.Conditions {
	if condition.Type == routev1.RouteAdmitted && condition.Status == kapi.ConditionTrue {
	return true
	}
	}
	}
	return false
	}

OCPBUGS-33958: secret re-creation scenario for externalCertificate with active informer #614

Are you sure you want to change the base?

OCPBUGS-33958: secret re-creation scenario for externalCertificate with active informer #614

Conversation

chiragkyal commented Jul 15, 2024 • edited Loading

openshift-ci bot commented Jul 15, 2024

openshift-ci-robot commented Jul 15, 2024

chiragkyal commented Jul 25, 2024

openshift-ci-robot commented Jul 25, 2024

openshift-ci-robot commented Aug 28, 2024

openshift-ci-robot commented Nov 25, 2024

chiragkyal commented Nov 25, 2024

openshift-ci-robot commented Nov 25, 2024

alebedev87 commented Nov 29, 2024 • edited Loading

chiragkyal commented Dec 9, 2024

chiragkyal commented Dec 17, 2024

chiragkyal commented Dec 18, 2024

alebedev87 commented Dec 19, 2024

openshift-ci bot commented Dec 19, 2024

chiragkyal commented Dec 19, 2024

chiragkyal commented Dec 19, 2024

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

openshift-ci bot commented Dec 19, 2024

chiragkyal commented Jul 15, 2024 •

edited

Loading

alebedev87 commented Nov 29, 2024 •

edited

Loading