fix clusterprovision creation race #518
Conversation
openshift-ci-robot
added
size/L
|
Working through trying to understand this but initial thought, could you update the git commit msg on the second commit with some heavier details on what this is doing, how it works and why it's needed in this specific situation. |
| r.expectations.ExpectCreations(types.NamespacedName{Namespace: cd.Namespace, Name: cd.Name}.String(), 1) | ||
| if err := r.Create(context.TODO(), provision); err != nil { | ||
| cdLog.WithError(err).Error("could not create provision") | ||
| r.expectations.CreationObserved(types.NamespacedName{Namespace: cd.Namespace, Name: cd.Name}.String()) |
There was a problem hiding this comment.
Something that's confusing me a little bit here, it looks like the intent here is to expect the creation of the cluster provision, but the expectation itself is using the cluster deployment's name. Should this be the provision's name? Or is that an issue with the random component of their names.
There was a problem hiding this comment.
The expectations are set up to expect a creation on behalf of the clusterdeployment. The key is always the resource being reconciled, and not the resource being created.
Do you mean the first commit? Or do you mean why I need to vendor in the package that contains the controller expectations code? |
staebler
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
I need to copy the controller expectations code from |
|
+1 copying sounds good. I meant the commit with your code :) Whichever it was. |
openshift-ci-robot
added
size/L
Sure. I will improve on the commit message. |
openshift-ci-robot
added
size/XL
staebler
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
This is great, thank you. /lgtm |
openshift-ci-robot
added
lgtm
create multiple overlapping clusterprovisions. There is currently a race condition where the controller will create a second clusterprovision if the first clusterprovision has not been added to the local cache by the time the clusterdeployment is reconciled again after creating the first clusterprovision. To prevent this, these changes use expectations to block the controller from creating the second clusterprovision unitl the controller sees that the first clusterprovision has been created. When the clusterDeployment controller creates a clusterprovision, the controller will increase the number of creation expectations for the clusterdeployment. When the controller sees an add event for the clusterprovision, the controller will reduce the number of creation expectations for the clusterdeployment. During a reconcile loop, the controller will not look at clusterprovisions while there are outstanding creation expectations for the clusterdeployment being reconciled.
openshift-ci-robot
added
size/L
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgoodwin, staebler 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 |
... and security groups. Pulling in openshift/installer@55630a304 (pkg/destroy/aws: Delete security groups by VPC, 2019-08-13, openshift/installer#2214). Generated with: $ sed -i 's/ae2baf820f22b9bf1ed40932e7b702d790087574/6fdffbb9c21d90ba97fc79dff3ccbeac22fb2a0e/' Gopkg.toml $ dep ensure using: $ dep version dep: version : v0.5.1 build date : 2019-03-20 git hash : faa61893 go version : go1.10.3 go compiler : gc platform : linux/amd64 features : ImportDuringSolve=false The k8s.io/apimachinery/pkg/util/clock addition in Gopkg.lock is probably catching up with c759bfa (Use expectations to ensure that clusterDeployment controller does not create multiple overlapping clusterprovisions, 2019-08-23, openshift#518).
4 participants
Use expectations to ensure that clusterDeployment controller does not create multiple overlapping clusterprovisions.
There is currently a race condition where the controller will create a second clusterprovision if the first clusterprovision has not been added to the local cache by the time the clusterdeployment is reconciled again after creating the first clusterprovision. To prevent this, these changes use expectations to block the controller from creating the second clusterprovision unitl the controller sees that the first clusterprovision has been created.
When the clusterDeployment controller creates a clusterprovision, the controller will increase the number of creation expectations for the clusterdeployment. When the controller sees an add event for the clusterprovision, the controller will reduce the number of creation expectations for the clusterdeployment. During a reconcile loop, the controller will not look at clusterprovisions while there are outstanding creation expectations for the clusterdeployment being reconciled.