Fix ansible-operator finalizer concurrency issue#5678
Fix ansible-operator finalizer concurrency issue#5678asmacdo merged 5 commits intooperator-framework:masterfrom
Conversation
For ansible-based operators, this change fixes an issue that caused finalizers to fail to run if the watched resource (CR) is deleted during reconciliation. Fixes operator-framework#4909 Signed-off-by: Austin Macdonald <austin@redhat.com>
asmacdo
force-pushed
the
4909-fix-ansible-finalizer-race
branch
from
ff4991c to
5d3b1c3
Compare
Signed-off-by: Austin Macdonald <austin@redhat.com>
Signed-off-by: Austin Macdonald <austin@redhat.com>
everettraven
left a comment
everettraven
left a comment
There was a problem hiding this comment.
The changes look good and make sense to me, but because I feel I don't know enough about Ansible operators I will leave explicit approval to someone more knowledgeable.
jmrodri
left a comment
jmrodri
left a comment
There was a problem hiding this comment.
/lgtm
Just a couple nits but they can be fixed later if we want. The biggest one is the makefile targets should be in a .PHONY.
| @@ -0,0 +1,58 @@ | |||
| --- | |||
| # TODO(asmacdo) this should be the only task. the other is getting magiced in | |||
| - name: Create the test.example.com/v1alpha1.FinalizerConcurrencyTest | |||
There was a problem hiding this comment.
is there a reason the file is finalizerconcurrencytest_test? vs finalizerconcurrency_test?
There was a problem hiding this comment.
Yes, but its not a good one. The testdata generation takes the Kind name "FinalizerConcurrenctTest" and automagically creates a verify task finalizerconcurrencytest_test. Rather than delete that and put mine in place, I just clobbered it.
openshift-ci
Bot
added
lgtm
asmacdo
force-pushed
the
4909-fix-ansible-finalizer-race
branch
from
6143ae2 to
518595e
Compare
Signed-off-by: Austin Macdonald <austin@redhat.com>
asmacdo
force-pushed
the
4909-fix-ansible-finalizer-race
branch
from
518595e to
18936db
Compare
|
|
||
| KUBEBUILDER_ASSETS = $(PWD)/$(shell go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest && $(shell go env GOPATH)/bin/setup-envtest use $(ENVTEST_K8S_VERSION) --bin-dir tools/bin/ -p path) | ||
| test-e2e-setup: build | ||
| test-e2e-setup:: build dev-install cluster-create |
There was a problem hiding this comment.
@ryantking Since you just changed this thought you might want to have a look. After digging in, the memcached-molecule test needs to create its own cluster, so I had to separate the make targets.
Signed-off-by: Austin Macdonald <austin@redhat.com>
4 participants
For ansible-based operators, this change fixes an issue that caused
finalizers to fail to run if the watched resource (CR) is deleted during
reconciliation.
Fixes #4909
Signed-off-by: Austin Macdonald austin@redhat.com
Description of the change:
Motivation for the change:
Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments(seechangelog/fragments/00-template.yaml)website/content/en/docs