Skip to content

Conversation

bokristoffersson
Copy link

When running in production we are investigating in running without an admin server since no configuration changes are to be made outside of gitops configuration.

@anestos
Copy link
Collaborator

anestos commented Apr 7, 2022

Isn't a better approach to not deploy anything related to admin when curity.onlyRuntimeNodes is set to true?
you can have an {{- if not .Values.curity.onlyRuntimes -}} at the top of the deployment-admin and service-admin.

Similarly, the backup, cluster-conf job and related resources are not necessary, since there is no connection between the runtime nodes and also there is no interface for confiugration changes to trigger a backup.

finally, the ingress resource should also if enabled, not configure a route for the admin, if onlyRuntimes is true

@bokristoffersson
Copy link
Author

True. That make sense.

@bokristoffersson bokristoffersson marked this pull request as draft April 12, 2022 06:05
@bokristoffersson
Copy link
Author

I put this into draft. I question if it is a good idea to have this in the charts? It will add some complexity and maybe it is not needed at all. If you want to deploy only runtime nodes you might as well use the manifest files anyway since it will be only deployment, service and ingress. Thus, If it will not be used there is no need for it.

bo.kristoffersson added 2 commits May 12, 2022 14:53
# Conflicts:
#	idsvr/README.md
#	idsvr/templates/cluster-conf.yaml
#	idsvr/templates/config-backup.yaml
#	idsvr/templates/deployment-admin.yaml
#	idsvr/templates/deployment-runtime.yaml
#	idsvr/templates/ingress.yaml
#	idsvr/templates/network.yaml
#	idsvr/templates/rbac.yaml
#	idsvr/templates/service-admin.yaml
@bokristoffersson bokristoffersson marked this pull request as ready for review May 16, 2022 13:34
@bokristoffersson
Copy link
Author

I opened this again. Since we are able to now convert keystores during startup it really make sense to also be able to use the Helm charts for production using only runtime nodes. I also implemented your suggestion above and now Helm will only create manifest files that are needed.

@bokristoffersson bokristoffersson deleted the only_runtime_nodes branch June 16, 2022 08:12
@bokristoffersson bokristoffersson restored the only_runtime_nodes branch June 16, 2022 08:13
@bokristoffersson
Copy link
Author

Sorry. I deleted the branch unintentionally.

@anestos
Copy link
Collaborator

anestos commented Jul 1, 2022

This PR needs a little bit more love @bokristoffersson .
using {{- if eq .Values.curity.onlyRuntimeNodes false }} can be problematic for users of the helm chart that don't have that setting in their values file. Can you negate it? Either {{- if not .Values.curity.onlyRuntimes -}} or {{- if ne .Values.curity.onlyRuntimeNodes true }}`.

Also can you document the curity.onlyRuntimeNodes setting in the readme ?

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.

2 participants