Page MenuHomePhabricator

update k8s images
ClosedPublic

Authored by johann on Aug 5 2020, 7:57 PM.

Details

Summary

k8s-dagit:

  • contains no user code (only works with grpc, should specify k8s-example image)
  • contains grapqhl

k8s-celery-worker

  • removed grapqhl

k8s-example

  • contains user repo
  • removed graphql
  • removed cron
  • removed dagit
Test Plan

manually tried on eks (note- only tried non-grpc with user code added to image)

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

johann updated this revision to Diff 20180.Aug 5 2020, 7:57 PM
johann created this revision.

up

johann requested review of this revision.Aug 5 2020, 8:17 PM
johann edited the summary of this revision. (Show Details)Aug 5 2020, 8:34 PM
johann edited the test plan for this revision. (Show Details)
johann added a reviewer: nate.
johann added a reviewer: catherinewu.
johann edited the summary of this revision. (Show Details)
nate added inline comments.Aug 5 2020, 9:12 PM
python_modules/automation/automation/docker/images/k8s-celery-worker/Dockerfile
6

given Cat's shiny new k8s scheduler, I don't think we need cron or dagster-cron at all in any of these images — @catherinewu is that correct?

python_modules/automation/automation/docker/images/k8s-example/Dockerfile
7

I think you can remove this line entirely now

catherinewu added inline comments.Aug 6 2020, 3:34 PM
python_modules/automation/automation/docker/images/k8s-celery-worker/Dockerfile
6

its good to remove in a non-dagit images.

right now, the k8s scheduler is off by default so i think we should still keep cron in the dagit image. we could add a comment clarifying that cron can be removed if they are using the k8s scheduler.

nate accepted this revision.Aug 6 2020, 4:36 PM

Looks great, thanks for doing this. after you build and push the dagit image to docker hub, can you also update the helm chart to use the dagit image?

This revision is now accepted and ready to land.Aug 6 2020, 4:36 PM
nate added a comment.Aug 6 2020, 4:37 PM

FWIW:

dagster-image build-all --name k8s-dagit
dagster-image build-all --name k8s-example
dagster-image build-all --name k8s-celery-worker
dagster-image push-dockerhub --name k8s-dagit
dagster-image push-dockerhub --name k8s-example
dagster-image push-dockerhub --name k8s-celery-worker

should take care of building & pushing these

This revision was landed with ongoing or failed builds.Aug 17 2020, 5:28 PM
Closed by commit R1:d9fae8b10651: update k8s images (authored by johann). · Explain Why
This revision was automatically updated to reflect the committed changes.