green-coding-solutions / green-metrics-tool

Measure energy and carbon consumption of software
https://metrics.green-coding.io
GNU Affero General Public License v3.0
166 stars 22 forks source link

Improve Healthcheck with start-interval parameter #783

Closed davidkopp closed 4 months ago

davidkopp commented 4 months ago

As discussed in #781 this PR implements the healthcheck parameter start-interval. In addition to that the logging around the healthcheck functionality is improved.

After some testing I came to the conclusion, that it is sufficient to just check every second what the health of the dependent container is without any additional logic. There are 3 possible outcomes:

As proposed in #781 the default max waiting time should be increased (20 seconds is too short).

A compose file using the start-interval parameter can be found here. Currently, I have defined the healthcheck configurations with the following parameters:

start_period: "30s"
start_interval: "1s"
interval: "1h"
ArneTR commented 4 months ago

The implementation looks sound.

It would be quite nice to see a test that actually validates if the feature is working as expected.

In my opinion the follow test layout would be helpful:

A container that matches this specification needs to set a healthcheck command, otherwise it will automatically be detected as healthy as soon as it has started.

Here is a sample Dockerfile that would create such a container:

# Use an official lightweight image as a base
FROM alpine:latest

# Add a startup script
COPY start.sh /start.sh

# Make the script executable
RUN chmod +x /start.sh

# Add a health check
HEALTHCHECK CMD [ "sh", "-c", "ps aux | grep '[s]leep 5'" ]

# Run the startup script
CMD ["/start.sh"]

start.sh

#!/bin/sh
echo "Starting up..."
sleep 5
echo "Finished startup."

This code is untested but the concept should work ... maybe with minor tweaks.

Summary

If you can implement a test, maybe even a better one if you can think of one, it would be very helpful.

If you cannot find the time we will try to take this on in the next two weeks.

davidkopp commented 4 months ago

I have now improved the tests for the healthcheck functionality in general. Thanks for your explanations for a good test setup. There were some changes necessary, so please have a look if I have covered what you wanted.

ArneTR commented 4 months ago

Running the tests and giving it a look now

ArneTR commented 4 months ago

Looks quite nice! Made a review and had a small change request

davidkopp commented 4 months ago

Added another test with start_interval defined, however without start_period -> no healthcheck gets executed. If you want to use start_interval, it's necessary to also define start_period.

ArneTR commented 4 months ago

sweet. thank you again so much for contributing!

Merged!!