-
Notifications
You must be signed in to change notification settings - Fork 22
Support optional use of Billing container in Helm chart #110
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
base: main
Are you sure you want to change the base?
Conversation
| updateStrategy: | ||
| type: RollingUpdate | ||
| rollingUpdate: | ||
| maxUnavailable: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would make more sense to explicitly define maxSurge alongside this to clearly describe the behavior.
However, I'm not sure if this will work with the way you've setup the volumes. If you have extra billing containers during a deployment, they can't all read and write from the same volume that is configured with ReadWriteOnce. You would need a ReadWriteMany, and you'd want to ensure you set it up as a PersistentVolume so that it could be accessed after the containers are spun down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The billing.deployment.yaml uses a StatefulSet rather than a Deployment, so each Billing pod gets its own dedicated journal file via volumeClaimTemplates. That does mean there will be N journal files corresponding to N Billing pods. The StatefulSet uses ordered, one-at-a-time rollouts, so a maxSurge doesn't apply. The ReadWriteOnce is okay here since each Billing pod owns its exclusive PVC. Let me know if you see any downside to Billing using a StatefulSet instead of a Deployment like API/Engine/LP are using, as well as implications of producing multiple journal files for multiple Billing pods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I missed that, makes sense. As long as we've tested it, makes sense.
It looks like we have the client provide a custom persistent volume to store that journal file. Could we add documentation in the README over how to do that itself, to help provide guardrails to guide folks to persist that correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on the need for documentation overall.
To answer your specific point, the journal persistence is automatic with no manual PV creation needed. The StatefulSet uses volumeClaimTemplates, so Kubernetes automatically provisions the PVCs/PVs via the cluster's StorageClass (like gp2 on AWS). Customers configure billing.journal.storageClass (optional, defaults to cluster default), and billing.journal.size (optional, defaults to 1Gi).
I will add documentation with guidance on how to back up the PVC outside of the chart (such as with Velero or cloud-specific snapshots).
It's also important to help customers know how to exfiltrate the journal file. I had to create a debugging container - some kind of sidecar container is needed, or else use a cron job to copy to S3, like on a set cadence. Do you have any feedback around a journal file exfiltration method? S3 might be difficult in an airgapped context, but a sidecar container also adds complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added documentation at a few levels: 82fe595
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In particular, fleshed out documentation in a new airgapped readme: https://github.com/deepgram/self-hosted-resources/blob/e2e5029c7460d7d83ddaba14f165a3c1687a0df5/charts/deepgram-self-hosted/samples/airgapped.md
charts/deepgram-self-hosted/templates/license-proxy/license-proxy.deployment.yaml
Outdated
Show resolved
Hide resolved
charts/deepgram-self-hosted/templates/license-proxy/license-proxy.deployment.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkroll-deepgram I have seen u already fixed that envsubst and API KEY error , i also got one more error related to flux , i think that config expects script_path and socket_path to be present,
Error: TOML parse error at line 26, column 1
|
26 | [flux]
| ^^^^^^
missing field `script_path`
| @@ -0,0 +1,137 @@ | |||
| {{- if .Values.billing.enabled -}} | |||
| apiVersion: apps/v1 | |||
| kind: StatefulSet | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkroll-deepgram I think as billing is statefulset , so it might be better to name this file as billing.statefulset.yaml so people dont get consfused by the name
Proposed changes
Adds optional support for the Billing container, providing airgapped license management and usage tracking for Deepgram self-hosted deployments.
This implementation provides enterprise customers with a robust, HA-capable, airgapped licensing solution while maintaining full backward compatibility with existing cloud-connected deployments.
Key Features & Implementation Choices
1. Deployment Flexibility
license.deepgram.com(default, no changes)2. High Availability (HA) Support
volumeClaimTemplates(ReadWriteOnce by default)billing.journal.existingPvcNamefor EFS/NFS (ReadWriteMany)3. Persistent Usage Tracking
/journal/journal) persists all usage data for billing and compliance4. Secure Secret Management
global.deepgramLicenseSecretRef: License key (env var:DEEPGRAM_LICENSE_KEY)billing.licenseFile.secretRef: License file (mounted as/license/license.dg)sedin initContainers for secure key injectionglobal.initContainer.image) withubuntu:22.04default5. Minimal Container Design
tar) for security and size optimizationsamples/airgapped.md)6. Seamless License Proxy Integration
licenseProxy.upstream.useBillingtrue: License Proxy forwards requests to Billingfalse: License Proxy useslicense.deepgram.com(default)7. Automatic Configuration Management
billing.enabledbilling-internalheadless serviceDEEPGRAM_API_KEY=airgapped-modefor API/Engine/License Proxy to satisfy internal checksbilling.enabled: false8. Resource Management
k8s.deepgram.com/node-type=billing)9. Comprehensive Documentation
samples/airgapped.mdwith step-by-step instructions06-basic-setup-aws-airgapped.values.yaml- Complete airgapped deployment values06-basic-setup-aws-airgapped.cluster-config.yaml- EKS cluster setup for airgapped deploymentsMigration Path
billing.enabled: trueand configure secretssamples/airgapped.mdTechnical Highlights
generic(license key),generic(license file as file mount)ubuntu:22.04) withsedfor runtime secret injectionTesting Completed
sedTypes of changes
What types of changes does your code introduce to the Deepgram self-hosted resources?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.I brought up a deployment with this Helm chart and the
06airgapped samples, served a test STT request, and confirmed that the usage was logged in the journal file.Further comments