Changed initialDelaySeconds to 120 and set CPU to 1vcpu and memory to 4G

[cabeaulac]

Changes to get LocalStack to deploy nicely on AWS EKS.

[alexrashed]

Hey @cabeaulac!
Thanks for the contribution!
I can see that these changes were necessary in your efforts to use the helm chart to deploy LocalStack in an EKS cluster.
But it would be great if we could try to avoid putting these (fairly restrictive) settings into the deployment directly, or set them as default values.
Let me know if I can help in adjusting the PR such that we can merge it.

[alexrashed]

The values here in the values.yaml are default values used for every single deployment, not only for those on EKS.
Your change here sets it for everyone, where I would prefer to have this in a documentation, or in a shared values file specifically for EKS which can then be used when deploying to EKS.
For example:

helm install -f eks-values.yaml localstack/localstack

If these settings are really applicable for every EKS cluster, we could share them here directly, or we could put them on the docs pages.

But actually, I am not sure why these are necessary in the first place. Could you maybe please add some explanation why it is necessary to limit the CPU and memory in EKS?

[cabeaulac]

Empty defaults are too low. The container fails to come up, runs out of memory and is killed. I think not setting the values at all leaves us with a LocalStack that won't run in most cases. Should we have an empty base default like this? Or always force an override?

[simonrw]

@alexrashed these are not limits, but minimum requirements. I don't know if k3s respects resource requests like this, but EKS with fargate certainly requires them.

I like the idea of having example "supported" override files for different scenarios, however I suspect there are only two, but there may be more:

• local using k3d, the default with LocalStack where it doesn't matter since the default nodes have the resources of the host, and
• a "real" k8s cluster e.g. EKS where we have to bump up the values a bit.

[alexrashed]

Could you explain a bit why this would be necessary?
120 seconds is quite long, if we have to add the initialDelaySeconds for a deployment on EKS, we should add it in a parameterized way with a way shorter value (because for the vast majority of users, this setting will slow down their deployment by at least one minute).
When this has been added as a parameterized value, we could add it to the eks-values.yaml (as mentioned in the comment below).

[cabeaulac]

Needed this in EKS with Fargate as it takes so long to launch a new pod. I'll move to eks-values.yaml override.

[simonrw]

I agree with @alexrashed here, and the way this is constructed always overrides what can be specified in an override file. See

helm-charts/charts/localstack/values.yaml

Lines 97 to 102 in ce47b15

	readinessProbe:
	initialDelaySeconds: 0
	periodSeconds: 10
	timeoutSeconds: 1
	successThreshold: 1
	failureThreshold: 3

I also however agree the initial delay shouldn't be 0 seconds, as the container always takes some time to start up, but the total 30 seconds (3 failure periods x a period of 10 seconds) is reasonably generous. When I try with EKS + fargate as you have been doing @cabeaulac, I can still get a working pod in 30 seconds, provided the pod resources are higher than the default. It feels to me that a bit of initial delay (e.g. 5 seconds) where failures to reach the health endpoint do not count towards a failing health check makes sense.

[cabeaulac]

Ok. These fields are overridable in values.yaml. Closing the PR.