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

Adding Postgres Helm chart to rad init #8072

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions deploy/Chart/templates/database/configmaps.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: v1
rynowak marked this conversation as resolved.
Show resolved Hide resolved
kind: ConfigMap
metadata:
name: database-secret
namespace: "{{ .Release.Namespace }}"
labels:
control-plane: database
rynowak marked this conversation as resolved.
Show resolved Hide resolved
app.kubernetes.io/name: database
app.kubernetes.io/part-of: radius
data:
POSTGRES_DB: ps_db
POSTGRES_USER: ps_user
POSTGRES_PASSWORD: SecurePassword
rynowak marked this conversation as resolved.
Show resolved Hide resolved
16 changes: 16 additions & 0 deletions deploy/Chart/templates/database/service.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: v1
kind: Service
metadata:
name: database
rynowak marked this conversation as resolved.
Show resolved Hide resolved
namespace: "{{ .Release.Namespace }}"
labels:
app.kubernetes.io/name: database
app.kubernetes.io/part-of: radius
spec:
ports:
- port: 5432
name: postgres
protocol: TCP
targetPort: 5432
selector:
app.kubernetes.io/name: database
8 changes: 8 additions & 0 deletions deploy/Chart/templates/database/serviceaccount.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: v1
kind: ServiceAccount
metadata:
name: database
namespace: {{ .Release.Namespace }}
labels:
app.kubernetes.io/name: database
app.kubernetes.io/part-of: radius
52 changes: 52 additions & 0 deletions deploy/Chart/templates/database/statefulset.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
apiVersion: apps/v1
kind: StatefulSet
metadata:
name: database
namespace: "{{ .Release.Namespace }}"
labels:
control-plane: database
app.kubernetes.io/name: database
app.kubernetes.io/part-of: radius
spec:
serviceName: "database"
replicas: 1
rynowak marked this conversation as resolved.
Show resolved Hide resolved
selector:
matchLabels:
app.kubernetes.io/name: database
template:
metadata:
labels:
control-plane: database
app.kubernetes.io/name: database
app.kubernetes.io/part-of: radius
spec:
serviceAccountName: database
containers:
- name: database
securityContext:
allowPrivilegeEscalation: false
image: "{{ .Values.database.image }}:{{ .Values.database.tag }}"
imagePullPolicy: IfNotPresent
resources:
requests:
memory: "{{ .Values.database.resources.requests.memory }}"
cpu: "{{ .Values.database.resources.requests.cpu }}"
limits:
memory: "{{ .Values.database.resources.limits.memory }}"
cpu: "{{ .Values.database.resources.limits.cpu }}"
envFrom:
- configMapRef:
name: database-secret
ports:
- containerPort: 5432
name: postgres

volumeClaimTemplates:
- metadata:
name: database
spec:
accessModes: ["ReadWriteOnce"]
storageClassName: "{{ .Values.database.storageClassName }}"
resources:
requests:
storage: {{ .Values.database.storageSize }}
13 changes: 13 additions & 0 deletions deploy/Chart/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,16 @@ dashboard:
memory: "60Mi"
limits:
memory: "300Mi"

database:
image: ghcr.io/radius-project/mirror/postgres
superbeeny marked this conversation as resolved.
Show resolved Hide resolved
tag: latest
rynowak marked this conversation as resolved.
Show resolved Hide resolved
storageClassName: "standard" # set to the storage class name if required
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this need to change depending on where the Kubernetes cluster is hosted?

storageSize: "1Gi"
resources:
requests:
cpu: "2"
memory: "512Mi"
limits:
cpu: "2"
memory: "1024Mi"
Comment on lines +120 to +127
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious how did we decide these values. Would helpful if you could add a comment in the code for context as well.

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'll add a comment here

Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,21 @@ resource sqlContainer 'Applications.Core/containers@2023-10-01-preview' = {
}
}
}
runtimes: {
Copy link
Contributor

Choose a reason for hiding this comment

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

are we adding this to resolve the sqlserver functional test issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was an attempt to try and limit the size of the sqlserver, I'll revert in a subsequent commit

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a pretty large amount of resources still (2 GB). Did this help?

Copy link
Contributor

Choose a reason for hiding this comment

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

@rynowak per our offline discussion, do we want to go ahead and delete this test? If so, @superbeeny can do this as part of this PR which will also unblock it.

For context, @superbeeny, we added a functional test for every portable resource type initially because we had custom implementation for each of them. Since then the implementation has evolved and majority of the code has converged across all types, so it is covered extensively through other functional tests. SQL server specifically is also covered in Radius eshop sample.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to the consensus that this is ok to remove. It think that would mean deleting https://github.com/radius-project/radius/blob/main/test/functional-portable/datastoresrp/noncloud/resources/sql_test.go and the other files it references.

There's actually more than one sql server test which is why I think we're hitting issues. That's 4gb minimum used by those tests.

kubernetes: {
pod: {
resources: {
requests: {
cpu: '1200m'
memory: '2048Mi'
}
limits: {
cpu: '1500m'
memory: '4096Mi'
}
}
}
}
}
}
}
Loading