Skip to content

Commit

Permalink
Merge pull request #1139 from alvaroaleman/error-on-multiple-start
Browse files Browse the repository at this point in the history
🐛 Controller: Return error when started more than once
  • Loading branch information
k8s-ci-robot authored Aug 26, 2020
2 parents bb54523 + db22614 commit 7dfe893
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 0 deletions.
4 changes: 4 additions & 0 deletions pkg/internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controller

import (
"context"
"errors"
"fmt"
"sync"
"time"
Expand Down Expand Up @@ -126,6 +127,9 @@ func (c *Controller) Start(stop <-chan struct{}) error {
// use an IIFE to get proper lock handling
// but lock outside to get proper handling of the queue shutdown
c.mu.Lock()
if c.Started {
return errors.New("controller was started more than once. This is likely to be caused by being added to a manager multiple times")
}

c.Queue = c.MakeQueue()
defer c.Queue.ShutDown() // needs to be outside the iife so that we shutdown after the stop channel is closed
Expand Down
11 changes: 11 additions & 0 deletions pkg/internal/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,17 @@ var _ = Describe("controller", func() {
close(stopped)
Expect(ctrl.Start(stopped)).To(Equal(err))
})

It("should return an error if it gets started more than once", func() {
// Use a stopped channel so Start doesn't block
stopped := make(chan struct{})
close(stopped)
Expect(ctrl.Start(stopped)).To(BeNil())
err := ctrl.Start(stopped)
Expect(err).NotTo(BeNil())
Expect(err.Error()).To(Equal("controller was started more than once. This is likely to be caused by being added to a manager multiple times"))
})

})

Describe("Watch", func() {
Expand Down

0 comments on commit 7dfe893

Please sign in to comment.