-
Notifications
You must be signed in to change notification settings - Fork 24
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
🤖 Sync from open-cluster-management-io/governance-policy-propagator: #127 #435
Conversation
The new `IsForPolicyOrPolicySet` may be very slightly faster than the previous implementation. The `GetPoliciesInPlacementBinding` function is from duplicated code. The removal of a `DeepCopy()` call may improve performance. Signed-off-by: Justin Kulikauskas <[email protected]> (cherry picked from commit fdc3118)
Previously, the `common` package had separate functions to get the decisions from a PlacementBinding depending on whether it was bound to a Placement or a PlacementRule. But every caller would need to use both, so that was unnecessary. Now, there is a unified `GetDecisions` func. Similarly, the `propagator` had separate functions to find the policy placements (not to be confused with Placements...), and the de- duplicated logic was brought to the `getPolicyPlacementDecisions` func more directly. Signed-off-by: Justin Kulikauskas <[email protected]> (cherry picked from commit 4178f22)
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: magic-mirror-bot[bot] The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
SonarCloud Quality Gate failed. 0 Bugs 91.2% Coverage Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
} | ||
} | ||
} | ||
result = append(result, common.GetPoliciesInPlacementBinding(ctx, c, &pb)...) |
Check warning
Code scanning / SonarCloud
<!--SONAR_ISSUE_KEY:AYqPSsPE3PkE-TAvSFWh-->Implicit memory aliasing in for loop. <p>See more on <a href="https://sonarcloud.io/project/issues?id=open-cluster-management_governance-policy-propagator&issues=AYqPSsPE3PkE-TAvSFWh&open=AYqPSsPE3PkE-TAvSFWh&pullRequest=435">SonarCloud</a></p>
foundInDecisions := false | ||
|
||
plcDecisions, plcPlacements, err := r.getPolicyPlacementDecisions(instance, pb) | ||
plcDecisions, plcPlacements, err := r.getPolicyPlacementDecisions(instance, &pb) |
Check warning
Code scanning / SonarCloud
<!--SONAR_ISSUE_KEY:AYqPSsTw3PkE-TAvSFWi-->Implicit memory aliasing in for loop. <p>See more on <a href="https://sonarcloud.io/project/issues?id=open-cluster-management_governance-policy-propagator&issues=AYqPSsTw3PkE-TAvSFWi&open=AYqPSsTw3PkE-TAvSFWi&pullRequest=435">SonarCloud</a></p>
continue | ||
} | ||
|
||
plcDecisions, plcPlacements, err := r.getPolicyPlacementDecisions(instance, pb) | ||
plcDecisions, plcPlacements, err := r.getPolicyPlacementDecisions(instance, &pb) |
Check warning
Code scanning / SonarCloud
<!--SONAR_ISSUE_KEY:AYqPSsTw3PkE-TAvSFWj-->Implicit memory aliasing in for loop. <p>See more on <a href="https://sonarcloud.io/project/issues?id=open-cluster-management_governance-policy-propagator&issues=AYqPSsTw3PkE-TAvSFWj&open=AYqPSsTw3PkE-TAvSFWj&pullRequest=435">SonarCloud</a></p>
} | ||
|
||
result = append(result, common.GetPoliciesInPlacementBinding(ctx, c, &pb)...) |
Check warning
Code scanning / SonarCloud
<!--SONAR_ISSUE_KEY:AYqPSsUU3PkE-TAvSFWk-->Implicit memory aliasing in for loop. <p>See more on <a href="https://sonarcloud.io/project/issues?id=open-cluster-management_governance-policy-propagator&issues=AYqPSsUU3PkE-TAvSFWk&open=AYqPSsUU3PkE-TAvSFWk&pullRequest=435">SonarCloud</a></p>
/cherry-pick release-2.9 |
@JustinKuli: new pull request created: #460 In 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/test-infra repository. |
Syncing the following PRs:
getPolicyPlacementDecisions
open-cluster-management-io/governance-policy-propagator#127