Skip to content

Commit

Permalink
Fixup
Browse files Browse the repository at this point in the history
  • Loading branch information
vincepri committed Feb 27, 2023
1 parent 22d9893 commit e561bb9
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pkg/builder/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import (
)

// Supporting mocking out functions for testing.
var newController = controller.New
var newController = controller.NewUnmanaged
var getGvk = apiutil.GVKForObject

// project represents other forms that the we can use to
Expand Down
2 changes: 1 addition & 1 deletion pkg/builder/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (l *testLogger) WithName(name string) logr.LogSink {

var _ = Describe("application", func() {
BeforeEach(func() {
newController = controller.New
newController = controller.NewUnmanaged
})

noop := reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
Expand Down
16 changes: 14 additions & 2 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,21 @@ type Controller interface {
GetLogger() logr.Logger
}

// New returns a new Controller based on the Manager, the caller is responsible
// for adding the controller to the manager as a Runnable.
// New returns a new Controller registered with the Manager. The Manager will ensure that shared Caches have
// been synced before the Controller is Started.
func New(name string, mgr manager.Manager, options Options) (Controller, error) {
c, err := NewUnmanaged(name, mgr, options)
if err != nil {
return nil, err
}

// Add the controller as a Manager components
return c, mgr.Add(c)
}

// NewUnmanaged returns a new Controller based on the Manager, the caller is responsible
// for adding the controller to the manager as a Runnable.
func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller, error) {
if options.Reconciler == nil {
return nil, fmt.Errorf("must specify Reconciler")
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ var _ = Describe("controller.Controller", func() {
It("should not leak goroutines when stopped", func() {
currentGRs := goleak.IgnoreCurrent()

ctx, cancel := context.WithCancel(context.Background())
watchChan := make(chan event.GenericEvent, 1)
watch := &source.Channel{Source: watchChan}
watchChan <- event.GenericEvent{Object: &corev1.Pod{}}
Expand All @@ -102,20 +101,21 @@ var _ = Describe("controller.Controller", func() {
Expect(c.Watch(watch, &handler.EnqueueRequestForObject{})).To(Succeed())
Expect(err).NotTo(HaveOccurred())

ctx, cancel := context.WithCancel(context.Background())
go func() {
defer GinkgoRecover()
Expect(m.Start(ctx)).To(Succeed())
close(controllerFinished)
}()

<-reconcileStarted
Eventually(reconcileStarted).Should(BeClosed())
cancel()
<-controllerFinished
Eventually(controllerFinished).Should(BeClosed())

// force-close keep-alive connections. These'll time anyway (after
// like 30s or so) but force it to speed up the tests.
clientTransport.CloseIdleConnections()
Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed())
Eventually(func() error { return goleak.Find(currentGRs) }, 10*time.Second).Should(Succeed())
})

It("should not create goroutines if never started", func() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/internal/testing/process/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ var _ = Describe("Start method", func() {
HealthCheck: HealthCheck{
URL: getServerURL(server),
},
StopTimeout: 2 * time.Second,
}
processState.Path = "bash"
processState.Args = simpleBashScript

})
Expand Down

0 comments on commit e561bb9

Please sign in to comment.