Skip to content

Commit

Permalink
feat(csv): require AllNamespaces install mode (cryostatio#719)
Browse files Browse the repository at this point in the history
* feat(csv): require AllNamespaces install mode

* Remove unused function

* Remove install mode variable from Makefile
  • Loading branch information
ebaron authored and mwangggg committed Feb 29, 2024
1 parent 2af4362 commit 4b257fa
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 162 deletions.
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ USE_IMAGE_DIGESTS ?= false
ifeq ($(USE_IMAGE_DIGESTS), true)
BUNDLE_GEN_FLAGS += --use-image-digests
endif
BUNDLE_INSTALL_MODE ?= AllNamespaces

IMAGE_BUILDER ?= podman
# Image URL to use all building/pushing image targets
Expand Down Expand Up @@ -482,7 +481,7 @@ undeploy: ## Undeploy controller from the configured cluster in ~/.kube/config.

.PHONY: deploy_bundle
deploy_bundle: check_cert_manager undeploy_bundle ## Deploy the controller in the bundle format with OLM.
$(OPERATOR_SDK) run bundle --install-mode $(BUNDLE_INSTALL_MODE) $(BUNDLE_IMG)
$(OPERATOR_SDK) run bundle --install-mode AllNamespaces $(BUNDLE_IMG)
ifeq ($(DISABLE_SERVICE_TLS), true)
@echo "Disabling TLS for in-cluster communication between Services"
@current_ns=`$(CLUSTER_CLIENT) config view --minify -o 'jsonpath={.contexts[0].context.namespace}'` && \
Expand Down
4 changes: 2 additions & 2 deletions bundle/manifests/cryostat-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1236,9 +1236,9 @@ spec:
serviceAccountName: cryostat-operator-service-account
strategy: deployment
installModes:
- supported: true
- supported: false
type: OwnNamespace
- supported: true
- supported: false
type: SingleNamespace
- supported: false
type: MultiNamespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -866,9 +866,9 @@ spec:
deployments: null
strategy: ""
installModes:
- supported: true
- supported: false
type: OwnNamespace
- supported: true
- supported: false
type: SingleNamespace
- supported: false
type: MultiNamespace
Expand Down
32 changes: 0 additions & 32 deletions internal/controllers/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

Expand All @@ -61,7 +60,6 @@ type ReconcilerConfig struct {
IsCertManagerInstalled bool
EventRecorder record.EventRecorder
RESTMapper meta.RESTMapper
Namespaces []string
InsightsProxy *url.URL // Only defined if Insights is enabled
common.ReconcilerTLS
}
Expand Down Expand Up @@ -299,32 +297,10 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance)
return reconcile.Result{}, nil
}

func namespaceEventFilter(scheme *runtime.Scheme, namespaceList []string) predicate.Predicate {
namespaces := namespacesToSet(namespaceList)
return predicate.NewPredicateFuncs(func(object client.Object) bool {
// Restrict watch for namespaced objects to specified namespaces
if len(object.GetNamespace()) > 0 {
_, pres := namespaces[object.GetNamespace()]
if !pres {
return false
}
}
return true
})
}

func (r *Reconciler) setupWithManager(mgr ctrl.Manager, impl reconcile.Reconciler) error {
c := ctrl.NewControllerManagedBy(mgr).
For(r.objectType)

// Filter watch to specified namespaces only if the CRD is namespaced and
// we're not running in AllNamespace mode
// TODO remove this once only AllNamespace mode is supported
if r.isNamespaced && len(r.Namespaces) > 0 {
r.Log.Info(fmt.Sprintf("Adding EventFilter for namespaces: %v", r.Namespaces))
c = c.WithEventFilter(namespaceEventFilter(mgr.GetScheme(), r.Namespaces))
}

// Watch for changes to secondary resources and requeue the owner Cryostat
resources := []client.Object{&appsv1.Deployment{}, &corev1.Service{}, &corev1.Secret{}, &corev1.PersistentVolumeClaim{},
&corev1.ServiceAccount{}, &rbacv1.Role{}, &rbacv1.RoleBinding{}, &netv1.Ingress{}}
Expand Down Expand Up @@ -595,11 +571,3 @@ func findDeployCondition(conditions []appsv1.DeploymentCondition, condType appsv
}
return nil
}

func namespacesToSet(namespaces []string) map[string]struct{} {
result := make(map[string]struct{}, len(namespaces))
for _, namespace := range namespaces {
result[namespace] = struct{}{}
}
return result
}
7 changes: 2 additions & 5 deletions internal/controllers/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ type controllerTest struct {
}

type cryostatTestInput struct {
controller controllers.CommonReconciler
objs []ctrlclient.Object
watchNamespaces []string
controller controllers.CommonReconciler
objs []ctrlclient.Object
test.TestReconcilerConfig
*test.TestResources
}
Expand All @@ -83,7 +82,6 @@ func (c *controllerTest) commonBeforeEach() *cryostatTestInput {
t.NewNamespace(),
t.NewApiServer(),
}
t.watchNamespaces = []string{t.Namespace}
return t
}

