Skip to content
This repository has been archived by the owner on Jan 24, 2023. It is now read-only.

Ingress support and dynamically set apiserver's memory #3

Merged
merged 6 commits into from
Feb 3, 2020

Conversation

Shaked
Copy link
Contributor

@Shaked Shaked commented Jan 8, 2020

Hey,

As mentioned in allegroai/clearml-server-helm#3, I have created a PR that allows developers to quickly setup their ingress. This can be done by using:

helm template ./trains-server-chart --values ./trains-server-chart/values.yaml --set ingress.enabled=true --set ingress.host=example.com --set ingress.annotations."certmanager\.k8s\.io\/cluster\-issuer"=letsencrypt-prod --set ingress.annotations.'kubernetes\.io\/ingress\.class'=nginx --set ingress.tls.secretName=test

Once ingress.enabled is set to true the ingress.host parameter is required (but I didn't find a way to throw an exception if not set). Annotations and secretName are up to developers in case needed.

The above should generate this YAML:

---
....
# Source: trains-server-chart/templates/ingress.yaml
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
    name: trains-server-ingress
    labels:
        app.kubernetes.io/name: trains-server-ingress
        app.kubernetes.io/instance: release-name
        app.kubernetes.io/part-of: trains-server
        app.kubernetes.io/managed-by: Tiller
        app.kubernetes.io/version: 0.13.0-260
        helm.sh/chart: trains-server-chart-0.13.0_1
    annotations:
        certmanager.k8s.io/cluster-issuer: letsencrypt-prod
        kubernetes.io/ingress.class: nginx
        

spec:
  tls:
    - hosts:
        - "app.example.com"
        - "files.example.com"
        - "api.example.com"
      secretName: test
  rules:
    - host: "app.example.com"
      http:
        paths:
          - path: /
            backend:
              serviceName: webserver-service
              servicePort: 80
    - host: "api.example.com"
      http:
        paths:
          - path: /
            backend:
              serviceName: apiserver-service
              servicePort: 8008
    - host: "files.example.com"
      http:
        paths:
          - path: /
            backend:
              serviceName: fileserver-service
              servicePort: 8081

I have also moved the containers.resources to values.yaml so that it will be easier to change in case needed:

The 50x error codes, I think, are a byproduct of the pod restarts, which we think are derived from k8s memory limit configuration. This is why on v0.13.0 we increased the memory limit, and to be honest I think we should be more generous with that.
I suggest you set it at 500M and check if the errors/restarts continue.

This can be set by using:
--set apiserver.resources.requests.memory="500m" --set apiserver.resources.limits.memory="500m"

What do you think?

Thank you!
Shaked

via --set apiserver.resources.requests.memory and via --set apiserver.resources.limits.memory
@sapir-allegro
Copy link
Contributor

Hey @Shaked,

Thanks for your contribution, this looks great!
What's the reason for the changes in the files under the trains-server-k8s folder?

@Shaked
Copy link
Contributor Author

Shaked commented Jan 26, 2020

@PSRosa

Thank you!

What's the reason for the changes in the files under the trains-server-k8s folder?

It's a mistake, didn't notice I was editing the wrong files 😐

@sapir-allegro
Copy link
Contributor

@Shaked
Do you want to update the pull request or can I revert these files?

@sapir-allegro sapir-allegro merged commit 1784f85 into allegroai:master Feb 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants