Skip to content
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

Only roll pods once for ClientsCa cert renewal #10814

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

katheris
Copy link
Contributor

@katheris katheris commented Nov 7, 2024

Type of change

Select the type of your PR

  • Bugfix

Description

Update CaReconciler to only roll pods when cluster CA key is replaced, not when clients CA key is replaced.

Currently we do two rolling updates for the key replacement, once in CaReconciler, and once in component reconcilers, e.g. KafkaReconciler. This is not required for clients CA since is is only used for trust.

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

@katheris katheris added this to the 0.45.0 milestone Nov 7, 2024
Update CaReconciler to only roll pods when cluster CA
key is replaced, not when clients CA key is replaced.

Currently we do two rolling updates for the key replacement,
once in CaReconciler, and once in component reconcilers, e.g.
KafkaReconciler. This is not required for clients CA since
is is only used for trust.

Signed-off-by: Katherine Stanley <[email protected]>
@katheris katheris force-pushed the caReconcilerOnlyRollForClusterCa branch from 1fa4618 to daee331 Compare November 7, 2024 14:31
Copy link
Contributor

@tinaselenge tinaselenge left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@ppatierno
Copy link
Member

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -375,38 +375,29 @@ Future<Void> reconcileClusterOperatorSecret(Clock clock) {

/**
* Perform a rolling update of the cluster so that CA certificates get added to their truststores, or expired CA
* certificates get removed from their truststores. Note this is only necessary when the CA certificate has changed
* due to a new CA key. It is not necessary when the CA certificate is replace while retaining the existing key.
* certificates get removed from their truststores. Note this is only necessary when the Cluster CA certificate has changed
Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to understand this doc and the method name. The doc says it's only needed for the cluster CA, but the method name doesn't make that distinction. Perhaps it should?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants