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

Simplify multi-namespace controller setup #87

Merged
merged 3 commits into from
Dec 18, 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: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,7 @@ clean:
verify:
hack/verify-gofmt.sh
hack/verify-generated-bindata.sh

.PHONY: uninstall
uninstall:
hack/uninstall.sh
22 changes: 15 additions & 7 deletions hack/uninstall.sh
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
#!/bin/bash
set -uo pipefail

WHAT="${WHAT:-all}"

# Disable the CVO
oc scale --replicas 0 -n openshift-cluster-version deployments/cluster-version-operator

# Uninstall the cluster-ingress-operator
oc delete -n openshift-ingress-operator deployments/ingress-operator
oc patch -n openshift-ingress-operator clusteringresses/default --patch '{"metadata":{"finalizers": []}}' --type=merge
oc delete -n openshift-ingress-operator clusteroperators.operatorstatus.openshift.io/openshift-ingress
oc delete clusteroperator.config.openshift.io/openshift-ingress-operator
oc delete --force --grace-period=0 -n openshift-ingress-operator clusteringresses/default
oc delete namespaces/openshift-ingress-operator

if [ "$WHAT" == "all" ]; then
oc delete namespaces/openshift-ingress-operator
fi
oc delete namespaces/openshift-ingress
oc delete clusterroles/openshift-ingress-operator
oc delete clusterroles/openshift-ingress-router
oc delete clusterrolebindings/openshift-ingress-operator
oc delete clusterrolebindings/openshift-ingress-router
oc delete customresourcedefinition.apiextensions.k8s.io/clusteringresses.ingress.openshift.io

if [ "$WHAT" == "all" ]; then
oc delete clusterroles/openshift-ingress-operator
oc delete clusterroles/openshift-ingress-router
oc delete clusterrolebindings/openshift-ingress-operator
oc delete clusterrolebindings/openshift-ingress-router
oc delete customresourcedefinition.apiextensions.k8s.io/clusteringresses.ingress.openshift.io
fi
2 changes: 0 additions & 2 deletions manifests/02-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,5 @@ spec:
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: OPERATOR_NAME
value: ingress-operator
- name: IMAGE
value: openshift/origin-haproxy-router:v4.0
12 changes: 4 additions & 8 deletions pkg/operator/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
ingressv1alpha1 "github.com/openshift/cluster-ingress-operator/pkg/apis/ingress/v1alpha1"
"github.com/openshift/cluster-ingress-operator/pkg/dns"
"github.com/openshift/cluster-ingress-operator/pkg/manifests"
operatormanager "github.com/openshift/cluster-ingress-operator/pkg/operator/manager"
"github.com/openshift/cluster-ingress-operator/pkg/util/slice"

