[ci-operator] propagate binary-input dependencies so images are rebuilt when any of their binaries changes#5032
[ci-operator] propagate binary-input dependencies so images are rebuilt when any of their binaries changes#5032droslean wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
@droslean: GitHub didn't allow me to request PR reviews from the following users: testplatform. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
WalkthroughDetector.AffectedTools now computes additional affected tools based on configured image "bin" inputs via a new helper getAffectedToolsByBinaryInputs, and unions those results with existing package/image-derived affected sets. Unit tests for the new helper were added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
faea6e2 to
b5e0fe5
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: droslean The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…lt when any of their binaries changes Signed-off-by: Nikolaos Moraitis <nmoraiti@redhat.com>
b5e0fe5 to
64c3054
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/tool-detector/detector.go`:
- Around line 79-80: The code is incorrectly seeding
getAffectedToolsByBinaryInputs with affectedByImageChanges which fans out
bin-image-only edits; change calls to d.getAffectedToolsByBinaryInputs so they
only receive the set of tools whose binary inputs actually changed (i.e., the
inputs["bin"] diff/changeset or an explicit affectedByBinaryChanges value)
instead of affectedByImageChanges, and remove the union with
affectedByImageChanges where used (also update the other occurrences that call
getAffectedToolsByBinaryInputs at the other two sites). Ensure the union/merge
only happens when a real binary-input change is detected, not for
image-context-only changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e286298-6528-456a-a26b-7fbf879f5814
📒 Files selected for processing (2)
pkg/tool-detector/detector.gopkg/tool-detector/detector_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/tool-detector/detector_test.go
| combined := affectedByImageChanges.Union(d.getAffectedToolsByBinaryInputs(affectedByImageChanges)) | ||
| return combined, nil |
There was a problem hiding this comment.
Don't fan out inputs["bin"] dependencies from image-context changes.
getAffectedToolsByBinaryInputs models consumers of binaries copied from the bin image. Seeding it with affectedByImageChanges means a Dockerfile/context-only edit can also rebuild downstream bundle images whose copied binary never changed, which widens the rebuild set beyond the PR goal.
Possible fix
if len(goFiles) == 0 {
- combined := affectedByImageChanges.Union(d.getAffectedToolsByBinaryInputs(affectedByImageChanges))
- return combined, nil
+ return affectedByImageChanges, nil
}
...
if len(changedPackages) == 0 {
- combined := affectedByImageChanges.Union(d.getAffectedToolsByBinaryInputs(affectedByImageChanges))
- return combined, nil
+ return affectedByImageChanges, nil
}
...
combined := affectedByPackages.Union(affectedByImageChanges)
-affectedByBinaryInputs := d.getAffectedToolsByBinaryInputs(combined)
+affectedByBinaryInputs := d.getAffectedToolsByBinaryInputs(affectedByPackages)Also applies to: 90-91, 101-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/tool-detector/detector.go` around lines 79 - 80, The code is incorrectly
seeding getAffectedToolsByBinaryInputs with affectedByImageChanges which fans
out bin-image-only edits; change calls to d.getAffectedToolsByBinaryInputs so
they only receive the set of tools whose binary inputs actually changed (i.e.,
the inputs["bin"] diff/changeset or an explicit affectedByBinaryChanges value)
instead of affectedByImageChanges, and remove the union with
affectedByImageChanges where used (also update the other occurrences that call
getAffectedToolsByBinaryInputs at the other two sites). Ensure the union/merge
only happens when a real binary-input change is detected, not for
image-context-only changes.
|
Scheduling tests matching the |
|
@droslean: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
/cc @testplatform