feat(api): correlate failures by container name across replicas - #77
Merged
Merged
Conversation
allReplicasFailing worked at pod level, requiring every replica to be failing in some way. That conflates a systematic cause, the same container broken in every replica because of a bad image or a missing Secret, with unrelated failures that happen to coincide. Adds failureCorrelation with Pod, the existing behaviour and the default, and Container, which additionally requires the same container name to be the failing one everywhere. Since the operator never scales anything back up, a stricter option is worth having. A new field rather than an enum on allReplicasFailing, which would have changed the type of a released field. The three per-kind branches ran identical sibling loops, so they now share one evaluator and cannot drift apart. isMoreRestrictive gains the new dimension: container correlation acts in fewer situations, so a policy using it ranks below one that does not, keeping the winner selection a total order. Closes #70 Signed-off-by: Simon Lauger <simon@lauger.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
allReplicasFailingworked at pod level: every replica had to be failing in some way. That conflates two different situations. The same container broken in every replica points at a systematic cause, a bad image or a missing Secret. Different containers failing in different pods may just be unrelated problems that happen to coincide.Adds
failureCorrelationwith two values:Pod, the existing behaviour and the default, andContainer, which additionally requires the same container name to be the failing one in every replica. Since the operator never scales anything back up, having a stricter option available is worth the field.A new field rather than an enum on
allReplicasFailing, which would have changed the type of a released field and needed a v1alpha2.Two things fell out of the implementation:
allReplicasFailingran identical sibling loops. They now share one evaluator, so they cannot drift apart. That was already a latent risk before this change.isMoreRestrictiveneeded the new dimension. Container correlation acts in fewer situations, so a policy using it ranks below one that does not. Without this the winner selection would no longer be a total order and overlapping policies could resolve inconsistently.Worth being plain about the value: this is a caution knob, not a fix. If all replicas are broken, pod correlation is a perfectly defensible signal. The container mode exists for people who would rather under-act than over-act, which given the absence of a scale-up path is a reasonable preference.
Closes #70
Test plan
make cipasses; coverage rose from 64.1 to 65.8 percent.