Expand Down Expand Up @@ -125,7 +123,6 @@ func (t *cryostatTestInput) newReconcilerConfig(scheme *runtime.Scheme, client c
RESTMapper: test.NewTESTRESTMapper(),
Log: logger,
ReconcilerTLS: test.NewTestReconcilerTLS(&t.TestReconcilerConfig),
Namespaces: t.watchNamespaces,
InsightsProxy: insightsURL,
}
}
Expand Down
79 changes: 0 additions & 79 deletions internal/controllers/reconciler_unit_test.go

This file was deleted.

43 changes: 3 additions & 40 deletions internal/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"fmt"
"net/url"
"os"
"strings"

// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
Expand All @@ -30,12 +29,10 @@ import (
configv1 "github.com/openshift/api/config/v1"
consolev1 "github.com/openshift/api/console/v1"
routev1 "github.com/openshift/api/route/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/healthz"
"sigs.k8s.io/controller-runtime/pkg/log/zap"

Expand Down Expand Up @@ -80,26 +77,8 @@ func main() {
opts.BindFlags(flag.CommandLine)
flag.Parse()

watchNamespace, err := getWatchNamespace()
if err != nil {
setupLog.Error(err, "unable to get WatchNamespace, "+
"the manager will watch and manage resources in all namespaces")
}
namespaces := []string{}
if len(watchNamespace) > 0 {
namespaces = append(namespaces, strings.Split(watchNamespace, ",")...)
}

ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))

// For OwnNamespace install mode, we need to see RoleBindings in other namespaces
// when used with ClusterCryostat
// https://github.com/cryostatio/cryostat-operator/issues/580
disableCache := []client.Object{}
if len(namespaces) > 0 {
disableCache = append(disableCache, &rbacv1.RoleBinding{})
}

// FIXME Disable metrics until this issue is resolved:
// https://github.com/operator-framework/operator-sdk/issues/4684
metricsAddr = "0"
Expand All @@ -110,7 +89,6 @@ func main() {
HealthProbeBindAddress: probeAddr,
LeaderElection: enableLeaderElection,
LeaderElectionID: "d696d7ab.redhat.com",
ClientDisableCacheFor: disableCache, // TODO can probably remove
})
if err != nil {
setupLog.Error(err, "unable to start manager")
Expand Down Expand Up @@ -156,7 +134,7 @@ func main() {
}

config := newReconcilerConfig(mgr, "ClusterCryostat", "clustercryostat-controller", openShift,
certManager, namespaces, insightsURL)
certManager, insightsURL)
clusterController, err := controllers.NewClusterCryostatReconciler(config)
if err != nil {
setupLog.Error(err, "unable to create controller", "controller", "ClusterCryostat")
Expand All @@ -167,7 +145,7 @@ func main() {
os.Exit(1)
}
config = newReconcilerConfig(mgr, "Cryostat", "cryostat-controller", openShift, certManager,
namespaces, insightsURL)
insightsURL)
controller, err := controllers.NewCryostatReconciler(config)
if err != nil {
setupLog.Error(err, "unable to create controller", "controller", "Cryostat")
Expand Down Expand Up @@ -195,20 +173,6 @@ func main() {
}
}

// getWatchNamespace returns the Namespace the operator should be watching for changes
func getWatchNamespace() (string, error) {
// WatchNamespaceEnvVar is the constant for env variable WATCH_NAMESPACE
// which specifies the Namespace to watch.
// An empty value means the operator is running with cluster scope.
var watchNamespaceEnvVar = "WATCH_NAMESPACE"

ns, found := os.LookupEnv(watchNamespaceEnvVar)
if !found {
return "", fmt.Errorf("%s must be set", watchNamespaceEnvVar)
}
return ns, nil
}

func isOpenShift(client discovery.DiscoveryInterface) (bool, error) {
return discovery.IsResourceEnabled(client, routev1.GroupVersion.WithResource("routes"))
}
Expand All @@ -218,7 +182,7 @@ func isCertManagerInstalled(client discovery.DiscoveryInterface) (bool, error) {
}

func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName string, openShift bool,
certManager bool, namespaces []string, insightsURL *url.URL) *controllers.ReconcilerConfig {
certManager bool, insightsURL *url.URL) *controllers.ReconcilerConfig {
return &controllers.ReconcilerConfig{
Client: mgr.GetClient(),
Log: ctrl.Log.WithName("controllers").WithName(logName),
Expand All @@ -227,7 +191,6 @@ func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName str
IsCertManagerInstalled: certManager,
EventRecorder: mgr.GetEventRecorderFor(eventRecorderName),
RESTMapper: mgr.GetRESTMapper(),
Namespaces: namespaces,
InsightsProxy: insightsURL,
ReconcilerTLS: common.NewReconcilerTLS(&common.ReconcilerTLSConfig{
Client: mgr.GetClient(),
Expand Down

0 comments on commit 4b257fa

Please sign in to comment.