Skip to content
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

Compose v2: don't emit waiting events for healthy containers #799

Closed
wants to merge 1 commit into from

Conversation

clinta
Copy link

@clinta clinta commented Feb 15, 2024

The output of the compose command that is used for parsing events can contain multiple events for containers with health checks. With one event status Waiting then a following event with the status Healthy. This causes the module to erroneously report changes.

This change ensures only the last event for each container is reported.

SUMMARY

docker compose --ansi never --progress plain --project-directory <dir> --detach --no-color --quiet-pull -- will emit multiple events for containers with health checks. For example:

# docker compose --ansi never --progress plain --project-directory /srv/netbox-docker up --detach --no-color --quiet-pull --
 Container netbox-docker-redis-cache-1  Running
 Container netbox-docker-postgres-1  Running
 Container netbox-docker-redis-1  Running
 Container netbox-docker-netbox-1  Running
 Container netbox-docker-netbox-housekeeping-1  Running
 Container netbox-docker-netbox-worker-1  Running
 Container netbox-docker-netbox-1  Waiting
 Container netbox-docker-netbox-1  Waiting
 Container netbox-docker-netbox-1  Healthy
 Container netbox-docker-netbox-1  Healthy

This causes the module to report changes because it reports 2 containers with the Waiting status. The last four events are all for the same container, and only the last status should be reported.

To fix this, when a "Healthy" event is processed, any "Waiting" events for that same container are removed from the events list.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

community.docker.docker_compose_v2

ADDITIONAL INFORMATION

Use ansible to manage a compose definition with health checks.


Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
The output of the compose command that is used for parsing events can contain multiple events for containers with health checks.  With one event status Waiting then a following event with the status Healthy. This causes the module to erroneously report changes.

This change removes waiting events for healthy containers
@felixfontein felixfontein changed the title Don't emit waiting events for healthy containers Compose v2: don't emit waiting events for healthy containers Feb 16, 2024
@felixfontein felixfontein added the docker-compose-v2 Docker Compose v2 label Feb 16, 2024
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

Whether a change happened is detected by the has_changes function, and events are converted to actions by the extract_actions function. Both rely on DOCKER_STATUS_WORKING to determine whether an event is a change or not (Waiting is on that list).

Simply removing Waiting from DOCKER_STATUS_WORKING should fix this problem. It should be added to another list DOCKER_STATUS_OTHER though, which should be added to DOCKER_STATUS, to make sure that parsing Waiting isn't changed.

Finally please note that you need a changelog fragment.

@@ -303,6 +303,8 @@ def parse_events(stderr, dry_run=False, warn_function=None):
warn_missing_dry_run_prefix = True
event = _extract_event(line)
if event is not None:
if event.status == "Healthy":
events = [e for e in events if not (e.resource_id == event.resource_id and e.status == "Waiting")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not remove events here. This function should return all events it finds and not contain any custom logic.

@clinta
Copy link
Author

clinta commented Feb 16, 2024

Removing Waiting from DOCKER_STATUS_WORKING will have the effect of not returning changes if the last status for the container is still Waiting. I'm not sure if this can happen, I'm guessing it can based on timeout values.

This seems like a potentially undesirable side-effect. Which is why I only make changes if Waiting is followed by a Heathy event for that container.

Is it okay to not return changes if the last event for a container is Waiting?

@felixfontein
Copy link
Collaborator

If it's simply waiting, then no change happened? I don't see why it should report changed when encountering Waiting at all.

@felixfontein
Copy link
Collaborator

I created #804 to simply stop treating Waiting as an action.

@felixfontein
Copy link
Collaborator

Closing since #804 has been merged. Thanks for getting this started!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker-compose-v2 Docker Compose v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants