Skip to content

Get pkg/controller tests to 100% coverage #12

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

Merged
merged 3 commits into from
Jun 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions OWNERS_ALIASES
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# See the OWNERS docs: https://git.k8s.io/community/contributors/devel/owners.md

aliases:
- controller-runtime-admins
- controller-runtime-admins:
- directxman12
- droot
- pwittrock
- controller-runtime-maintainers
- controller-runtime-maintainers:
5 changes: 5 additions & 0 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ type controller struct {
// defaults to cache.WaitForCacheSync
waitForCache func(stopCh <-chan struct{}, cacheSyncs ...cache.InformerSynced) bool

// started is true if the Controller has been started
started bool

// TODO(pwittrock): Consider initializing a logger with the controller name as the tag
}

Expand Down Expand Up @@ -178,6 +181,8 @@ func (c *controller) Start(stop <-chan struct{}) error {
}, c.jitterPeriod, stop)
}

c.started = true

<-stop
log.Info("Stopping workers", "controller", c.name)
return nil
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/controller_suite_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"
"time"

"github.com/kubernetes-sigs/controller-runtime/pkg/controller"
"github.com/kubernetes-sigs/controller-runtime/pkg/internal/informer"
logf "github.com/kubernetes-sigs/controller-runtime/pkg/runtime/log"
"github.com/kubernetes-sigs/controller-runtime/pkg/test"
Expand All @@ -46,6 +47,7 @@ var _ = BeforeSuite(func() {

var err error
cfg, err = testenv.Start()
controller.TestConfig = cfg
Expect(err).NotTo(HaveOccurred())

time.Sleep(1 * time.Second)
Expand Down
22 changes: 14 additions & 8 deletions pkg/controller/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import (
"sync"

"github.com/kubernetes-sigs/controller-runtime/pkg/client"
"github.com/kubernetes-sigs/controller-runtime/pkg/client/config"
"github.com/kubernetes-sigs/controller-runtime/pkg/controller/reconcile"
"github.com/kubernetes-sigs/controller-runtime/pkg/internal/apiutil"
"github.com/kubernetes-sigs/controller-runtime/pkg/internal/informer"
"github.com/kubernetes-sigs/controller-runtime/pkg/runtime/inject"
"k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
Expand Down Expand Up @@ -92,8 +92,12 @@ func (cm *controllerManager) NewController(ca Args, r reconcile.Reconcile) (Cont
cm.mu.Lock()
defer cm.mu.Unlock()

if r == nil {
return nil, fmt.Errorf("must specify Reconcile")
}

if len(ca.Name) == 0 {
return nil, fmt.Errorf("must specify name for Controller")
return nil, fmt.Errorf("must specify Name for Controller")
}

if ca.MaxConcurrentReconciles <= 0 {
Expand Down Expand Up @@ -196,6 +200,9 @@ type ManagerArgs struct {
// Scheme is the scheme used to resolve runtime.Objects to GroupVersionKinds / Resources
// Defaults to the kubernetes/client-go scheme.Scheme
Scheme *runtime.Scheme

// Mapper is the rest mapper used to map go types to Kubernetes APIs
MapperProvider func(c *rest.Config) (meta.RESTMapper, error)
}

// NewManager returns a new fully initialized Manager.
Expand All @@ -204,11 +211,7 @@ func NewManager(args ManagerArgs) (Manager, error) {

// Initialize a rest.config if none was specified
if cm.config == nil {
var err error
cm.config, err = config.GetConfig()
if err != nil {
return nil, err
}
return nil, fmt.Errorf("must specify Config")
}

// Use the Kubernetes client-go scheme if none is specified
Expand All @@ -228,7 +231,10 @@ func NewManager(args ManagerArgs) (Manager, error) {
objCache := client.NewObjectCache(spi.KnownInformersByType(), cm.scheme)
spi.Callbacks = append(spi.Callbacks, objCache)

mapper, err := apiutil.NewDiscoveryRESTMapper(cm.config)
if args.MapperProvider == nil {
args.MapperProvider = apiutil.NewDiscoveryRESTMapper
}
mapper, err := args.MapperProvider(cm.config)
if err != nil {
log.Error(err, "Failed to get API Group-Resources")
return nil, err
Expand Down
Loading