manager never sets started boolean

[terinjokes]

If the manager is started multiple times in error, subsequent calls to Start are supposed to return a friendly error message.

controller-runtime/pkg/manager/internal.go

Lines 403 to 408 in 44c5d50

	func (cm *controllerManager) Start(ctx context.Context) (err error) {
	cm.Lock()
	if cm.started {
	cm.Unlock()
	return errors.New("manager already started")
	}

There are also several other checks against started, such as preventing modifications to metrics, health checks, and readiness check endpoints while the HTTP server is doing concurrent reads.

controller-runtime/pkg/manager/internal.go

Lines 221 to 223 in 44c5d50

	if cm.started {
	return fmt.Errorf("unable to add new metrics handler because metrics endpoint has already been created")
	}

controller-runtime/pkg/manager/internal.go

Lines 243 to 245 in 44c5d50

	if cm.started {
	return fmt.Errorf("unable to add new checker because healthz endpoint has already been created")
	}

controller-runtime/pkg/manager/internal.go

Lines 260 to 262 in 44c5d50

	if cm.started {
	return fmt.Errorf("unable to add new checker because healthz endpoint has already been created")
	}

It looks like the manager's internals were refactored in 612e9b2, and in the process the code to set started was removed.

Example Program

package main

import (
	"context"
	"fmt"
	"os"
	"time"

	"sigs.k8s.io/controller-runtime/pkg/client/config"
	"sigs.k8s.io/controller-runtime/pkg/manager"
)

func main() {
	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
	defer cancel()

	controller := manager.RunnableFunc(func(ctx context.Context) error {
		fmt.Println("controller started")
		<-ctx.Done()
		return nil
	})

	cfg := config.GetConfigOrDie()

	m, err := manager.New(cfg, manager.Options{})
	if err != nil {
		fmt.Printf("error creating manager: %s", err)
		os.Exit(1)
	}

	if err := m.Add(controller); err != nil {
		fmt.Printf("error add controller: %s", err)
		os.Exit(1)
	}

        // mimic a user accidentally calling Start on the manager twice.
	go func() {
		err := m.Start(ctx)
		fmt.Printf("err: %s\n", err)
	}()

	go func() {
		err := m.Start(ctx)
		fmt.Printf("err: %s\n", err)
	}()

	<-ctx.Done()
	fmt.Println("Done")
}

Example Program Output

panic: close of closed channel

goroutine 283 [running]:
sigs.k8s.io/controller-runtime/pkg/cache/internal.(*specificInformersMap).Start.func1(0xc0007c00f0, {0x17c8990, 0xc0002ee900})
	/build/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.13.1/pkg/cache/internal/informers_map.go:163 +0x1e5
sigs.k8s.io/controller-runtime/pkg/cache/internal.(*specificInformersMap).Start(0xc0003023e0?, {0x17c8990, 0xc0002ee900})
	/build/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.13.1/pkg/cache/internal/informers_map.go:164 +0x25
created by sigs.k8s.io/controller-runtime/pkg/cache/internal.(*InformersMap).Start
	/build/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.13.1/pkg/cache/internal/deleg_map.go:70 +0x195

[cuisongliu]

done := make(chan struct{})
	go func() {
		if mgr.GetCache().WaitForCacheSync(context.Background()) {
			done <- struct{}{}
		}
	}()
	<-done

add code retry it ?

[terinjokes]

That code doesn't set the internal started boolean either?