voxel51 / fiftyone-teams-app-deploy

Deployment Assets for the Fiftyone Teams App
Apache License 2.0
3 stars 1 forks source link

Adding workers to helm/k8s to be used for executing delegated operations #210

Open swheaton opened 3 weeks ago

swheaton commented 3 weeks ago

Rationale

K8s/helm companion to https://github.com/voxel51/fiftyone-teams-app-deploy/pull/207 (docker)

As a new initiative we want users to be able to run delegated operations out of the box without airflow. These changes will allow users to configure X amount of workers which will pull delegated operations at a set interval and run them in the background in separate pods.

Changes

Checklist

Testing

  1. Spun up ephemeral k8s environment
  2. Generated yaml file via: helm template helm/fiftyone-teams-app s templates/delegated-operator-executor-deployment.yaml -f helm/fiftyone-teams-app/values.yaml -f stuart-ephem-values.yaml > teams-do.yaml.
  3. Edited teams-do.yaml to contain delegatedOperatorApiKey instead of pulling from the secrets resource
    • Because the secrets resource would be auto-sync'ed by ArgoCD if I were to make edits to it.
  4. Deployment contains teams-do deployment, ReplicaSet and Pods created successfully, queued delegated operation is successfully completed.

TODO

findtopher commented 3 weeks ago

Will Delegated Operators need access to the PV that plugins and the API use to store plugins? If so, we should update the documentation accordingly.

We'll want to update the README.md.gotmpl to include a section in Teams Features regarding how to turn this on (and adding volumes if necessary)

You'll want to add tests, similar to the other services, please.

swheaton commented 3 weeks ago

Will Delegated Operators need access to the PV that plugins and the API use to store plugins? If so, we should update the documentation accordingly.

We'll want to update the README.md.gotmpl to include a section in Teams Features regarding how to turn this on (and adding volumes if necessary)

Added a TODO for this probably while you were typing. Want to get feedback on the overall approach in the meantime :D

You'll want to add tests, similar to the other services, please.

Sure thing

swheaton commented 3 weeks ago

Added teams-do deployment to the integration tests.

  1. I am not able to run integration tests locally due to not enough RAM. Kevin volunteered to run them for me.
  2. Same comment applies from https://github.com/voxel51/fiftyone-teams-app-deploy/pull/207

    Intg tests will not pass because code from develop>v2.1 is required but tag 2.1 is used for these tests. We can wait until dev-cut, or fudge it with a develop tag in this branch? Not sure how this has been done in past releases.

swheaton commented 3 weeks ago

Thanks @kevin-dimichel !

kevin-dimichel commented 2 weeks ago

@swheaton - https://github.com/voxel51/fiftyone-teams-app-deploy/pull/220 will cause merge conflicts in this PR since the changes I made to the integration tests were also improved by @afoley587

afoley587 commented 2 weeks ago

Yes, I can help with merge conflicts!

swheaton commented 2 weeks ago

thanks alex