-
Notifications
You must be signed in to change notification settings - Fork 68
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
add watch logic for addondeployment #1289
Conversation
@coleenquadros there's an e2e test failing. Can you have a look, please? |
a23276a
to
b0746c9
Compare
/test test-unit |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Would love to achieve that 70% test coverage that SonarCloud is asking for though. It's fine if it's not feasible because we can't add 100% coverage of the proxy scenarios.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: coleenquadros, douglascamata The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Coleen Iona Quadros <[email protected]>
Signed-off-by: Coleen Iona Quadros <[email protected]>
Signed-off-by: Coleen Iona Quadros <[email protected]>
017be5d
to
cc9b70a
Compare
New changes are detected. LGTM label has been removed. |
67947f7
to
29a5c73
Compare
Signed-off-by: Coleen Iona Quadros <[email protected]>
Signed-off-by: Coleen Iona Quadros <[email protected]>
29a5c73
to
5dc9574
Compare
/test test-unit |
Signed-off-by: Coleen Iona Quadros <[email protected]>
Signed-off-by: Coleen Iona Quadros <[email protected]>
Quality Gate failedFailed conditions 47.6% Coverage on New Code (required ≥ 70%) |
@coleenquadros: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
@subbarao-meduri Testing working as expected on QA test bed - changed the proxy setting values in addondeploymentconfig and the placement controller reconciled to pick up the new proxy settings |
* add watch logic for addondeployment Signed-off-by: Coleen Iona Quadros <[email protected]> * make sonarcloud happy Signed-off-by: Coleen Iona Quadros <[email protected]> * lint Signed-off-by: Coleen Iona Quadros <[email protected]> * refactor predicate Signed-off-by: Coleen Iona Quadros <[email protected]> * lint Signed-off-by: Coleen Iona Quadros <[email protected]> * fix test case Signed-off-by: Coleen Iona Quadros <[email protected]> * refactor Signed-off-by: Coleen Iona Quadros <[email protected]> --------- Signed-off-by: Coleen Iona Quadros <[email protected]>
* add watch logic for addondeployment Signed-off-by: Coleen Iona Quadros <[email protected]> * make sonarcloud happy Signed-off-by: Coleen Iona Quadros <[email protected]> * lint Signed-off-by: Coleen Iona Quadros <[email protected]> * refactor predicate Signed-off-by: Coleen Iona Quadros <[email protected]> * lint Signed-off-by: Coleen Iona Quadros <[email protected]> * fix test case Signed-off-by: Coleen Iona Quadros <[email protected]> * refactor Signed-off-by: Coleen Iona Quadros <[email protected]> --------- Signed-off-by: Coleen Iona Quadros <[email protected]>
* add watch logic for addondeployment * make sonarcloud happy * lint * refactor predicate * lint * fix test case * refactor --------- Signed-off-by: Coleen Iona Quadros <[email protected]> Co-authored-by: Coleen Iona Quadros <[email protected]>
https://issues.redhat.com/browse/RHOBS-949
https://issues.redhat.com/browse/ACM-8312
From what I understand is that the placement controller has no logic to watch over the changes to addondeployment config. So any update to it will not be picked up by managerclusteraddon and clustermanagementaddon which are watched already but they only hold a ref to addondeploymentconfig and not the actual config.
The PR adds the watch logic in the placement controller to monitor changes in addondeployment config and sets up the request to reconcile and also apply change to all managed clusters