-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(containers): add UI to view and edit Docker HEALTHCHECK configuration values on containers #4407
base: develop
Are you sure you want to change the base?
Conversation
…ation values on containers
*Ruff* 🐶 The test environment for this pull request has been destroyed 💥 This may have happened explicitly via a command, because the environment expired, or because the pull request was closed. What is this?Pull Dog is a GitHub app that makes test environments for your pull requests using Docker, from a Visit our website to learn more. Commands
TroubleshootingNeed help? Don't hesitate to file an issue in our repository Configuration {
"isLazy": false,
"dockerComposeYmlFilePaths": [
"docker-compose.pull-dog.yml"
],
"buildArguments": {
"PORTAINER_TAG": "pr4407-linux-amd64"
},
"label": "test-instance-available",
"expiry": "1.00:00:00",
"conversationMode": "singleComment"
} Trace ID |
}) | ||
.catch(function error(err) { | ||
Notifications.error('Failure', err, 'Unable to retrieve container'); | ||
}); | ||
} | ||
|
||
function loadFromContainerLogging(config) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note this function is moved before loadFromContainerSpec() to avoid IDE warnings about the undefined function. Works out at runtime, but things should be declared before used in JS (and C...and life in general...).
/azp run |
/pull-dog up |
Note that the remaining "conflicts" are where there are added lines, and not actually conflicts. The worst of which is this import in the createContainerController.js which seems to be replacing a blank line. All of the other conflicts are new code replacing empty blocks.
|
Another refresh from origin/develop, and all conflicts have been resolved. I cannot clear the "has conflicts" label, but there are none in this PR. Please review and merge to enable Docker HEALTHCHECK directive editing in Portainer. |
This issue is probably better tied to #3321, which is closed, but mentions that the solution belongs with #3572 instead, thus the branch named after that issue.
When launching a Docker container with a HEALTHCHECK in the container or added at the command-line, the values are not visible in Portainer. Previous enhancements to the container views show the health status while the container is starting or running.
For example, this line in a container's Dockerfile will trigger a curl of the root of a website, issuing an error if content isn't returned (it's a short example, which could barf out a lot of content, so better examples are in order). It will pause for 30 seconds to allow the container to finish starting, wait for 1 second at each request, and repeat every 5 minutes, waiting for 5 failures to report the container unhealthy.
This PR provides a simple view of the health-check values in the container details, next to the health check status data previously displayed. When creating or editing (duplicating) a container, the "Advanced container settings" area has a Healtcheck tab, which allows disabling or editing these fields, with some validation for the intervals, to allow the "5m" shorthand entry.
Changes to the Docker views are complete to the documentation found at https://docs.docker.com/engine/reference/builder/#healthcheck and information found in the config.v2.json file in the container on the running system. Additionally, disabling the health check will allow maintaining the other settings between edits.
The npm module https://www.npmjs.com/package/simple-duration has been added to simplify the date formatting and conversion between Docker's nanoseconds and a human-readable value. This code does limit the resolution to ms, but if that's found to be important, it's a small change to allow passing µs or ns as well, as are allowed by the simple duration module.