k8s: implement invoker-node affinity and eliminate usage of kubectl #3963
Conversation
dgrove-oss
commented
Aug 13, 2018
•
dgrove-oss
commented
Codecov Report
@@ Coverage Diff @@
## master #3963 +/- ##
==========================================
- Coverage 85.2% 80.77% -4.43%
==========================================
Files 146 146
Lines 7057 7054 -3
Branches 413 412 -1
==========================================
- Hits 6013 5698 -315
- Misses 1044 1356 +312
Continue to review full report at Codecov.
|
dgrove-oss
force-pushed
the
upgrade-kube-client
branch
3 times, most recently
from
f977382
to
dcef2d0
Compare
dgrove-oss
changed the title
|
PG2 3499 🏃 |
|
@dgrove-oss what prevents us from removing the binary here? |
|
I was going to stage it in a separate PR to make it easier to find and revert just that piece in case we wanted the binary back later (or it was being used in openshift in some way I wasn't aware of). I could remove the binary from the Dockerfile in this PR if you'd prefer a single PR. |
|
pushed a second commit to remove the kubectl executable from the Dockerfile. |
| @@ -54,6 +54,11 @@ whisk { | |||
| enabled: false | |||
| port: 3233 | |||
| } | |||
| invoker-node-affinity { | |||
There was a problem hiding this comment.
Should this be "pod-node-affinity" or "user-pod-node-affinity"?
| } | ||
|
|
||
| def rm(key: String, value: String, ensureUnpaused: Boolean = false)(implicit transid: TransactionId): Future[Unit] = { | ||
| runCmd(Seq("delete", "--now", "pod", "-l", s"$key=$value"), config.timeouts.rm).map(_ => ()) |
There was a problem hiding this comment.
Are the timeouts still in use? If not: Delete from the config?
| @@ -17,11 +16,6 @@ rm -f docker-${DOCKER_VERSION}.tgz && \ | |||
| chmod +x /usr/bin/docker && \ | |||
| chmod +x /usr/bin/docker-runc | |||
|
|
|||
| # Install kubernetes client | |||
| /** | ||
| * Configuration for invoker node affinity for user action containers | ||
| */ | ||
| case class KubernetesInvokerNodeAffinity(enabled: Boolean, key: String, value: String) |
There was a problem hiding this comment.
Can we document what key,value are used for ?
|
Thanks for review comments; working on addressing them this morning. |
1. Upgrade to latest released version of the fabric8 Kubernetes client to get access to an implementation of node affinity. Use that implementation to optionally add a scheduling affinity to the pods created for actions to bind them to worker nodes labeled as invoker nodes. 2. implement the container removal operation via the kube rest client instead of via an exec to the kubectl cli. This eliminates the last usage of kubectl in the KubernetesClient and allows the kubectl executable to be removed from the invoker Docker image.
dgrove-oss
force-pushed
the
upgrade-kube-client
branch
from
5f47194
to
e6318b8
Compare
|
addressed review comments & squashed to single commit. Should be ready to merge once CI completes. |
|
PG1: 3250 👍 |
…he#3963) 1. Upgrade to latest released version of the fabric8 Kubernetes client to get access to an implementation of node affinity. Use that implementation to optionally add a scheduling affinity to the pods created for actions to bind them to worker nodes labeled as invoker nodes. 2. implement the container removal operation via the kube rest client instead of via an exec to the kubectl cli. This eliminates the last usage of kubectl in the KubernetesClient and allows the kubectl executable to be removed from the invoker Docker image.
