various Kubernetes deployment improvements #862
Conversation
…nstead of loadbalancer. Implemented the generation of a secret from the .env file and then put as environment variables into the deployments. Nextauth_URL is now set in the kustomization file and is then generated into a configmap and put as an env into the deployments. Opionated change: the web service is now a clusterIP Service and an ingress object is included.
There was a problem hiding this comment.
Hey, thanks a lot for taking the time and sending a PR! I also appreciate the detailed PR summary.
I left a couple of small comments. I think we should also update the docs here (https://docs.hoarder.app/Installation/kubernetes) in the same PR so that it doesn't become outdated once this PR lands.
kubernetes/ingress.yaml
Outdated
| tls: | ||
| - hosts: | ||
| - hoarder.example.com | ||
| secretName: tls-secret |
There was a problem hiding this comment.
I'd like people to be able to download the chart and then having it work with minimal configuration. Let's not force people to have TLS configuration, maybe comment those lines and add a comment to uncomment them if TLS is needed?
There was a problem hiding this comment.
Good point, haven't thought about that, i will comment these lines and document how to configure TLS.
There was a problem hiding this comment.
Thinking about the idea of having it work with minimal configuration i see a general "problem" with the idea of using an ingress.
Using an ingress object requires that the user has a proper way to manage their DNS and that could already be seen as a quite advanced task, however they might manage their domain.
However, i think people that are deploying into an kubernetes cluster might be already advanced enough to also have configured a DNS service for their infrastructure.
If we want to accomodate both possibilities there would need to be way to be able to choose between the LoadBalancer or ingress at installation but currently i'm not versed enough in kustomize to know if that is possible to implement, so we might need to decide on one or another and i can document how to implement either LoadBalancer or ingress manually.
kubernetes/kustomization.yaml
Outdated
| - name: nextauth-configuration | ||
| literals: | ||
| - NEXTAUTH_URL=https://hoarder.example.com |
There was a problem hiding this comment.
I'd like to still keep the .env file as the place were people keep thier configuration for hoarder. Should we maybe have a .env and .secrets files? One generates a config map and the other generates secrets?
There was a problem hiding this comment.
Yes, in the current way of installation that is no problem, just need to add a seperate configmapGenerator.
kubernetes/kustomization.yaml
Outdated
|
|
||
| images: | ||
| - name: ghcr.io/hoarder-app/hoarder | ||
| newTag: 0.21.0 # Specify the version here |
There was a problem hiding this comment.
I'm not a big fan of pinning the version here. While I get your point (and I do this to all my images personally). I think for convenience we should keep it pointing at the release tag so that people can update their images without any hassle. Whoever wants to pin it, can still do so.
There was a problem hiding this comment.
Should not be a problem, i will update the deployment to have the imagePullPolicy set to always so people don't get stuck on an old version and just need to deploy a new pod.
changed the default from ingress back to Loadbalancer. Added Documentation on how to change to ingress and add TLS Support. split env to secret and env file which have to be configured before deploying.
|
@MohamedBassem I took your feedback into consideration and changed the following:
My reasoning for going back to using the LoadBalancer by default is that it is a lower barrier of entry since you are not forced to configure DNS or your hosts file. i added documention for the people who want to use an ingress and possibly TLS. I also added an instruction on how to update the hoarder image in the web deployment without using the make file, since i don't use it since i use kustomize directly. Lastly, while testing my changes i have one open question: The documentation states that the kubernetes config also has to change the NEXTAUTH_URL var, i have it on the default "http://localhost:3000" and using an ingress object i still can access the service just fine and sign up and use it. Did i miss something anywhere? Also, depending on the setup of the service which provides the loadbalancer there is the possibility that the user doesn't know its IP beforehand, it appears to me that could be a problem if the user actually has to change the setting. (see my question before about the NEXTAUTH_URL var.) Kind regards. |
|
@techtrd this is great, I was just making very similar changes in my own install and was going to submit a PR, but you covered it all and more. One change that I can suggest it to change the The default |
| # Use `openssl rand -base64 36` to generate the random strings | ||
| NEXTAUTH_SECRET=generated_secret | ||
| MEILI_MASTER_KEY=generated_secret | ||
| NEXT_PUBLIC_SECRET="my-super-duper-secret-string" |
There was a problem hiding this comment.
I am actually not sure why NEXT_PUBLIC_SECRET exists in the k8s config. It is not mentioned in the https://docs.hoarder.app/configuration and does not exist in the Docker config https://github.com/hoarder-app/hoarder/blob/main/docker/.env.sample
My guess is that NEXT_PUBLIC_SECRET was used in older version of NextAuth and that NEXTAUTH_SECRET is the correct way to do it for the current version, but I could be wrong here
I have removed NEXT_PUBLIC_SECRET from my config and it is continues to work fine
There was a problem hiding this comment.
Ah yeah, this indeed should be removed.
|
Really appreciate the effort in responding to the comments and improving the docs! Thanks a lot! |
What has changed?
Changed the secrets from configmap to secret object.
kustomize creates a secret from the .env file containing the three secrets. the Nextauth_URL is defined in the kustomize.yaml and is put into a configMap when kustomize builds the manifests.
Changed web loadbalancer service from loadbalancer to ClusterIP and implemented ingress
This may be opionated but i think it may be better to use the full toolkit of kubernetes and therefore use an ingress object instead of publishing via loadbalancer.
Also the ingress is by default build to use TLS, this might be overkill for homelab use but i think in the long run it would be best to always default to build with tls first, people who are not able to use TLS in their cluster can remove the TLS configuration from the ingress before or after deployment.
Example: I use a basic RKE2 install which comes with an ingress-nginx listening on the node IP but it does not come with a loadbalancer by default or atleast as far as i know.
The Version Tag is now controlled in the kustomize.yaml
Following the kubernetes documentation and best practices we follow at work, i think it is better to put an explicit version in the image tag of the deployment. It should be up to the operator to always keep up to date with the software which is deployed in the cluster.
Maybe this can be improved in other ways, maybe even automated but i'm not knowledgeable enough yet to know of a way to automate it.
What is missing?
There is no TLS cert created during the kustomize build process. I always include my cert with a seperately created secret via kubectl.
The other thing missing is installation documentation, i will create another PR for the README soon.
What and how did i test?
I put the secrets into an .env file and changed the relevant parts in the kustomize.yaml. After that i used
kustomize build . > manifests.yamlto create the manifests and applied them withkubectl apply -f manifests.yamlinto my local RKE2 node.i could successfully create a user on the web interface, login and create a node with a news site which also fetched the title and the image. Therefore i concluded that everything works as it should.
Thank you in advance,
Kind regards