allegroai / clearml-server-k8s

ClearML Server for Kubernetes Clusters
Other
21 stars 26 forks source link

Ingress support and dynamically set apiserver's memory #3

Closed Shaked closed 4 years ago

Shaked commented 4 years ago

Hey,

As mentioned in https://github.com/allegroai/trains-server-helm/issues/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

sapir-allegro commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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