Skip to content

Conversation

Techassi
Copy link
Member

@Techassi Techassi commented Oct 20, 2025

The splits the deployment of the secret-operator as a whole into two parts:

  • The controller is deployed via a Deployment which ensures only a single instance of the secret-operator in controller mode is running in a Kubernetes cluster. This can potentially lead to perfomance issues and as such should be monitored going forward.
  • The CSI server is deployed via a DaemonSet (unchanged) as this server is needed on every node to provision requested secret volumes.

This refactor is introduced in preparation for #634, in which only a single instance of the CRD conversion webhook must exist as otherwise TLS certificate verification will fail with multiple available certificates. The Helm values.yaml structure is similar to the structure introduced in stackabletech/listener-operator#334.

Follow-up research: Look into Leases which seems to be a Kubernetes native way of enforcing that only a single instance of an application is running/does a particular piece of work in a cluster.

This is in line with what listener-operator is already doing

Author

  • Changes are OpenShift compatible (consult with @razvan and @adwk67)
  • Helm chart can be installed and deployed operator works
  • Integration tests passed (for non trivial changes)
  • Release note snippet added

Reviewer

  • Code contains useful comments
  • Code contains useful logging statements
  • Documentation added or updated. Follows the style guide.
  • Changelog updated
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

  • Feature Tracker has been updated
  • Proper release label has been added
  • Links to generated (nightly) docs added

The splits the deployment of the secret-operator as a whole into
two parts:

- The controller is deployed via a Deployment which ensures only
  a single instance of the secret-operator in controller mode is
  running in a Kubernetes cluster. This can potentially lead to
  perfomance issues and as such should be monitored going forward.
- The CSI server is deployed via a DaemonSet (unchanged) as this
  server is needed on every node to provision requested secret
  volumes.

This refactor is introduced in preparation for #634, in which only
a single instance of the CRD conversion webhook must exist as
otherwise TLS certificate verification will fail with multiple
available certificates.
@Techassi Techassi self-assigned this Oct 20, 2025
@Techassi Techassi moved this to Development: In Progress in Stackable Engineering Oct 20, 2025
@Techassi Techassi added release-note Denotes a PR that will be considered when it comes time to generate release notes. scheduled-for/25.11.0 labels Oct 20, 2025
@Techassi
Copy link
Member Author

Techassi commented Oct 20, 2025

Release Note

==== Platform improvements

===== Stackable secret-operator

The Helm Chart now deploys the secret-operator as two parts.
This separation is needed for CRD versioning and conversion by the operator.
See https://github.com/stackabletech/secret-operator/pull/645[secret-operator#645].

* The controller (which reconciles resources, maintains CRDs and provides the CRD conversion webhook) runs as a Deployment with a single replica.
* The CSI server runs on every Kubernetes cluster node via a DaemonSet (this behaviour is unchanged).
* The Helm values are adjusted in accordance to the changes above.
  See the secret-operator https://github.com/stackabletech/secret-operator/blob/25.11.0/CHANGELOG.md[changelog] for a complete overview of these changes.

@Techassi Techassi marked this pull request as ready for review October 20, 2025 11:30
@Techassi Techassi moved this from Development: In Progress to Development: Waiting for Review in Stackable Engineering Oct 20, 2025
sbernauer
sbernauer previously approved these changes Oct 20, 2025
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

The Rust code change LGTM.
I let you and @NickLarsenNZ decide about the helm structure and values.yaml, as he did related things for listener-operator in the past

@Techassi Techassi moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Oct 21, 2025
@Techassi Techassi requested a review from NickLarsenNZ October 21, 2025 10:51
Copy link
Member

@NickLarsenNZ NickLarsenNZ left a comment

Choose a reason for hiding this comment

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

LGTM

@Techassi Techassi changed the title refactor: Split into Deployment and DaemonSet refactor!: Split into Deployment and DaemonSet Oct 21, 2025
@Techassi Techassi added the release-note/action-required Denotes a PR that introduces potentially breaking changes that require user action. label Oct 21, 2025
@Techassi Techassi added this pull request to the merge queue Oct 21, 2025
@Techassi Techassi moved this from Development: In Review to Development: Done in Stackable Engineering Oct 21, 2025
github-merge-queue bot pushed a commit that referenced this pull request Oct 21, 2025
* refactor: Split into Deployment and DaemonSet

The splits the deployment of the secret-operator as a whole into
two parts:

- The controller is deployed via a Deployment which ensures only
  a single instance of the secret-operator in controller mode is
  running in a Kubernetes cluster. This can potentially lead to
  perfomance issues and as such should be monitored going forward.
- The CSI server is deployed via a DaemonSet (unchanged) as this
  server is needed on every node to provision requested secret
  volumes.

This refactor is introduced in preparation for #634, in which only
a single instance of the CRD conversion webhook must exist as
otherwise TLS certificate verification will fail with multiple
available certificates.

* chore: Add changelog entry

* chore: Update comment

* refactor: Adjust values.yaml file to be closer to listener-operator

* chore: Adjust changelog entry
@Techassi Techassi removed this pull request from the merge queue due to a manual request Oct 21, 2025
@Techassi Techassi added this pull request to the merge queue Oct 21, 2025
Merged via the queue into main with commit 4afdcc9 Oct 21, 2025
17 checks passed
@Techassi Techassi deleted the refactor/split-into-daemonset-and-deployment branch October 21, 2025 12:25
Techassi added a commit that referenced this pull request Oct 21, 2025
This was accidentally introduced in #645.
github-merge-queue bot pushed a commit that referenced this pull request Oct 21, 2025
This was accidentally introduced in #645.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/action-required Denotes a PR that introduces potentially breaking changes that require user action. release-note Denotes a PR that will be considered when it comes time to generate release notes. scheduled-for/25.11.0

Projects

Status: Development: Done

Development

Successfully merging this pull request may close these issues.

3 participants