Kube Queueing POC #3464
Merged
Kube Queueing POC #3464
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davinchia
commented
May 18, 2021
...onnectors/source-always-works/docker-shim-mvp/destination-listen-and-echo/listen-and-echo.sh
Outdated
Show resolved
Hide resolved
…It also waits for the pod to be ready before doing anything else. Sync worker will also remove the pod on termination.
…rbyte into davinchia/kube-queueing-poc
…elete command in Sync worker.
…rbyte into davinchia/kube-queueing-poc
…rbyte into davinchia/kube-queueing-poc
* nearly working sources * update * stdin example
…t the airbyte-workers resource folder; place all the poc yamls together.
…es to Dest pod after refactor.
…ot seem to be passing in.
…rbyte into davinchia/kube-queueing-poc
also clean up kubernetes deploys.
Merged
The first 6 points of #3464. The only interesting thing about this PR is the kube pod shutdown. For whatever reason, the OkHttpPool isn't respecting the evictAll call and 1 idle thread remains. So instead of shutting down immediately, the worker pod shuts down after 5 mins when the idle thread id reaped. There isn't an easy way to modify the pool's idle reap configuration now. I do not think this issue is blocking since it's relatively benign, so I vote we create a ticket and come back to this once we do an e2e test.
* processes must handle file mounting * remove comment * default to base entrypoint * use process builder factory / select stdin / use a pool of ports * fix up * add super hacky copying example * Checkpoint: Works end to end! * Checkpoint: Use API to make sure init container is ready instead of blind sleep. Propagate exception in DefaultCheckConnectionWorker. * Refactor KubePodProcess. Checked to make sure everything still works. * Format. * Clean up code. Begin putting this into variables and breaking up long constructor function. * Add comments to explain what is happening. * fix normalization test * increase timeout for initcontainer Co-authored-by: Davin Chia <davinchia@gmail.com>
* clean up * remove source-always-works * create separate commons-docker * fix test
* enable kube e2e tests * use more generally accepted env definition * use new runners * use its own runner and install minikube differently * update name * use kubectl alias * use link instead of alias that doesn't propagate * start minikube * use driver=none * go back to using action * mess with versions * revert runner * install socat * print logs after run * also try re-runnining tasks * always wait for file transfer * use ports * increase wait timeout for kube * use different localhost ips and bump normalization to include an entrypoint * proposed fix * all working locally * revert temporary changes * revert normalization image change that's happening in a separate pr * readability * final comment
github-actions
bot
added
area/platform
davinchia
commented
Jun 9, 2021
| ports: | ||
| - containerPort: 8001 | ||
| - containerPort: 9000 |
There was a problem hiding this comment.
@jrhizor why is this necessary?
There was a problem hiding this comment.
This is the pool of worker ports that can be used by jobs on this scheduler node.
* Port over the basic changes. * Add logic to return proper exit code in the event of termination. Add comments to explain why.
* use older env format * fix build
jrhizor
approved these changes
Jun 10, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Required changes for Airbyte to be deployed into separate nodes on Kubernetes.
How
See https://docs.google.com/document/d/1h1aJr0_AFx68EQL2W0QqVN8OSWpmik-uli6ynQ2weF0/edit?usp=sharing for up to date information.
Remaining work required to merge this into
master:getPodIP(remove scan).startthat uses KubePodProcess under the hood. we may need to adjust the interface to specify I/O requirements -- like if it uses stdin -- and update usages. We may want to use a different abstraction thanProcessBuilderitself since we only want a startable process, not something where we can modify the environment, etc. MaybeSupplier<Process>? Or maybe we could just produce the process itself if nobody modifies it?add tests for not logging records in any of the containers(not doing see airbytehq/airbyte-internal-issues#103 )test failure modes for connector images (does waitFor work, is the return code correct, etc)(not doing see airbytehq/airbyte-internal-issues#103 )Recommended reading order
This is a big change. Each change is reviewed as it's merged into this branch. Not applicable.
┆Issue is synchronized with this Asana task by Unito