Expose function to find controller namespace in controllerutil

A number of downstream CRs have been re-implementing the ability to
find the namespace the controller is running under, so we make this
a public function, and ensure memoization of the resolution.

Signed-off-by: Naadir Jeewa <jeewan@vmware.com>

pkg/controller/controllerutil/controllerutil.go

@@ -18,11 +18,13 @@ package controllerutil
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"os"
 	"reflect"
 
 	"k8s.io/apimachinery/pkg/api/equality"
-	"k8s.io/apimachinery/pkg/api/errors"
+	apiErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -32,6 +34,86 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 )
 
+var (
+	// Will store memoized controller namespace
+	inClusterNamespace     string
+	inClusterNamespacePath = "/var/run/secrets/kubernetes.io/serviceaccount/namespace"
+	// ErrNotRunningInCluster is the error for when the controller is not running in cluster
+	ErrNotRunningInCluster = &NotRunningInClusterError{}
+	// ErrNamespaceReadError is the error when the controller cannot read its namespace
+	ErrNamespaceReadError = &NamespaceReadError{}
+)
+
+// ReadInClusterNamespace returns the namespace by which the controller is running under.
+// read from "/var/run/secrets/kubernetes.io/serviceaccount/namespace".
+func ReadInClusterNamespace() (string, error) {
+	data, err := os.ReadFile(inClusterNamespacePath)
+	if errors.Is(err, os.ErrNotExist) {
+		return "", &NotRunningInClusterError{cause: err}
+	} else if err != nil {
+		return "", &NamespaceReadError{cause: err}
+	}
+
+	return string(data), nil
+}
+
+// ReadInClusterNamespaceCached returns the namespace by which the controller is running under.
+// read from "/var/run/secrets/kubernetes.io/serviceaccount/namespace".
+// Note that the resolved namespace is cached after it is successfully read from disk.
+func ReadInClusterNamespaceCached() (string, error) {
+	if inClusterNamespace != "" {
+		return inClusterNamespace, nil
+	}
+	ns, err := ReadInClusterNamespace()
+	if err != nil {
+		return "", err
+	}
+
+	// memoize result for next call
+	inClusterNamespace := ns
+	return inClusterNamespace, nil
+}
+
+// NotRunningInClusterError is an error returned if the controller is not running in the cluster
+// determined by "/var/run/secrets/kubernetes.io/serviceaccount/namespace" not existing.
+type NotRunningInClusterError struct {
+	cause error
+}
+
+func (e *NotRunningInClusterError) Error() string {
+	return "not running in-cluster"
+}
+
+// Unwrap returns the error cause
+func (e *NotRunningInClusterError) Unwrap() error {
+	return e.cause
+}
+
+// Is determines if the target error is a NotRunningInClusterError
+func (e *NotRunningInClusterError) Is(target error) bool {
+	return target.Error() == e.Error()
+}
+
+// NamespaceReadError is an error returned if the controller cannot read
+// from "/var/run/secrets/kubernetes.io/serviceaccount/namespace"
+type NamespaceReadError struct {
+	cause error
+}
+
+func (e *NamespaceReadError) Error() string {
+	return fmt.Sprintf("error reading namespace file at %q", inClusterNamespacePath)
+}
+
+// Unwrap returns the error cause
+func (e *NamespaceReadError) Unwrap() error {
+	return e.cause
+}
+
+// Is determines if the target error is a NamespaceReadError
+func (e *NamespaceReadError) Is(target error) bool {
+	return target.Error() == e.Error()
+}
+
 // AlreadyOwnedError is an error returned if the object you are trying to assign
 // a controller reference is already owned by another controller Object is the
 // subject and Owner is the reference for the current owner
