Support replicaset and deployment info #342
Conversation
| @@ -6,7 +6,7 @@ image: | |||
| pullPolicy: IfNotPresent | |||
| sideCartContainerName: kubectl | |||
| sideCartContainerArgs: "[ \"proxy\", \"-p\", \"8001\" ]" | |||
| sideCartContainerImage: gcr.io/google_containers/kubectl:v0.18.0-350-gfb3305edcf6c1a | |||
| sideCartContainerImage: lachlanevenson/k8s-kubectl:v1.9.10 | |||
There was a problem hiding this comment.
What is this source? Can we just trust it? Nothing more Google official?
There was a problem hiding this comment.
The google one has not been updated since january 2017.
This is the source: https://github.com/lachie83/k8s-kubectl
It has most stars on github and most pulls on dockerhub from the images I found
| logger.warn('Could not retrive deployment information, verify your RBAC settings'); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Does not lint complain about this extra empty line?
There was a problem hiding this comment.
Nope, but I removed it anyways ;)
| @@ -54,8 +75,15 @@ class KubernetesClient { | |||
| const ip = pod.status.podIP; | |||
| const engine = { networks: [{ ip }], labels }; | |||
| const key = pod.metadata.uid; | |||
| const replicaSet = replicaMap.get(pod.metadata.ownerReferences[0].uid); | |||
There was a problem hiding this comment.
So if it does not exist, does it become undefined?
There simply is no property in the response, or the key is is there but with value undefined?
There was a problem hiding this comment.
No key at all
| * pod: | ||
| * type: object | ||
| * description: Pod information in verbatim format as returned by the Kubernetes API. | ||
| * replicaSet: |
There was a problem hiding this comment.
How do we document that this key might not exist? Or, will it always exist but with possible value undefined? Should we clarify this?
There was a problem hiding this comment.
I think the key will not exist, but I will verify and add that info in the docummentation
There was a problem hiding this comment.
Verified that the key does not exist if the value is undefined.
The values are not marked as required in the swagger spec, so they are not expected to always exist.
| replicaMap.set(item.metadata.uid, item); | ||
| }); | ||
| } catch (error) { | ||
| logger.warn('Could not retrive replicaset information, verify your RBAC settings'); |
There was a problem hiding this comment.
Add unit test that covers this case and verify expected output.
| deploymentMap.set(item.metadata.uid, item); | ||
| }); | ||
| } catch (error) { | ||
| logger.warn('Could not retrive deployment information, verify your RBAC settings'); |
There was a problem hiding this comment.
Add unit test that covers this case and verify expected output.
| @@ -40,6 +40,26 @@ class KubernetesClient { | |||
| * @returns {Promise<EngineContainerSpec[]>} A promise to a list of engine container specs. | |||
| */ | |||
| static async listEngines() { | |||
| const replicaMap = new Map(); | |||
| try { | |||
There was a problem hiding this comment.
Any reason why we're doing these sequentially? AFAIK they don't use any data from previous calls and can be parallelized?
E.g.
const replicas = kubeHttpGet('/apis/apps/v1/replicasets');
const deployments = kubeHttpGet('/apis/apps/v1/deployments');
const result = await Promise.all([replicas, deployments]);
// result[0] = replicas, etc...
There was a problem hiding this comment.
No you are right, we should definitely parallelize them :)
| replicaMap.set(item.metadata.uid, item); | ||
| }); | ||
| } catch (error) { | ||
| // Do nothing. |
There was a problem hiding this comment.
We should probably log something if this fails, or?
There was a problem hiding this comment.
I tried logging if it fails, since this will be the same on every "round" it gets very spammy.
…he replicaset or deployment endpoints
Supporting replicaset and deployment info.
Also changing sidecar container since the one we had before was very old and did not support newer APIs