Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mrc-4908 k8s setup #1

Merged
merged 17 commits into from
Jan 31, 2024
Merged

Mrc-4908 k8s setup #1

merged 17 commits into from
Jan 31, 2024

Conversation

absternator
Copy link
Contributor

This PR is the k8s version of setting up and running the shiny server. Follow the readme kubernetes section to set up if wanted. The current shiny server can be reached on https://shiny-dev.dide.ic.ac.uk/ (must be in dide network)

The following is done in this PR:

  • Kubernetes resources to get the shiny server running
  • Ingress used for routing, sticky cookies load balancing, and certs
  • bash scripts to get the full app up and running
  • shiny-sync scrip to get initial app data into Kubernetes volume

README.md Outdated

### Prerequisites

A production kubernetes cluster using k3s is needed to be setup first. To setup a k8s cluster follow the guide [here](https://mrc-ide.myjetbrains.com/youtrack/articles/RESIDE-A-31/Setting-up-Kubernetes-k8s-Cluster). Note: If using dev cluster (KIND), the storageClassName: local-path & longhorn is unavailable and you will need to provision own storage class and persistant volume.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the recommended setup look like for developing this locally? or on a staging machine (separate from the shiny staging, which is really for the users)? Do we need a second cluster somewhere that we use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the link shows the dev and prod setup and they both can be used locally... for staging, we can use the same cluster and just do another deployment and rename the deployment to something else... But if you wanted a seperate machine for just staging spinning up another cluster might be easiest

mountPath: /shiny
containers:
- name: shiny
image: absternator/shiny-server:dev # todo: change to actual one!!! mrcide-
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What changes did you make here? Can we get the image build script/Dockerfile here and building on BK?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just changed the image so it has an image to pull from the docker hub... i can add the docker build & push step to the startup script? is that you want? ps.. sorry whats BK?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BK=BuildKite, where we have some non-gha CI builds running,

volumeMounts:
- name: shiny-data
mountPath: /shiny
# todo: create appropriate resource requests and limits
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the consequence of these limits? Are they speculative (affecting scheduling) or caps (meaning the scheduler kills processes that exceed them)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have updated comments but i think usually requests are set or nothing is fine most of the time ... the most common is to set up requests so that each pod has allocated specific resources before being scheduled...

annotations:
nginx.ingress.kubernetes.io/affinity: "cookie"
nginx.ingress.kubernetes.io/session-cookie-name: "shinycookie"
nginx.ingress.kubernetes.io/session-cookie-expires: "172800"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this in seconds? 48 hours? Seems reasonable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup in seconds... 48 hours

README.md Outdated
@@ -33,7 +33,7 @@ Some applications (copied over from the original deployment are in `apps`). Thes

## Bringing the bits up

```
```bash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest you remove the docker instructions and the haproxy/httpd bits entirely

k8s/krsync Outdated
@@ -0,0 +1,26 @@
#!/usr/bin/env bash

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest some hardening of your bash scripts (basically any time you write one). In general we end up with:

set -eu

which will cause the script to fail if any line errors or if you reference an undefined variable.

k8s/krsync Outdated
shift
fi

# customise this to cluster needs!!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What sort of customisation is expected here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loose comment when i first did have now removed

@@ -0,0 +1,12 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤨

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😕 ?

@@ -0,0 +1,12 @@
apiVersion: kustomize.config.k8s.io/v1beta1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably will need a name for what the users think of staging that won't confuse us - preview or testing perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have changed naming to testing... because you are right we dont have a specific staging k8s cluster

@@ -0,0 +1,11 @@
#!/usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is just a temporary thing until we have the management bit working?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup temporary until we sort the management side

@absternator absternator requested a review from richfitz January 19, 2024 10:40
Copy link

@EmmaLRussell EmmaLRussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tried running this up, will do so tomorrow, just a couple of initial questions, and typos.

Can we make a gha to test this cluster? Something like here?

What's the implication for haproxy of Ingress doing the load balancing and sticky cookies? Do we even still need haproxy? Or do we keep it as the entry point to the system from httpd?

README.md Outdated

### apache
The following is guide to run the shiny server in kubernetes. They are based on running a Kind cluster. The configuration may
nee to be adjusted for other k8s clusters.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
nee to be adjusted for other k8s clusters.
need to be adjusted for other k8s clusters.

README.md Outdated

### haproxy
A production kubernetes cluster using k3s is needed to be setup first. To setup a k8s cluster follow the guide [here](https://mrc-ide.myjetbrains.com/youtrack/articles/RESIDE-A-31/Setting-up-Kubernetes-k8s-Cluster). Note: If using dev cluster (KIND), the storageClassName: local-path & longhorn is unavailable and you will need to provision own storage class and persistant volume.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A production kubernetes cluster using k3s is needed to be setup first. To setup a k8s cluster follow the guide [here](https://mrc-ide.myjetbrains.com/youtrack/articles/RESIDE-A-31/Setting-up-Kubernetes-k8s-Cluster). Note: If using dev cluster (KIND), the storageClassName: local-path & longhorn is unavailable and you will need to provision own storage class and persistant volume.
A production kubernetes cluster using k8s is needed to be setup first. To setup a k8s cluster follow the guide [here](https://mrc-ide.myjetbrains.com/youtrack/articles/RESIDE-A-31/Setting-up-Kubernetes-k8s-Cluster). Note: If using dev cluster (KIND), the storageClassName: local-path & longhorn is unavailable and you will need to provision own storage class and persistent volume.

README.md Outdated

Build the image with `./haproxy/build` which builds `mrcide/haproxy` with a configuration that can be seen in [`haproxy/haproxy.cfg`](haproxy/haproxy.cfg) and some utilities which enable some degree of dynamic scaling of shiny servers.
Run `start-k8s-shiny <env>` to run the shint server in k8s.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Run `start-k8s-shiny <env>` to run the shint server in k8s.
Run `./start-k8s-shiny <env>` to run the shiny server in k8s.

ingressClassName: nginx
tls:
- hosts:
- shiny-dev.dide.ic.ac.uk

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we'll need to update this setting for prod, and for running locally?

namespace: twinkle # assume this namespace exists

resources:
- persistance.yaml

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- persistance.yaml
- persistence.yaml

README.md Outdated

### haproxy
A production kubernetes cluster using k3s is needed to be setup first. To setup a k8s cluster follow the guide [here](https://mrc-ide.myjetbrains.com/youtrack/articles/RESIDE-A-31/Setting-up-Kubernetes-k8s-Cluster). Note: If using dev cluster (KIND), the storageClassName: local-path & longhorn is unavailable and you will need to provision own storage class and persistant volume.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

storageClassName: local-path & longhorn is unavailable and you will need to provision own storage class and persistant volume.

Sorry, don't really understand this. Why aren't these available? You talk about using local-path below, in persistance.yaml.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have updated to avoid confusion

mountPath: /shiny
containers:
- name: shiny
image: absternator/shiny-server:dev # todo: change to actual one!!! mrcide-

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BK=BuildKite, where we have some non-gha CI builds running,

@absternator
Copy link
Contributor Author

absternator commented Jan 25, 2024

@EmmaLRussell have added GHA to add shiny server image to build and push... also removed haproxy and apache server and fixed typos.. will seee the testing stuff as linked for testing cluster in another PR...as this needs more thought around what really needs to be tested

@absternator absternator merged commit 05db8b5 into main Jan 31, 2024
@absternator absternator deleted the mrc-4908-k8s-setup branch January 31, 2024 09:09
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.

3 participants