@@ -197,7 +279,7 @@ const ( // They should complete the sentence "Deployment default/foo has been ..
 func CreateOrUpdate(ctx context.Context, c client.Client, obj client.Object, f MutateFn) (OperationResult, error) {
 	key := client.ObjectKeyFromObject(obj)
 	if err := c.Get(ctx, key, obj); err != nil {
-		if !errors.IsNotFound(err) {
+		if !apiErrors.IsNotFound(err) {
 			return OperationResultNone, err
 		}
 		if err := mutate(f, key, obj); err != nil {
@@ -234,7 +316,7 @@ func CreateOrUpdate(ctx context.Context, c client.Client, obj client.Object, f M
 func CreateOrPatch(ctx context.Context, c client.Client, obj client.Object, f MutateFn) (OperationResult, error) {
 	key := client.ObjectKeyFromObject(obj)
 	if err := c.Get(ctx, key, obj); err != nil {
-		if !errors.IsNotFound(err) {
+		if !apiErrors.IsNotFound(err) {
 			return OperationResultNone, err
 		}
 		if f != nil {

pkg/controller/controllerutil/controllerutil_internal_test.go

@@ -0,0 +1,52 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controllerutil
+
+import (
+	"errors"
+	"io/ioutil"
+	"os"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Controllerutil internal", func() {
+
+	Describe("ReadInClusterNamespaceCached", func() {
+		It("shouldn't provide a namespace if running outside of the cluster", func() {
+			_, err := ReadInClusterNamespaceCached()
+			// For the env test case, the controller is not run in a cluster
+			Expect(err).To(HaveOccurred())
+			Expect(errors.Is(err, ErrNotRunningInCluster)).To(Equal(true))
+		})
+		It("should return a namespace if file exists", func() {
+			file, err := ioutil.TempFile("", "namespace")
+			Expect(err).NotTo(HaveOccurred())
+			_, err = file.WriteString("kube-system")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(file.Close()).To(Succeed())
+			inClusterNamespacePath = file.Name()
+			namespace, err := ReadInClusterNamespaceCached()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(namespace).To(Equal("kube-system"))
+			Expect(os.Remove(file.Name())).To(Succeed())
+			inClusterNamespacePath = "/var/run/secrets/kubernetes.io/serviceaccount/namespace"
+		})
+
+	})
+})

pkg/controller/controllerutil/controllerutil_test.go

@@ -36,6 +36,7 @@ import (
 )
 
 var _ = Describe("Controllerutil", func() {
+
 	Describe("SetOwnerReference", func() {
 		It("should set ownerRef on an empty list", func() {
 			rs := &appsv1.ReplicaSet{}

pkg/leaderelection/leader_election.go

@@ -19,18 +19,16 @@ package leaderelection
 import (
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"os"
 
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/recorder"
 )
 
-const inClusterNamespacePath = "/var/run/secrets/kubernetes.io/serviceaccount/namespace"
-
 // Options provides the required configuration to create a new resource lock
 type Options struct {
 	// LeaderElection determines whether or not to use leader election when
@@ -45,6 +43,10 @@ type Options struct {
 	// election resource will be created.
 	LeaderElectionNamespace string
 
+	// LeaderElectionUncachedNamespaceLookup if set to true will do an uncached read of the
+	// in-cluster namespace
+	LeaderElectionUncachedNamespaceLookup bool
+
 	// LeaderElectionID determines the name of the resource that leader election
 	// will use for holding the leader lock.
 	LeaderElectionID string
@@ -71,9 +73,13 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op
 	// Default the namespace (if running in cluster)
 	if options.LeaderElectionNamespace == "" {
 		var err error
-		options.LeaderElectionNamespace, err = getInClusterNamespace()
+		if options.LeaderElectionUncachedNamespaceLookup {
+			options.LeaderElectionNamespace, err = controllerutil.ReadInClusterNamespace()
+		} else {
+			options.LeaderElectionNamespace, err = controllerutil.ReadInClusterNamespaceCached()
+		}
 		if err != nil {
-			return nil, fmt.Errorf("unable to find leader election namespace: %w", err)
+			return nil, fmt.Errorf("unable to find leader election namespace, please specify LeaderElectionNamespace: %w", err)
 		}
 	}
 
@@ -100,21 +106,3 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op
 			EventRecorder: recorderProvider.GetEventRecorderFor(id),
 		})
 }
-
-func getInClusterNamespace() (string, error) {
-	// Check whether the namespace file exists.
-	// If not, we are not running in cluster so can't guess the namespace.
-	_, err := os.Stat(inClusterNamespacePath)
-	if os.IsNotExist(err) {
-		return "", fmt.Errorf("not running in-cluster, please specify LeaderElectionNamespace")
-	} else if err != nil {
-		return "", fmt.Errorf("error checking namespace file: %w", err)
-	}
-
-	// Load the namespace file and return its content
-	namespace, err := ioutil.ReadFile(inClusterNamespacePath)
-	if err != nil {
-		return "", fmt.Errorf("error reading namespace file: %w", err)
-	}
-	return string(namespace), nil
-}

pkg/manager/manager.go

@@ -159,6 +159,10 @@ type Options struct {
 	// election resource will be created.
 	LeaderElectionNamespace string
 
+	// LeaderElectionUncachedNamespaceLookup will not use the cached namespace reader. Only
+	// useful for testing
+	LeaderElectionUncachedNamespaceLookup bool
+
 	// LeaderElectionID determines the name of the resource that leader election
 	// will use for holding the leader lock.
 	LeaderElectionID string
@@ -332,10 +336,11 @@ func New(config *rest.Config, options Options) (Manager, error) {
 		leaderConfig = rest.CopyConfig(config)
 	}
 	resourceLock, err := options.newResourceLock(leaderConfig, recorderProvider, leaderelection.Options{
-		LeaderElection:             options.LeaderElection,
-		LeaderElectionResourceLock: options.LeaderElectionResourceLock,
-		LeaderElectionID:           options.LeaderElectionID,
-		LeaderElectionNamespace:    options.LeaderElectionNamespace,
+		LeaderElection:                        options.LeaderElection,
+		LeaderElectionResourceLock:            options.LeaderElectionResourceLock,
+		LeaderElectionID:                      options.LeaderElectionID,
+		LeaderElectionNamespace:               options.LeaderElectionNamespace,
+		LeaderElectionUncachedNamespaceLookup: options.LeaderElectionUncachedNamespaceLookup,
 	})
 	if err != nil {
 		return nil, err

pkg/manager/manager_test.go

@@ -437,10 +437,10 @@ var _ = Describe("manger.Manager", func() {
 				Expect(err).To(MatchError(ContainSubstring("expected error")))
 			})
 			It("should return an error if namespace not set and not running in cluster", func() {
-				m, err := New(cfg, Options{LeaderElection: true, LeaderElectionID: "controller-runtime"})
+				m, err := New(cfg, Options{LeaderElection: true, LeaderElectionID: "controller-runtime", LeaderElectionUncachedNamespaceLookup: true})
 				Expect(m).To(BeNil())
 				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("unable to find leader election namespace: not running in-cluster, please specify LeaderElectionNamespace"))
+				Expect(err.Error()).To(ContainSubstring("unable to find leader election namespace, please specify LeaderElectionNamespace"))
 			})
 
 			// We must keep this default until we are sure all controller-runtime users have upgraded from the original default