appsv1 "k8s.io/api/apps/v1"
Expand All @@ -22,6 +21,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"
)
Expand All @@ -37,9 +37,9 @@ const (
// controller that handles all the logic for implementing ingress based on
// ClusterIngress resources.
//
// The controller will be pre-configured to watch events from a component
// source provided by the manager.
func New(mgr operatormanager.ComponentManager, config Config) (controller.Controller, error) {
// The controller will be pre-configured to watch for ClusterIngress resources
// in the manager namespace.
func New(mgr manager.Manager, config Config) (controller.Controller, error) {
reconciler := &reconciler{
Config: config,
}
Expand All @@ -51,10 +51,6 @@ func New(mgr operatormanager.ComponentManager, config Config) (controller.Contro
if err != nil {
return nil, err
}
err = c.Watch(mgr.ComponentSource(), &handler.EnqueueRequestForObject{})
if err != nil {
return nil, err
}
return c, nil
}

Expand Down
149 changes: 0 additions & 149 deletions pkg/operator/manager/manager.go

This file was deleted.

82 changes: 61 additions & 21 deletions pkg/operator/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/openshift/cluster-ingress-operator/pkg/manifests"
operatorconfig "github.com/openshift/cluster-ingress-operator/pkg/operator/config"
operatorcontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller"
operatormanager "github.com/openshift/cluster-ingress-operator/pkg/operator/manager"
"github.com/openshift/cluster-ingress-operator/pkg/util"

configv1 "github.com/openshift/api/config/v1"
Expand All @@ -27,9 +26,12 @@ import (

_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"

"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/source"
)

// scheme contains all the API types necessary for the operator's dynamic
Expand All @@ -56,6 +58,7 @@ type Operator struct {
client client.Client

manager manager.Manager
caches []cache.Cache
}

// New creates (but does not start) a new operator from configuration.
Expand All @@ -66,31 +69,17 @@ func New(config operatorconfig.Config, installConfig *util.InstallConfig, dnsMan
}
mf := manifests.NewFactory(config)

// Set up an operator manager with a component manager watching for resources
// in the router namespace. Any new namespaces or types the operator should
// react to will be added here.
components := []operatormanager.ComponentOptions{
{
Options: manager.Options{
Namespace: "openshift-ingress",
Scheme: scheme,
},
Types: []runtime.Object{
&appsv1.Deployment{},
&corev1.Service{},
},
},
}
operatorManager, err := operatormanager.New(kubeConfig, manager.Options{
// Set up an operator manager for the operator namespace.
operatorManager, err := manager.New(kubeConfig, manager.Options{
Namespace: config.Namespace,
Scheme: scheme,
}, components...)
})
if err != nil {
return nil, fmt.Errorf("failed to create operator manager: %v", err)
}

// Create and register the operator controller with the operator manager.
_, err = operatorcontroller.New(operatorManager, operatorcontroller.Config{
operatorController, err := operatorcontroller.New(operatorManager, operatorcontroller.Config{
Client: kubeClient,
Namespace: config.Namespace,
ManifestFactory: mf,
Expand All @@ -99,8 +88,33 @@ func New(config operatorconfig.Config, installConfig *util.InstallConfig, dnsMan
if err != nil {
return nil, fmt.Errorf("failed to create operator controller: %v", err)
}

// Create additional controller event sources from informers in the managed
// namespace. Any new managed resources outside the operator's namespace
// should be added here.
mapper, err := apiutil.NewDiscoveryRESTMapper(kubeConfig)
if err != nil {
return nil, fmt.Errorf("failed to get API Group-Resources")
}
ingressCache, err := cache.New(kubeConfig, cache.Options{Namespace: "openshift-ingress", Scheme: scheme, Mapper: mapper})
if err != nil {
return nil, fmt.Errorf("failed to create openshift-ingress cache: %v", err)
}

for _, obj := range []runtime.Object{
&appsv1.Deployment{},
&corev1.Service{},
} {
informer, err := ingressCache.GetInformer(obj)
if err != nil {
return nil, fmt.Errorf("failed to create informer for %v: %v", obj, err)
}
operatorController.Watch(&source.Informer{Informer: informer}, &handler.EnqueueRequestForObject{})
}

return &Operator{
manager: operatorManager,
caches: []cache.Cache{ingressCache},

// TODO: These are only needed for the default cluster ingress stuff, which
// should be refactored away.
Expand All @@ -119,8 +133,34 @@ func (o *Operator) Start(stop <-chan struct{}) error {
return fmt.Errorf("failed to ensure default cluster ingress: %v", err)
}

// Start the primary manager.
return o.manager.Start(stop)
errChan := make(chan error)

// Start secondary caches.
for _, cache := range o.caches {
go func() {
if err := cache.Start(stop); err != nil {
errChan <- err
}
}()
logrus.Infof("waiting for cache to sync")
if !cache.WaitForCacheSync(stop) {
return fmt.Errorf("failed to sync cache")
}
logrus.Infof("cache synced")
}

// Secondary caches are all synced, so start the manager.
go func() {
errChan <- o.manager.Start(stop)
}()

// Wait for the manager to exit or a secondary cache to fail.
select {
case <-stop:
return nil
case err := <-errChan:
return err
}
}

// ensureDefaultClusterIngress ensures that a default ClusterIngress exists.
Expand Down