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

Refactor fileserver into sidecar with progress reporting #1404

Merged
merged 14 commits into from Mar 4, 2020

Conversation

@DaoWen
Copy link
Contributor

DaoWen commented Feb 4, 2020

Note: This PR currently targets a throw-away branch. This PR should be rebased onto master after PR #1403 is merged, and the throw-away branch should be deleted.

Changes proposed in this PR

  • Renames the current "fileserver" to the more generic "sidecar"
  • Adds progress-reporting functionality to the sidecar (copied and adapted from the Cook Executor)
  • Enables the existing progress reporting integration tests to also run on Kubernetes.

Why are we making these changes?

Better parity of our Kuberentes scheduler with our Mesos scheduler with regard to job progress reporting.

Copy link
Contributor

shamsimam left a comment

We should move

  • Renames the current "fileserver" to the more generic "sidecar"
    to a separate PR to reduce the scope of this PR.
scheduler/src/cook/config.clj Outdated Show resolved Hide resolved
scheduler/src/cook/config.clj Outdated Show resolved Hide resolved
scheduler/src/cook/kubernetes/api.clj Outdated Show resolved Hide resolved
scheduler/src/cook/kubernetes/api.clj Outdated Show resolved Hide resolved
sidecar/cook/sidecar/__main__.py Outdated Show resolved Hide resolved
sidecar/cook/sidecar/file_server.py Outdated Show resolved Hide resolved
sidecar/cook/sidecar/progress.py Show resolved Hide resolved
sidecar/cook/sidecar/progress.py Outdated Show resolved Hide resolved
sidecar/cook/sidecar/progress.py Show resolved Hide resolved
sidecar/cook/sidecar/progress.py Outdated Show resolved Hide resolved
@DaoWen DaoWen force-pushed the twosigma:draft/refactor-progress-tests branch from 06fa902 to 4f8f31c Feb 10, 2020
@DaoWen

This comment has been minimized.

Copy link
Contributor Author

DaoWen commented Feb 10, 2020

@shamsimam - Opened PR #1419 to split the rename of "fileserver" to "sidecar" into a separate patch.

@DaoWen DaoWen force-pushed the DaoWen:feature/k8s-sidecar-progress branch 2 times, most recently from f6b0e34 to c9309e0 Feb 10, 2020
@DaoWen DaoWen changed the base branch from draft/refactor-progress-tests to master Feb 10, 2020
@DaoWen DaoWen marked this pull request as ready for review Feb 10, 2020
@DaoWen

This comment has been minimized.

Copy link
Contributor Author

DaoWen commented Feb 10, 2020

The fileserver module renaming PR (#1419) has been merged, and this PR has been rebased on top of that new master commit.

@DaoWen DaoWen force-pushed the DaoWen:feature/k8s-sidecar-progress branch 2 times, most recently from f4eb6a6 to 5c5b7c8 Feb 19, 2020
@DaoWen DaoWen removed the internal-green label Feb 25, 2020
@shamsimam

This comment has been minimized.

Copy link
Contributor

shamsimam commented Feb 26, 2020

Awaiting green build.

@DaoWen DaoWen force-pushed the DaoWen:feature/k8s-sidecar-progress branch 8 times, most recently from d0fb62e to fea91ef Feb 27, 2020
@DaoWen

This comment has been minimized.

Copy link
Contributor Author

DaoWen commented Mar 3, 2020

Currently failing because we're not doing this in the job's container:
https://github.com/twosigma/Cook/blob/master/executor/cook/executor.py#L284

@DaoWen DaoWen force-pushed the DaoWen:feature/k8s-sidecar-progress branch 2 times, most recently from 33a879c to 1b11909 Mar 3, 2020
scheduler/config-k8s.edn Outdated Show resolved Hide resolved
scheduler/src/cook/kubernetes/api.clj Show resolved Hide resolved
scheduler/src/cook/kubernetes/api.clj Show resolved Hide resolved
scheduler/test/cook/test/kubernetes/api.clj Outdated Show resolved Hide resolved
@DaoWen DaoWen force-pushed the DaoWen:feature/k8s-sidecar-progress branch from 876991c to c5dffb1 Mar 4, 2020
@DaoWen DaoWen force-pushed the DaoWen:feature/k8s-sidecar-progress branch from c5dffb1 to c031e9e Mar 4, 2020
@shamsimam shamsimam merged commit c9b7791 into twosigma:master Mar 4, 2020
2 checks passed
2 checks passed
Mergeable Mergeable Run has been Completed!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.