From ddc803b2406fa3998efdd497c98a36dee3e48886 Mon Sep 17 00:00:00 2001 From: varshaprasad96 Date: Fri, 20 Nov 2020 16:15:23 -0800 Subject: [PATCH 1/2] Address review comments for opertor conditions helpers * Move getOperatorNamespace() to a seprate utils package. * Allow namespace to be set locally with an env variable. * Modify the order of tests and add godoc to env variables. * Remove redundant code. --- conditions/operator_conditions.go | 37 +++--- conditions/operator_conditions_test.go | 168 ++++++++++--------------- internal/utils/utils.go | 38 ++++++ internal/utils/utils_suite_test.go | 27 ++++ internal/utils/utils_test.go | 56 +++++++++ leader/leader.go | 20 +-- leader/leader_test.go | 30 ----- 7 files changed, 204 insertions(+), 172 deletions(-) create mode 100644 internal/utils/utils.go create mode 100644 internal/utils/utils_suite_test.go create mode 100644 internal/utils/utils_test.go diff --git a/conditions/operator_conditions.go b/conditions/operator_conditions.go index fd8082f..c84d19e 100644 --- a/conditions/operator_conditions.go +++ b/conditions/operator_conditions.go @@ -15,13 +15,12 @@ package conditions import ( - "errors" "fmt" "io/ioutil" "os" - "strings" api "github.com/operator-framework/api/pkg/operators/v1" + "github.com/operator-framework/operator-lib/internal/utils" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -32,9 +31,12 @@ var ( ErrNoOperatorCondition = fmt.Errorf("operator Condition CRD is nil") ) -// TODO: verify from OLM if this will be the name of the environment variable -// which is set for the Condition resource owned by the operator. -const operatorCondEnvVar = "OPERATOR_CONDITION_NAME" +const ( + // operatorCondEnvVar is the env variable which + // contains the name of the Condition CR associated to the operator, + // set by OLM. + operatorCondEnvVar = "OPERATOR_CONDITION_NAME" +) var readNamespace = func() ([]byte, error) { return ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") @@ -42,16 +44,18 @@ var readNamespace = func() ([]byte, error) { // GetNamespacedName returns the NamespacedName of the CR. It returns an error // when the name of the CR cannot be found from the environment variable set by -// OLM, or when the namespace cannot be found from the associated service account -// secret. +// OLM. Hence, GetNamespacedName() can provide the NamespacedName when the operator +// is running on cluster and is being managed by OLM. If running locally, operator +// writers are encouraged to skip this method or gracefully handle the errors by logging +// a message. func GetNamespacedName() (*types.NamespacedName, error) { conditionName := os.Getenv(operatorCondEnvVar) if conditionName == "" { - return nil, fmt.Errorf("required env %s not set, cannot find operator condition CR for the operator", operatorCondEnvVar) + return nil, fmt.Errorf("could not determine operator condition name: environment variable %s not set", operatorCondEnvVar) } - operatorNs, err := getOperatorNamespace() + operatorNs, err := utils.GetOperatorNamespace(readNamespace) if err != nil { - return nil, err + return nil, fmt.Errorf("could not determine operator namespace: %v", err) } return &types.NamespacedName{Name: conditionName, Namespace: operatorNs}, nil } @@ -118,16 +122,3 @@ func IsConditionStatusPresentAndEqual(operatorCondition *api.OperatorCondition, } return false, nil } - -// getOperatorNamespace returns the namespace the operator should be running in. -func getOperatorNamespace() (string, error) { - nsBytes, err := readNamespace() - if err != nil { - if os.IsNotExist(err) { - return "", errors.New("cannot find namespace of the operator") - } - return "", err - } - ns := strings.TrimSpace(string(nsBytes)) - return ns, nil -} diff --git a/conditions/operator_conditions_test.go b/conditions/operator_conditions_test.go index 6c792a2..82bd0e8 100644 --- a/conditions/operator_conditions_test.go +++ b/conditions/operator_conditions_test.go @@ -57,6 +57,47 @@ var _ = Describe("Conditions helpers", func() { }, } }) + + Describe("GetNamespacedName", func() { + It("should error when name of the operator condition cannot be found", func() { + err := os.Unsetenv(operatorCondEnvVar) + Expect(err).NotTo(HaveOccurred()) + + objKey, err := GetNamespacedName() + Expect(err).To(HaveOccurred()) + Expect(objKey).To(BeNil()) + Expect(err.Error()).To(ContainSubstring("could not determine operator condition name")) + }) + + It("should error when object namespace cannot be found", func() { + err := os.Setenv(operatorCondEnvVar, "test") + Expect(err).NotTo(HaveOccurred()) + + readNamespace = func() ([]byte, error) { + return nil, os.ErrNotExist + } + + objKey, err := GetNamespacedName() + Expect(err).To(HaveOccurred()) + Expect(objKey).To(BeNil()) + Expect(err.Error()).To(ContainSubstring("could not determine operator namespace")) + }) + + It("should return the right namespaced name from SA namespace file", func() { + err := os.Setenv(operatorCondEnvVar, "test") + Expect(err).NotTo(HaveOccurred()) + + readNamespace = func() ([]byte, error) { + return []byte("testns"), nil + } + objKey, err := GetNamespacedName() + Expect(err).NotTo(HaveOccurred()) + Expect(objKey).NotTo(BeNil()) + Expect(objKey.Name).To(BeEquivalentTo("test")) + Expect(objKey.Namespace).To(BeEquivalentTo("testns")) + }) + }) + Describe("SetOperatorCondition", func() { It("should set condition status", func() { newCond := metav1.Condition{ @@ -110,34 +151,23 @@ var _ = Describe("Conditions helpers", func() { }) Describe("RemoveOperatorCondition", func() { - var ( - rmvConditionType string - ) It("should remove the condition", func() { - rmvConditionType = conditionFoo - Expect(RemoveOperatorCondition(operatorCondition, rmvConditionType)).NotTo(HaveOccurred()) + Expect(RemoveOperatorCondition(operatorCondition, conditionFoo)).NotTo(HaveOccurred()) Expect(len(operatorCondition.Status.Conditions)).To(BeEquivalentTo(0)) }) It("should not error when condition to be removed is not available", func() { - rmvConditionType = conditionBar - Expect(RemoveOperatorCondition(operatorCondition, rmvConditionType)).NotTo(HaveOccurred()) + Expect(RemoveOperatorCondition(operatorCondition, conditionBar)).NotTo(HaveOccurred()) Expect(len(operatorCondition.Status.Conditions)).To(BeEquivalentTo(1)) }) It("should error when operatorCondition is nil", func() { - rmvConditionType = conditionFoo - err := RemoveOperatorCondition(nil, rmvConditionType) + err := RemoveOperatorCondition(nil, conditionFoo) Expect(err).To(HaveOccurred()) Expect(err).Should(MatchError(ErrNoOperatorCondition)) }) }) Describe("FindOperatorCondition", func() { - var ( - findConditionType string - ) - It("should return the condition if it exists", func() { - findConditionType = conditionFoo conditionToFind := &metav1.Condition{ Type: conditionFoo, Status: metav1.ConditionTrue, @@ -145,20 +175,18 @@ var _ = Describe("Conditions helpers", func() { Message: "The operator is in foo condition", LastTransitionTime: transitionTime, } - c, err := FindOperatorCondition(operatorCondition, findConditionType) + c, err := FindOperatorCondition(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(reflect.DeepEqual(c, conditionToFind)).To(BeTrue()) }) It("should return error when condition does not exist", func() { - findConditionType = conditionBar - c, err := FindOperatorCondition(operatorCondition, findConditionType) + c, err := FindOperatorCondition(operatorCondition, conditionBar) Expect(err).To(HaveOccurred()) Expect(c).To(BeNil()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%s not found", findConditionType))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%s not found", conditionBar))) }) It("should error when operatorCondition is nil", func() { - findConditionType = conditionFoo - c, err := FindOperatorCondition(nil, findConditionType) + c, err := FindOperatorCondition(nil, conditionFoo) Expect(err).To(HaveOccurred()) Expect(c).To(BeNil()) Expect(err).Should(MatchError(ErrNoOperatorCondition)) @@ -166,166 +194,102 @@ var _ = Describe("Conditions helpers", func() { }) Describe("Verfiy status of the condition", func() { - var ( - conditionStatusFor string - ) - It("should return correct value when condition exists", func() { - conditionStatusFor = conditionFoo - // IsConditionStatusTrue should return true - val, err := IsConditionStatusTrue(operatorCondition, conditionStatusFor) + val, err := IsConditionStatusTrue(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeTrue()) // IsConditionStatusFalse should return false - val, err = IsConditionStatusFalse(operatorCondition, conditionStatusFor) + val, err = IsConditionStatusFalse(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeFalse()) // IsConditionStatusUnknown should return false - val, err = IsConditionStatusUnknown(operatorCondition, conditionStatusFor) + val, err = IsConditionStatusUnknown(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeFalse()) }) It("should return false if condition status is not set to true", func() { - conditionStatusFor = conditionFoo operatorCondition.Status.Conditions[0].Status = metav1.ConditionFalse // IsConditionStatusTrue should return false - val, err := IsConditionStatusTrue(operatorCondition, conditionStatusFor) + val, err := IsConditionStatusTrue(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeFalse()) // IsConditionStatusFalse should return true - val, err = IsConditionStatusFalse(operatorCondition, conditionStatusFor) + val, err = IsConditionStatusFalse(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeTrue()) // IsConditionStatusUnknown should return false - val, err = IsConditionStatusUnknown(operatorCondition, conditionStatusFor) + val, err = IsConditionStatusUnknown(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeFalse()) }) It("should return false if condition status is unknown", func() { - conditionStatusFor = conditionFoo operatorCondition.Status.Conditions[0].Status = metav1.ConditionUnknown // IsConditionStatusTrue should return false - val, err := IsConditionStatusTrue(operatorCondition, conditionStatusFor) + val, err := IsConditionStatusTrue(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeFalse()) // IsConditionStatusFalse should return false - val, err = IsConditionStatusFalse(operatorCondition, conditionStatusFor) + val, err = IsConditionStatusFalse(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeFalse()) // IsConditionStatusUnknown should return true - val, err = IsConditionStatusUnknown(operatorCondition, conditionStatusFor) + val, err = IsConditionStatusUnknown(operatorCondition, conditionFoo) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeTrue()) }) It("should error when condition cannot be found", func() { - conditionStatusFor = conditionBar - // IsConditionStatusTrue should return error - val, err := IsConditionStatusTrue(operatorCondition, conditionStatusFor) + val, err := IsConditionStatusTrue(operatorCondition, conditionBar) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%s not found", conditionStatusFor))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%s not found", conditionBar))) Expect(val).To(BeFalse()) // IsConditionStatusFalse should return error - val, err = IsConditionStatusFalse(operatorCondition, conditionStatusFor) + val, err = IsConditionStatusFalse(operatorCondition, conditionBar) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%s not found", conditionStatusFor))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%s not found", conditionBar))) Expect(val).To(BeFalse()) // IsConditionStatusUnknown should return error - val, err = IsConditionStatusUnknown(operatorCondition, conditionStatusFor) + val, err = IsConditionStatusUnknown(operatorCondition, conditionBar) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%s not found", conditionStatusFor))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%s not found", conditionBar))) Expect(val).To(BeFalse()) }) }) Describe("IsConditionStatusPresentAndEqual", func() { - var ( - conditionType string - conditionStatus metav1.ConditionStatus - ) It("should return true when condition is in the specified status", func() { - conditionType = conditionFoo - conditionStatus = metav1.ConditionTrue - - val, err := IsConditionStatusPresentAndEqual(operatorCondition, conditionType, conditionStatus) + val, err := IsConditionStatusPresentAndEqual(operatorCondition, conditionFoo, metav1.ConditionTrue) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeTrue()) }) It("should return false when condition is not present in the specified status", func() { - conditionType = conditionFoo - conditionStatus = metav1.ConditionUnknown - - val, err := IsConditionStatusPresentAndEqual(operatorCondition, conditionType, conditionStatus) + val, err := IsConditionStatusPresentAndEqual(operatorCondition, conditionFoo, metav1.ConditionUnknown) Expect(err).NotTo(HaveOccurred()) Expect(val).To(BeFalse()) }) It("should return error when condition is not present", func() { - conditionType = conditionBar - conditionStatus = metav1.ConditionTrue - - val, err := IsConditionStatusPresentAndEqual(operatorCondition, conditionType, conditionStatus) + val, err := IsConditionStatusPresentAndEqual(operatorCondition, conditionBar, metav1.ConditionTrue) Expect(err).To(HaveOccurred()) Expect(val).To(BeFalse()) }) }) - - Describe("GetNamespacedName", func() { - It("should error when name of the operator condition cannot be found", func() { - err := os.Unsetenv(operatorCondEnvVar) - Expect(err).NotTo(HaveOccurred()) - - objKey, err := GetNamespacedName() - Expect(err).To(HaveOccurred()) - Expect(objKey).To(BeNil()) - Expect(err.Error()).To(ContainSubstring("cannot find operator condition CR for the operator")) - }) - - It("should error when object namespace cannot be found", func() { - err := os.Setenv(operatorCondEnvVar, "test") - Expect(err).NotTo(HaveOccurred()) - - readNamespace = func() ([]byte, error) { - return nil, os.ErrNotExist - } - - objKey, err := GetNamespacedName() - Expect(err).To(HaveOccurred()) - Expect(objKey).To(BeNil()) - Expect(err.Error()).To(ContainSubstring("cannot find namespace")) - }) - - It("should return the right namespaced name", func() { - err := os.Setenv(operatorCondEnvVar, "test") - Expect(err).NotTo(HaveOccurred()) - - readNamespace = func() ([]byte, error) { - return []byte("testns"), nil - } - objKey, err := GetNamespacedName() - Expect(err).NotTo(HaveOccurred()) - Expect(objKey).NotTo(BeNil()) - Expect(objKey.Name).To(BeEquivalentTo("test")) - Expect(objKey.Namespace).To(BeEquivalentTo("testns")) - }) - }) - }) func isConditionPresent(arr []metav1.Condition, con metav1.Condition) bool { diff --git a/internal/utils/utils.go b/internal/utils/utils.go new file mode 100644 index 0000000..019dd8c --- /dev/null +++ b/internal/utils/utils.go @@ -0,0 +1,38 @@ +// Copyright 2020 The Operator-SDK 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 utils + +import ( + "fmt" + "os" + "strings" +) + +// ErrNoNamespace indicates that a namespace could not be found for the current +// environment +var ErrNoNamespace = fmt.Errorf("namespace not found for current environment") + +// GetOperatorNamespace returns the namespace the operator should be running in. +func GetOperatorNamespace(readNamespace func() ([]byte, error)) (string, error) { + nsBytes, err := readNamespace() + if err != nil { + if os.IsNotExist(err) { + return "", ErrNoNamespace + } + return "", err + } + ns := strings.TrimSpace(string(nsBytes)) + return ns, nil +} diff --git a/internal/utils/utils_suite_test.go b/internal/utils/utils_suite_test.go new file mode 100644 index 0000000..bd7fc9e --- /dev/null +++ b/internal/utils/utils_suite_test.go @@ -0,0 +1,27 @@ +// Copyright 2020 The Operator-SDK 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 utils + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestLeader(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Helpers Suite") +} diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go new file mode 100644 index 0000000..6e2631d --- /dev/null +++ b/internal/utils/utils_test.go @@ -0,0 +1,56 @@ +// Copyright 2020 The Operator-SDK 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 utils + +import ( + "os" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Helpers test", func() { + Describe("GetOperatorNamespace", func() { + It("should return error when namespace not found", func() { + readNamespace := func() ([]byte, error) { + return nil, os.ErrNotExist + } + namespace, err := GetOperatorNamespace(readNamespace) + Expect(err).To(Equal(ErrNoNamespace)) + Expect(namespace).To(Equal("")) + }) + It("should return namespace", func() { + readNamespace := func() ([]byte, error) { + return []byte("testnamespace"), nil + } + + // test + namespace, err := GetOperatorNamespace(readNamespace) + Expect(err).Should(BeNil()) + Expect(namespace).To(Equal("testnamespace")) + }) + It("should trim whitespace from namespace", func() { + readNamespace := func() ([]byte, error) { + return []byte(" testnamespace "), nil + } + + // test + namespace, err := GetOperatorNamespace(readNamespace) + Expect(err).Should(BeNil()) + Expect(namespace).To(Equal("testnamespace")) + }) + }) + +}) diff --git a/leader/leader.go b/leader/leader.go index 5e58e11..bd3450d 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -19,9 +19,9 @@ import ( "fmt" "io/ioutil" "os" - "strings" "time" + "github.com/operator-framework/operator-lib/internal/utils" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -33,7 +33,7 @@ import ( // ErrNoNamespace indicates that a namespace could not be found for the current // environment -var ErrNoNamespace = fmt.Errorf("namespace not found for current environment") +var ErrNoNamespace = utils.ErrNoNamespace // podNameEnvVar is the constant for env variable POD_NAME // which is the name of the current pod. @@ -103,7 +103,7 @@ func Become(ctx context.Context, lockName string, opts ...Option) error { return err } - ns, err := getOperatorNamespace() + ns, err := utils.GetOperatorNamespace(readNamespace) if err != nil { return err } @@ -236,20 +236,6 @@ func isPodEvicted(pod corev1.Pod) bool { return podFailed && podEvicted } -// getOperatorNamespace returns the namespace the operator should be running in. -func getOperatorNamespace() (string, error) { - nsBytes, err := readNamespace() - if err != nil { - if os.IsNotExist(err) { - return "", ErrNoNamespace - } - return "", err - } - ns := strings.TrimSpace(string(nsBytes)) - log.V(1).Info("Found namespace", "Namespace", ns) - return ns, nil -} - // getPod returns a Pod object that corresponds to the pod in which the code // is currently running. // It expects the environment variable POD_NAME to be set by the downwards API. diff --git a/leader/leader_test.go b/leader/leader_test.go index e308abe..6cd4ec3 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -113,36 +113,6 @@ var _ = Describe("Leader election", func() { Expect(isPodEvicted(*leaderPod)).To(Equal(true)) }) }) - Describe("getOperatorNamespace", func() { - It("should return error when namespace not found", func() { - readNamespace = func() ([]byte, error) { - return nil, os.ErrNotExist - } - namespace, err := getOperatorNamespace() - Expect(err).To(Equal(ErrNoNamespace)) - Expect(namespace).To(Equal("")) - }) - It("should return namespace", func() { - readNamespace = func() ([]byte, error) { - return []byte("testnamespace"), nil - } - - // test - namespace, err := getOperatorNamespace() - Expect(err).Should(BeNil()) - Expect(namespace).To(Equal("testnamespace")) - }) - It("should trim whitespace from namespace", func() { - readNamespace = func() ([]byte, error) { - return []byte(" testnamespace "), nil - } - - // test - namespace, err := getOperatorNamespace() - Expect(err).Should(BeNil()) - Expect(namespace).To(Equal("testnamespace")) - }) - }) Describe("myOwnerRef", func() { var ( client crclient.Client From 934e0e610c94570fe5ac23a095ecac18b1a363cb Mon Sep 17 00:00:00 2001 From: varshaprasad96 Date: Tue, 1 Dec 2020 16:35:42 -0800 Subject: [PATCH 2/2] Update GetOperatorNamespace --- conditions/operator_conditions.go | 7 ++----- conditions/operator_conditions_test.go | 8 ++++---- internal/utils/utils.go | 12 +++++++++--- internal/utils/utils_test.go | 12 ++++++------ leader/leader.go | 7 ++----- leader/leader_test.go | 8 ++++---- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/conditions/operator_conditions.go b/conditions/operator_conditions.go index c84d19e..9e47d8c 100644 --- a/conditions/operator_conditions.go +++ b/conditions/operator_conditions.go @@ -16,7 +16,6 @@ package conditions import ( "fmt" - "io/ioutil" "os" api "github.com/operator-framework/api/pkg/operators/v1" @@ -38,9 +37,7 @@ const ( operatorCondEnvVar = "OPERATOR_CONDITION_NAME" ) -var readNamespace = func() ([]byte, error) { - return ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") -} +var readNamespace = utils.GetOperatorNamespace // GetNamespacedName returns the NamespacedName of the CR. It returns an error // when the name of the CR cannot be found from the environment variable set by @@ -53,7 +50,7 @@ func GetNamespacedName() (*types.NamespacedName, error) { if conditionName == "" { return nil, fmt.Errorf("could not determine operator condition name: environment variable %s not set", operatorCondEnvVar) } - operatorNs, err := utils.GetOperatorNamespace(readNamespace) + operatorNs, err := readNamespace() if err != nil { return nil, fmt.Errorf("could not determine operator namespace: %v", err) } diff --git a/conditions/operator_conditions_test.go b/conditions/operator_conditions_test.go index 82bd0e8..4c33fc7 100644 --- a/conditions/operator_conditions_test.go +++ b/conditions/operator_conditions_test.go @@ -73,8 +73,8 @@ var _ = Describe("Conditions helpers", func() { err := os.Setenv(operatorCondEnvVar, "test") Expect(err).NotTo(HaveOccurred()) - readNamespace = func() ([]byte, error) { - return nil, os.ErrNotExist + readNamespace = func() (string, error) { + return "", os.ErrNotExist } objKey, err := GetNamespacedName() @@ -87,8 +87,8 @@ var _ = Describe("Conditions helpers", func() { err := os.Setenv(operatorCondEnvVar, "test") Expect(err).NotTo(HaveOccurred()) - readNamespace = func() ([]byte, error) { - return []byte("testns"), nil + readNamespace = func() (string, error) { + return "testns", nil } objKey, err := GetNamespacedName() Expect(err).NotTo(HaveOccurred()) diff --git a/internal/utils/utils.go b/internal/utils/utils.go index 019dd8c..ec1b4de 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -16,6 +16,7 @@ package utils import ( "fmt" + "io/ioutil" "os" "strings" ) @@ -24,9 +25,14 @@ import ( // environment var ErrNoNamespace = fmt.Errorf("namespace not found for current environment") -// GetOperatorNamespace returns the namespace the operator should be running in. -func GetOperatorNamespace(readNamespace func() ([]byte, error)) (string, error) { - nsBytes, err := readNamespace() +var readSAFile = func() ([]byte, error) { + return ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") +} + +// GetOperatorNamespace returns the namespace the operator should be running in from +// the associated service account secret. +var GetOperatorNamespace = func() (string, error) { + nsBytes, err := readSAFile() if err != nil { if os.IsNotExist(err) { return "", ErrNoNamespace diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index 6e2631d..04c5aae 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -24,30 +24,30 @@ import ( var _ = Describe("Helpers test", func() { Describe("GetOperatorNamespace", func() { It("should return error when namespace not found", func() { - readNamespace := func() ([]byte, error) { + readSAFile = func() ([]byte, error) { return nil, os.ErrNotExist } - namespace, err := GetOperatorNamespace(readNamespace) + namespace, err := GetOperatorNamespace() Expect(err).To(Equal(ErrNoNamespace)) Expect(namespace).To(Equal("")) }) It("should return namespace", func() { - readNamespace := func() ([]byte, error) { + readSAFile = func() ([]byte, error) { return []byte("testnamespace"), nil } // test - namespace, err := GetOperatorNamespace(readNamespace) + namespace, err := GetOperatorNamespace() Expect(err).Should(BeNil()) Expect(namespace).To(Equal("testnamespace")) }) It("should trim whitespace from namespace", func() { - readNamespace := func() ([]byte, error) { + readSAFile = func() ([]byte, error) { return []byte(" testnamespace "), nil } // test - namespace, err := GetOperatorNamespace(readNamespace) + namespace, err := GetOperatorNamespace() Expect(err).Should(BeNil()) Expect(namespace).To(Equal("testnamespace")) }) diff --git a/leader/leader.go b/leader/leader.go index bd3450d..5fdcfa3 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -17,7 +17,6 @@ package leader import ( "context" "fmt" - "io/ioutil" "os" "time" @@ -39,9 +38,7 @@ var ErrNoNamespace = utils.ErrNoNamespace // which is the name of the current pod. const podNameEnvVar = "POD_NAME" -var readNamespace = func() ([]byte, error) { - return ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") -} +var readNamespace = utils.GetOperatorNamespace var log = logf.Log.WithName("leader") @@ -103,7 +100,7 @@ func Become(ctx context.Context, lockName string, opts ...Option) error { return err } - ns, err := utils.GetOperatorNamespace(readNamespace) + ns, err := readNamespace() if err != nil { return err } diff --git a/leader/leader_test.go b/leader/leader_test.go index 6cd4ec3..b5b3125 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -70,8 +70,8 @@ var _ = Describe("Leader election", func() { }) It("should return an ErrNoNamespace", func() { os.Setenv("POD_NAME", "leader-test") - readNamespace = func() ([]byte, error) { - return nil, os.ErrNotExist + readNamespace = func() (string, error) { + return "", ErrNoNamespace } err := Become(context.TODO(), "leader-test", WithClient(client)) Expect(err).ShouldNot(BeNil()) @@ -80,8 +80,8 @@ var _ = Describe("Leader election", func() { }) It("should not return an error", func() { os.Setenv("POD_NAME", "leader-test") - readNamespace = func() ([]byte, error) { - return []byte("testns"), nil + readNamespace = func() (string, error) { + return "testns", nil } err := Become(context.TODO(), "leader-test", WithClient(client))