Skip to content
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

Deleting a binding on a non-existing RabbitMQ Cluster causes a panic #95

Closed
Zerpet opened this issue Apr 6, 2021 · 0 comments · Fixed by #102
Closed

Deleting a binding on a non-existing RabbitMQ Cluster causes a panic #95

Zerpet opened this issue Apr 6, 2021 · 0 comments · Fixed by #102
Labels
bug Something isn't working

Comments

@Zerpet
Copy link
Contributor

Zerpet commented Apr 6, 2021

Describe the bug

When a Binding object is marked for deletion and RabbitMQ Cluster does not exist, the operator panics.

To Reproduce

Steps to reproduce the behavior: Untested

  1. Create a RabbitMQ cluster and a valid binding
  2. Delete RabbitMQ cluster
  3. Delete the binding

Include any YAML or manifest necessary to reproduce the problem.

Expected behavior
The Operator deletes the binding object and/or removes the finalizer in the Binding object.

Screenshots

Log snippet

2021-04-06T09:07:59.848Z        INFO    controller-runtime.manager.controller.binding   Deleting        {"reconciler group": "rabbitmq.com", "reconciler kind": "Binding", "name": "x-invoices-to-invoices", "namespace": "rabbitmq-red"}
E0406 09:07:59.849198       1 runtime.go:78] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference)
goroutine 374 [running]:
k8s.io/apimachinery/pkg/util/runtime.logPanic(0x1581480, 0x22be540)
        /go/pkg/mod/k8s.io/[email protected]/pkg/util/runtime/runtime.go:74 +0x95
k8s.io/apimachinery/pkg/util/runtime.HandleCrash(0x0, 0x0, 0x0)
        /go/pkg/mod/k8s.io/[email protected]/pkg/util/runtime/runtime.go:48 +0x86
panic(0x1581480, 0x22be540)
        /usr/local/go/src/runtime/panic.go:965 +0x1b9
github.com/michaelklishin/rabbit-hole/v2.newRequestWithBody(0x0, 0x171d9c8, 0x6, 0xc00099e990, 0x26, 0x0, 0x0, 0x0, 0x7f9f229665a0, 0xc0005e9728, ...)
        /go/pkg/mod/github.com/michaelklishin/rabbit-hole/[email protected]/client.go:100 +0x37
github.com/michaelklishin/rabbit-hole/v2.(*Client).DeleteBinding(0x0, 0x22808b8, 0x1, 0xc000047d30, 0xa, 0x22808b8, 0x1, 0xc000047d20, 0x8, 0xc000047d28, ...)
        /go/pkg/mod/github.com/michaelklishin/rabbit-hole/[email protected]/bindings.go:217 +0x330
github.com/rabbitmq/messaging-topology-operator/controllers.(*BindingReconciler).deleteBinding(0xc000422900, 0x197ab58, 0xc00077d590, 0x0, 0xc000754d00, 0x0, 0xc000047cd0)
        /workspace/controllers/binding_controller.go:156 +0x17b
github.com/rabbitmq/messaging-topology-operator/controllers.(*BindingReconciler).Reconcile(0xc000422900, 0x197ab58, 0xc00077d590, 0xc000047cd0, 0xc, 0xc000403170, 0x16, 0xc00077d590, 0xc000030000, 0x15feb80, ...)
        /workspace/controllers/binding_controller.go:67 +0xcfe
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc0002e8140, 0x197aab0, 0xc000326000, 0x15cb320, 0xc000774a80)
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:298 +0x30d

Additional context

The problem seems to be in Reconcile() function, which does not cover the case of a binding marked for deletion and RabbitMQ Cluster object does not exist.

if errors.Is(err, internal.NoSuchRabbitmqClusterError) && binding.ObjectMeta.DeletionTimestamp.IsZero() {
logger.Info("Could not generate rabbitClient for non existent cluster: " + err.Error())
return reconcile.Result{RequeueAfter: 10 * time.Second}, err
} else if err != nil && !errors.Is(err, internal.NoSuchRabbitmqClusterError) {
logger.Error(err, failedGenerateRabbitClient)
return reconcile.Result{}, err
}

We have done a quick spike to verify this, but we should fix this properly and add a test using the framework in #89.

@Zerpet Zerpet added the bug Something isn't working label Apr 6, 2021
@coro coro closed this as completed in #102 Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant