search

1Password / connect-helm-charts

Official 1Password Helm Charts
https://developer.1password.com
MIT License
90 stars 73 forks source link

Allow Connect server to be disabled in Helm chart deployment #126

Closed ag-adampike closed 1 year ago

ag-adampike commented 1 year ago

This PR introduces a connect.create value (defaulting to true) to allow Helm to deploy the Kubernetes operator without also deploying 1Password Connect (as discussed in #125).

Corresponding flow control is introduced throughout to disable other components not needed by the operator when it is being deployed standalone, a fail condition in case both components are disabled, and an update to the NOTES.txt file for this configuration.

(I also did some housecleaning in the README based on my Markdown linter in a separate commit for clarity. 🧹)

This is a draft PR and has not yet been tested. 😅

Resolves #125.

priscilasolis commented 1 year ago

My team is interested in deploying the operator standalone too. Is it possible to prioritize this? It looks like the requested changes are resolved.

Matthiasvanderhallen commented 1 year ago

@priscilasolis we tested the fixes from this PR, with the goal of using multiple standalone operators side by side. If that's your use case as well, you will soon notice that the fix indeed allows starting 1 operator standalone, but that spinning up a second causes an issue where both try to acquire the same lock. This is something that still needs to be fixed.

priscilasolis commented 1 year ago

@Matthiasvanderhallen Is this issue something that can be fixed inside the Helm charts? If not, I suggest supporting for now the deployment of a single operator, as it is initially the case.

My use case is using a single operator per cluster, but there are no official charts that support just the operator deployment.

Matthiasvanderhallen commented 1 year ago

@priscilasolis I don't know, I'll let someone from 1password itself answer that question.

I think we have more or less the same use case.

jpcoenen commented 1 year ago

This looks good to me! Let's get it merged 🚀

@priscilasolis we tested the fixes from this PR, with the goal of using multiple standalone operators side by side. If that's your use case as well, you will soon notice that the fix indeed allows starting 1 operator standalone, but that spinning up a second causes an issue where both try to acquire the same lock. This is something that still needs to be fixed.

I think we can address that as a separate issue. Would you mind opening an issue for that?