[ISSUE-102] Add support SUM on CPU and Memory

[jameskim0987]

Resolves #102

Changes made in this PR:

1. Implement logic for adding support SUM on CPU and Memory resources
2. Implement helper functions such as convertToMilliCPU(), convertMilliCPUToStandard, convertMemoryToBytes, convertBytesToMemory for handling string and int manipulation for summing K8 CPU and Memory values ( which needs unit conversion ).
3. add unit tests
4. add SUM cpu and memory query example in docs

Notes:

1. Manually tested with cyphernetes shell -A, trying these commands:

   
   MATCH (p:Pod) RETURN SUM{p.spec.containers[0].resources.limits.cpu};
   MATCH (p:Pod) RETURN SUM{p.spec.containers[0].resources.requests.cpu};
   MATCH (p:Pod) RETURN SUM{p.spec.containers[0].resources.limits.memory};
   MATCH (p:Pod) RETURN SUM{p.spec.containers[0].resources.requests.memory};

MATCH (p:Pod) RETURN SUM{p.spec.containers[].resources.limits.cpu}; MATCH (p:Pod) RETURN SUM{p.spec.containers[].resources.requests.cpu}; MATCH (p:Pod) RETURN SUM{p.spec.containers[].resources.limits.memory}; MATCH (p:Pod) RETURN SUM{p.spec.containers[].resources.requests.memory};

and compared the summed values with `kube-capacity --pods` ( https://github.com/robscott/kube-capacity ) 

[jameskim0987]

Hi @AvitalTamir, this PR took longer than I expected due to time spent learning about lex and yacc, the grammar structure, and the existing codebase to think through about the implementation. Currently, I think I have the general logic down for using SUM for cpu and memory resources. I was able to verify the summed values on my local minikube cluster. Let me know how it looks so far and what can be changed / improved!

As a follow up, I had a question about writing tests for this implementation. Since this implementation is applied on resources that have cpu and mem requests and/or limits, would it be good to create something like dummy resources with cpu and memory values and execute the SUM query on those? Any pointers or guidance would be much appreciated!

[AvitalTamir]

Hey @jameskim0987 first of all this is brilliant! Just did this and I'm blown away, this is going to be very useful:

MATCH (d:deployment {name:"auth-service"})->(s:svc)->(p:pod) RETURN SUM { p.spec.containers[0].resources.requests.cpu } AS totalCPUReq, SUM {p.spec.containers[0].resources.requests.memory } AS totalMemReq;

{
  "aggregate": {
    "totalCPUReq": "75m",
    "totalMemReq": "3.0Mi"
  },
  "p": [
    {
      "name": "auth-service-5d96db584d-2d5l7"
    },
    {
      "name": "auth-service-5d96db584d-ctvrj"
    },
    {
      "name": "auth-service-5d96db584d-x4bn9"
    }
  ]
}

Query executed in 17.813125ms

There's an issue I spot the fact we assume the value of cpu or memory is always a string, but if I do this (notice the [*] in containers instead of [0]):

MATCH (d:deployment {name:"auth-service"})->(s:svc)->(p:pod) RETURN SUM { p.spec.containers[*].resources.requests.cpu } AS totalCPUReq

then we get:

Error >> error executing query >> unsupported type for SUM: slice

If we ask to return some value under a [*] path, then the return value will be a slice ([ "25m", "50m", "25m"... ]). I think we should definitely support that because a common use case will be "I want the total cpu req/lim for all the containers of this deployment".

Otherwise, this is looking good! I'm looking forward to having this merged 😁

[AvitalTamir]

Re. tests: I think I would start with unit testing the actual summing functionality across all different variations (cpu, memory, strings/slices). We could add integration tests with envtest later or add an e2e but I think most of this is already covered by existing integration tests + the operator e2e test - So would take care first of simple unit tests to cover the new sum functions. We can then look at the coverage report and see if anything is missing.

[jameskim0987]

Hi @AvitalTamir, thank you for your feedback! This is valuable input. I have been working on getting that logic down and will push the changes shortly. I will also add unit tests for the functions created.

[AvitalTamir]

Sounds good, let me know anything you need!

[jameskim0987]

@AvitalTamir I added the logic for slices of results, did some minor clean ups, and added unit tests for the helper functions. PR is ready for review!

[jameskim0987]

@AvitalTamir I'm always happy to contribute!