-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fix template sync set to Nomcompliant when context cancel #127
Fix template sync set to Nomcompliant when context cancel #127
Conversation
@@ -213,3 +223,143 @@ func TestHasDuplicateNames(t *testing.T) { | |||
t.Fatal("Duplicate name not detected") | |||
} | |||
} | |||
|
|||
func TestContextCancel(t *testing.T) { |
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.
A Simple workaround but Not a simple test. Do we need this?
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.
Are you asking if we need the test?
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.
Yes
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.
I think the test is a bit much for this, so I'm okay with removing it.
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.
This test cannot catcheObject, err := res.Get(ctx, tName, metav1.GetOptions{})
line 534 actually
eObject, err := res.Get(ctx, tName, metav1.GetOptions{}) |
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.
But I used this test to verify in my local. I could control the cancel ctx point
5d93ac0
to
a204d67
Compare
Bug: If the template-sync is shutting down and it's in the middle of a reconcile request, it can set the policy to noncompliant with this Ref: https://issues.redhat.com/browse/ACM-10402 Signed-off-by: Yi Rae Kim <[email protected]>
a204d67
to
e03eaaf
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mprahl, yiraeChristineKim 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 |
6a8baf3
into
open-cluster-management-io:main
Bug: If the template-sync is shutting down and it's in the middle of a reconcile request, it can set the policy to noncompliant with this
Ref: https://issues.redhat.com/browse/ACM-10402