-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
wait for group cache in templateinstance tests #17009
wait for group cache in templateinstance tests #17009
Conversation
So is a fair summary:
thanks |
@gabemontero I think there were three issues causing flakes:
2a 2b are already on the merge queue at #16979, this just builds upon that commit. |
On Mon, Oct 23, 2017 at 4:28 PM, Jim Minter ***@***.***> wrote:
@gabemontero <https://github.com/gabemontero> I think there were three
issues causing flakes:
1. not waiting for group cache to sync after changing group
memberships (affected two tests, causing permissions problems)
2a) not waiting for cascading deletion to complete before trying to
reinstantiate an identical object (caused rare unexpected AlreadyExists
errors)
2b) GC sometimes reordered json fields when removing the finaliser on
the templateinstance, causing the update to be rejected by the api server,
causing knock-on strangeness to happen on cascaded deletion (hence changes
to validation logic)
2a 2b are already on the merge queue at #16979
<#16979>, this just builds upon
that commit.
gotcha ... ok that break down helps after revisiting the changes - thanks
… —
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17009 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADbadEou_XvaNhWM7ZLondQmxcmfA49Aks5svPbRgaJpZM4QDZP3>
.
|
/test origin-ut |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, jim-minter The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Automatic merge from submit-queue (batch tested with PRs 17007, 17003, 17001, 17009). |
fixes #16654
(builds on #16979)
@gabemontero fyi