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

Helm chart refactoring #3797

Merged
merged 5 commits into from
Feb 3, 2025
Merged

Helm chart refactoring #3797

merged 5 commits into from
Feb 3, 2025

Conversation

trial-danswer
Copy link
Collaborator

@trial-danswer trial-danswer commented Jan 27, 2025

Description

[Provide a brief description of the changes in this PR]

How Has This Been Tested?

I used eksctl cli to provision a eks cluster in aws. The following is the config.yaml that I used to provision the cluster.

apiVersion: eksctl.io/v1alpha5
kind: ClusterConfig

metadata:
name: onyx-cluster
region: us-west-1

nodeGroups:

  • name: ng-1
    instanceType: m5.large
    desiredCapacity: 1
    maxSize: 2
    minSize: 1
    volumeSize: 80
    ssh:
    allow: true # will use ~/.ssh/id_rsa.pub as the default ssh key
  • name: ng-2
    instanceType: m5.xlarge
    desiredCapacity: 1
    maxSize: 2
    minSize: 1
    volumeSize: 100
    ssh:
    publicKeyPath: ~/.ssh/id_rsa.pub
    iam:
    withAddonPolicies:
    imageBuilder: true
    autoScaler: true
    externalDNS: true
    certManager: true
    appMesh: true
    appMeshPreview: true
    ebs: true
    fsx: true
    efs: true
    awsLoadBalancerController: true
    xRay: true
    cloudWatch: true

addonsConfig:
autoApplyPodIdentityAssociations: true

addons:

  • name: aws-ebs-csi-driver
    version: latest
    useDefaultPodIdentityAssociations: true
  • name: eks-pod-identity-agent
    version: latest
    useDefaultPodIdentityAssociations: true

iam:
podIdentityAssociations:

  • namespace: default
    serviceAccountName: default
    permissionPolicyARNs:
    • "arn:aws:iam::aws:policy/service-role/AmazonEBSCSIDriverPolicy"
      permissionPolicy:
      Version: "2012-10-17"
      Statement:
    • Effect: Allow
      Action:
      • "autoscaling:DescribeAutoScalingGroups"
      • "autoscaling:DescribeAutoScalingInstances"
      • "autoscaling:DescribeLaunchConfigurations"
      • "autoscaling:DescribeTags"
      • "autoscaling:SetDesiredCapacity"
      • "autoscaling:TerminateInstanceInAutoScalingGroup"
      • "ec2:DescribeLaunchTemplateVersions"
        Resource: '*'

The following are the commands used to provision a cluster, pull in latest sub-charts, and install onyx onto the new EKS cluster.

eksctl create cluster -f config.yaml # creates the 2 node eks cluster

helm upgrade dependency # upgrades the sub-charts

helm install onyx . # installs the onyx apps onto the cluster

[Describe the tests you ran to verify your changes]

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

  • This PR should be backported (make sure to check that the backport attempt succeeds)
  • [Optional] Override Linear Check

jpb80 added 2 commits January 23, 2025 17:27
…e apps to a cluster in aws. The bottleneck was setting up PVC dynamic provisioning.
Copy link

vercel bot commented Jan 27, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
internal-search ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 27, 2025 10:52pm

@@ -6,7 +6,7 @@ sources:
- "https://github.com/onyx-dot-app/onyx"
type: application
version: 0.2.1
appVersion: "latest"
appVersion: latest
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The appVersion maps to the onyx version for the apps. A user can set this value to deploy and pin the specific version of onyx.

@trial-danswer trial-danswer requested a review from Weves January 27, 2025 19:00
@@ -473,7 +498,8 @@ configMap:
LOG_DANSWER_MODEL_INTERACTIONS: ""
LOG_VESPA_TIMING_INFORMATION: ""
# Shared or Non-backend Related
WEB_DOMAIN: "http://localhost:3000" # for web server and api server
DOMAIN: "localhost" # for nginx
WEB_DOMAIN: "http://localhost:3000"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: any reason we removed this comment?

# keys are lowercased version of env vars (e.g. SMTP_USER -> smtp_user)
existingSecret: "" # onyx-secrets
existingSecret: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this one do? It doesn't seem clear from this comment

Copy link
Contributor

Choose a reason for hiding this comment

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

also, is the below the right way to do secrets in helm?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Helm does not have built-in secret management capabilities. However, there are several approaches to managing secrets in Helm deployments:

1️⃣ External Secrets Operator (ESO) – ESO allows Kubernetes to sync secrets from external providers (e.g., AWS Secrets Manager, HashiCorp Vault) by creating a SecretStore. Applications can then access these secrets based on namespace permissions and role-based access control (RBAC). More details: External Secrets Operator - AWS Secrets Manager

2️⃣ Helm Secrets Plugin – The helm-secrets plugin enables Helm to decrypt secrets on the fly when deploying charts. It supports encrypted values stored in Git (via SOPS) or dynamic retrieval from providers like AWS Secrets Manager using Vals.

3️⃣ GitOps-Based Secret Management (ArgoCD, Flux, etc.) – Tools like ArgoCD provide flexible secret management options, integrating with both helm-secrets and external-secrets, among others, to securely sync secrets in Kubernetes clusters.

Each approach has its own advantages, depending on security requirements and deployment workflows.

@Weves Weves merged commit 715359c into main Feb 3, 2025
13 of 15 checks passed
@Weves Weves deleted the jbuttkevitz/helm_chart_refactor branch February 3, 2025 18:56
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