Fix HasDiff returning false when OwnerIds or ParentIds actually differ - #7137
Merged
khanhtc1202 merged 3 commits intoAug 22, 2026
Conversation
HasDiff is supposed to tell the caller whether a Kubernetes resource's state changed, so piped knows whether to report an update. For every other field (ApiVersion, Namespace, HealthStatus, HealthDescription) a mismatch correctly returns true. For OwnerIds and ParentIds, a mismatch returns false instead - the exact opposite of what the rest of the function does. Practical effect: when a resource's owner or parent changes (for example a Pod getting picked up by a different ReplicaSet during a rolling update), HasDiff says nothing changed, so the update is never reported and the stored live state keeps showing the old relationship. Added a table test covering this case; it fails on the current code and passes with the fix. Signed-off-by: Vipul Subhash Pandey <vipulpandey7917@gmail.com>
✅ Deploy Preview for pipecd-site canceled.
|
khanhtc1202
reviewed
Aug 22, 2026
Member
|
Good catch, I left a comment 👍 |
khanhtc1202
approved these changes
Aug 22, 2026
khanhtc1202
left a comment
khanhtc1202
left a comment
Member
There was a problem hiding this comment.
Good catch, I almost forgot about this line 😅
Contributor
Author
|
Hi Khanh sir, thanks for double-checking the sort order instead of just taking the fix at face value. Appreciate you tracing it back to state.go rather than trusting the comparison blind. Good to have this one closed out, especially since it had been sitting unchanged since 2020. |
srinivasr
pushed a commit
to srinivasr/pipecd
that referenced
this pull request
Aug 24, 2026
pipe-cd#7137) HasDiff is supposed to tell the caller whether a Kubernetes resource's state changed, so piped knows whether to report an update. For every other field (ApiVersion, Namespace, HealthStatus, HealthDescription) a mismatch correctly returns true. For OwnerIds and ParentIds, a mismatch returns false instead - the exact opposite of what the rest of the function does. Practical effect: when a resource's owner or parent changes (for example a Pod getting picked up by a different ReplicaSet during a rolling update), HasDiff says nothing changed, so the update is never reported and the stored live state keeps showing the old relationship. Added a table test covering this case; it fails on the current code and passes with the fix. Signed-off-by: Vipul Subhash Pandey <vipulpandey7917@gmail.com> Co-authored-by: Khanh Tran <32532742+khanhtc1202@users.noreply.github.com>
srinivasr
added a commit
to srinivasr/pipecd
that referenced
this pull request
Aug 24, 2026
The test added in pipe-cd#7137 passes struct values to HasDiff; adapt it to the pointer-receiver signature used by this branch, following the same index-loop style as the other model tests here. Signed-off-by: srinivasr <sriniv4sreddy@gmail.com>
srinivasr
added a commit
to srinivasr/pipecd
that referenced
this pull request
Aug 24, 2026
Cover the OwnerIds/ParentIds mismatch cases fixed on master (pipe-cd#7137), written against the pointer-receiver signature used by this branch. Signed-off-by: srinivasr <sriniv4sreddy@gmail.com>
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
2 participants
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.
What this PR does:
Fixes
KubernetesResourceState.HasDiffso a mismatch inOwnerIdsorParentIdsreturnstrueinstead offalse.(both loops, same one-line change)
Why we need it:
Every other check in this function returns
trueon a mismatch — that's the whole point ofHasDiff. ForOwnerIds/ParentIdsit did the opposite, so when a resource's owner or parent changes, piped never reports it as an update. Details in #7135.How I tested it:
Added
TestKubernetesResourceState_HasDiff. Fails on current code:Passes with the fix.
go test ./pkg/model/...passes,go build ./...is clean.Which issue(s) this PR fixes:
Fixes #7135
Does this PR introduce a user-facing change?: