Impose resource constraints on demos #6
Conversation
There was a problem hiding this comment.
You need to make sure that changes to the docker compose files are consistent.
- The changes to the location of the images here contradicts the ones in Provide a demo of EVerest's existing automated testing capabilities #5, for example
- The images are not the most recent ones
I don't see any other high level issues with this although it is clearly a very preliminary change.
nodered/Dockerfile
Outdated
| @@ -4,6 +4,6 @@ RUN npm install node-red-contrib-ui-actions | |||
| RUN npm install node-red-node-ui-table | |||
| RUN npm install node-red-contrib-ui-level | |||
|
|
|||
| COPY config /config | |||
| COPY --chown=node-red:root config /config | |||
There was a problem hiding this comment.
can you clarify why we need this?
There was a problem hiding this comment.
This is a general improvement that snuck its way into this PR; it isn't necessary for imposing resource constraints.
Currently, if you attempt to modify and redeploy a Node-Red flow in a nodered container, the deploy process fails due to permissions not being appropriately set on the config folder. This change allows the node-red user to apply your configuration changes.
There was a problem hiding this comment.
@drmrd can you expand a bit further? My general assumption was that, if there were a new or modified node-red flow, we would just create a new container with the new flow. When would we modify a node-red flow in an existing container? Is this primarily in a dev environment?
There was a problem hiding this comment.
@shankari currently no changes can be easily made within the node-red container. While these are labeled demo the users may want to experiment with them after getting them running. This change will allow them to easily do so. This has the added advantage that if those changes are committed, i.e. the node-red config, then they are part of the new image. It will assist in the full lifecycle of these docker images and the demo.
There was a problem hiding this comment.
I see. I am fine with including this for now, but I would like us to consider splitting dev and test versions of the containers while working on the refactoring the containers to test resource constraints.
drmrd
force-pushed
the
resource-constraints
branch
from
3910b61 to
2f2b0ae
Compare
drmrd
force-pushed
the
resource-constraints
branch
from
2f2b0ae to
5fcb9ab
Compare
|
@shankari: I have another PR ready for you. 😃 While we're here, there was an interesting discussion at the Weekly Technical Sync on Tuesday that's relevant to improved versions of this demo. Someone had asked about the possibility of extracting EV simulations into their own containers/running them on separate boards, and @corneliusclaussen had an in-depth comprehensive response. The short of it is that
|
README.md
Outdated
| @@ -51,6 +51,10 @@ The use cases supported by the three demos are summarized in conceptual block di | |||
| - 🚨 Two EVSE Charging ⚡: `curl https://raw.githubusercontent.com/everest/everest-demo/main/demo-two-evse.sh | bash` | |||
| - 🚨 E2E Automated Tests ⚡: `curl https://raw.githubusercontent.com/everest/everest-demo/main/demo-automated-testing.sh | bash` | |||
|
|
|||
| #### Demo Notes | |||
| EVerest is designed with embedded applications in mind. To illustrate this, we've imposed maximum CPU usage and RAM constraints of 100% (1 core) and 1024MB, respectively, in each of the demos. Even on modest desktop hardware, these constraints should only result in slightly longer boot times. | |||
| The resource constraints for a demo can be changed in its Docker Compose file (the file downloaded in "STEP 1" below). | |||
There was a problem hiding this comment.
This is now below STEP 1. And since we now have the wrapper shell script, the docker-compose is now tucked away in a temporary location somewhere. To keep the original "one-line demo" concept, and to make it even better than "change them by editing a file", why not have them be passed in as environment variables?
It looks like
CPU='1.0' MEM="1024mb" docker-compose ..
should work
There was a problem hiding this comment.
@shankari: I've extracted the resource limits into the .env file and provided instructions in the README.md for how to customize these values.
|
Since I merged the OCPP change first, the semver needs to be bumped up by 2. |
Signed-off-by: Daniel Moore <[email protected]>
drmrd
force-pushed
the
resource-constraints
branch
from
5fcb9ab to
82303d6
Compare
|
Although I am not sure that the limits should be applied to the CSMS, since it will typically run in the cloud. |
Doh! 😆 Thanks for catching this, @shankari. I've added constraints to the OCPP 1.6J demo in #13. I had intentionally not included constraints on the automated tests demo, since it didn't seem useful to impose constraints on a Pytest run. (If constraints are needed for testing, I'd assume we would impose them from within the Pytest test cases themselves.) The automated testing demo is also called out in the README as the sole exception to the rule on resource constraints. That said, they're easy to add to the automated testing demo if you'd like them there. |
As a first step towards emphasizing that EVerest is built for embedded systems, we're adding maximum CPU usage and RAM constraints of 100% (1 core) and 1 GB, respectively, in each demo.
Note that EVerest can run under much more aggressive resource constraints in realistic conditions. See for example this discussion from the mailing list for more details. The
managercontainers in our demos require more CPU and memory resources at present due to the inclusion of unoptimized test and simulation modules in these containers. Follow-up work will include a demo where the test and simulation code is isolated into a separate image to provide greater realism.