From eb4eaf24258e693eb192a0e12b5ea9fa22615cce Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Wed, 22 Jul 2020 17:05:18 -0400 Subject: [PATCH 01/12] Remove runlocal bits --- leader/leader.go | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/leader/leader.go b/leader/leader.go index 2d1c895..6e95ea1 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -31,24 +31,10 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" ) -type runModeType string - -const ( - localRunMode runModeType = "local" -) - -// forceRunModeEnv indicates if the operator should be forced to run in either local -// or cluster mode (currently only used for local mode) -var forceRunModeEnv = "OSDK_FORCE_RUN_MODE" - // errNoNamespace indicates that a namespace could not be found for the current // environment var errNoNamespace = fmt.Errorf("namespace not found for current environment") -// errRunLocal indicates that the operator is set to run in local mode (this error -// is returned by functions that only work on operators running in cluster mode) -var errRunLocal = fmt.Errorf("operator run mode forced to local") - // podNameEnvVar is the constant for env variable POD_NAME // which is the name of the current pod. const podNameEnvVar = "POD_NAME" @@ -71,7 +57,7 @@ func Become(ctx context.Context, lockName string) error { ns, err := getOperatorNamespace() if err != nil { - if err == errNoNamespace || err == errRunLocal { + if err == errNoNamespace { log.Info("Skipping leader election; not running in a cluster.") return nil } @@ -210,9 +196,6 @@ func isPodEvicted(pod corev1.Pod) bool { // getOperatorNamespace returns the namespace the operator should be running in. func getOperatorNamespace() (string, error) { - if isRunModeLocal() { - return "", errRunLocal - } nsBytes, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") if err != nil { if os.IsNotExist(err) { @@ -225,17 +208,10 @@ func getOperatorNamespace() (string, error) { return ns, nil } -func isRunModeLocal() bool { - return os.Getenv(forceRunModeEnv) == string(localRunMode) -} - // 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. func getPod(ctx context.Context, client crclient.Client, ns string) (*corev1.Pod, error) { - if isRunModeLocal() { - return nil, errRunLocal - } podName := os.Getenv(podNameEnvVar) if podName == "" { return nil, fmt.Errorf("required env %s not set, please configure downward API", podNameEnvVar) From 909be0b595a08cdbb1292c7811069f62c8ced6d1 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Wed, 22 Jul 2020 18:46:27 -0400 Subject: [PATCH 02/12] Add ginkgo tests for leader election --- leader/leader.go | 5 +- leader/leader_suite_test.go | 13 +++++ leader/leader_test.go | 110 ++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 leader/leader_suite_test.go create mode 100644 leader/leader_test.go diff --git a/leader/leader.go b/leader/leader.go index 6e95ea1..22bc628 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -39,6 +39,9 @@ var errNoNamespace = fmt.Errorf("namespace not found for current environment") // which is the name of the current pod. const podNameEnvVar = "POD_NAME" +// defaultNamespaceLocation is the default location where namespace information is stored +var defaultNamespaceLocation = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" + var log = logf.Log.WithName("leader") // maxBackoffInterval defines the maximum amount of time to wait between @@ -196,7 +199,7 @@ func isPodEvicted(pod corev1.Pod) bool { // getOperatorNamespace returns the namespace the operator should be running in. func getOperatorNamespace() (string, error) { - nsBytes, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") + nsBytes, err := ioutil.ReadFile(defaultNamespaceLocation) if err != nil { if os.IsNotExist(err) { return "", errNoNamespace diff --git a/leader/leader_suite_test.go b/leader/leader_suite_test.go new file mode 100644 index 0000000..9c23d3c --- /dev/null +++ b/leader/leader_suite_test.go @@ -0,0 +1,13 @@ +package leader + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestLeader(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Leader Suite") +} diff --git a/leader/leader_test.go b/leader/leader_test.go new file mode 100644 index 0000000..4700e3b --- /dev/null +++ b/leader/leader_test.go @@ -0,0 +1,110 @@ +package leader + +import ( + "context" + "io/ioutil" + "os" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" +) + +var _ = Describe("Leader", func() { + + Describe("Become", func() { + os.Unsetenv("POD_NAME") + It("should return an error when POD_NAME is not set", func() { + err := Become(context.TODO(), "leader-test") + Expect(err).Should(BeNil()) + }) + // TODO: write a test to ensure Become works + }) + Describe("isPodEvicted", func() { + var ( + leaderPod *corev1.Pod + ) + BeforeEach(func() { + leaderPod = &corev1.Pod{} + }) + It("should return false with an empty status", func() { + Expect(isPodEvicted(*leaderPod)).To(Equal(false)) + }) + It("should return false if reason is incorrect", func() { + leaderPod.Status.Phase = corev1.PodFailed + leaderPod.Status.Reason = "invalid" + Expect(isPodEvicted(*leaderPod)).To(Equal(false)) + }) + It("should return false if pod is in the wrong phase", func() { + leaderPod.Status.Phase = corev1.PodRunning + Expect(isPodEvicted(*leaderPod)).To(Equal(false)) + }) + It("should return true when Phase and Reason are set", func() { + leaderPod.Status.Phase = corev1.PodFailed + leaderPod.Status.Reason = "Evicted" + Expect(isPodEvicted(*leaderPod)).To(Equal(true)) + }) + }) + Describe("getOperatorNamespace", func() { + It("should return error when namespace not found", func() { + namespace, err := getOperatorNamespace() + Expect(err).To(Equal(errNoNamespace)) + Expect(namespace).To(Equal("")) + }) + It("should return namespace", func() { + + nsFile, err := setupNamespace("testnamespace") + if err != nil { + Fail(err.Error()) + } + defer os.Remove(nsFile.Name()) + defaultNamespaceLocation = nsFile.Name() + + // test + namespace, err := getOperatorNamespace() + Expect(err).Should(BeNil()) + Expect(namespace).To(Equal("testnamespace")) + }) + It("should trim whitespace from namespace", func() { + + nsFile, err := setupNamespace(" testnamespace ") + if err != nil { + Fail(err.Error()) + } + defer os.Remove(nsFile.Name()) + defaultNamespaceLocation = nsFile.Name() + + // test + namespace, err := getOperatorNamespace() + Expect(err).Should(BeNil()) + Expect(namespace).To(Equal("testnamespace")) + }) + }) + Describe("myOwnerRef", func() { + os.Unsetenv("POD_NAME") + It("should return an error when POD_NAME is not set", func() { + _, err := myOwnerRef(context.TODO(), nil, "") + Expect(err).ShouldNot(BeNil()) + }) + // TODO: write a test to ensure we get an OwnerReference + }) + Describe("getPod", func() { + os.Unsetenv("POD_NAME") + It("should return an error when POD_NAME is not set", func() { + _, err := getPod(context.TODO(), nil, "") + Expect(err).ShouldNot(BeNil()) + }) + // TODO: write a test to ensure we get a Pod + }) +}) + +func setupNamespace(ns string) (*os.File, error) { + nsFile, err := ioutil.TempFile("/tmp", "operator-ns-test") + if err != nil { + return nil, err + } + if _, err := nsFile.Write([]byte(ns)); err != nil { + return nil, err + } + return nsFile, nil +} From 34e0a503399c2d5915a0f4494c410cd19486f113 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 23 Jul 2020 11:53:24 -0400 Subject: [PATCH 03/12] Replace defaultNS with a function for better testing --- leader/leader.go | 7 ++++--- leader/leader_test.go | 8 ++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/leader/leader.go b/leader/leader.go index 22bc628..12565c4 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -39,8 +39,9 @@ var errNoNamespace = fmt.Errorf("namespace not found for current environment") // which is the name of the current pod. const podNameEnvVar = "POD_NAME" -// defaultNamespaceLocation is the default location where namespace information is stored -var defaultNamespaceLocation = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" +var readNamespace = func() ([]byte, error) { + return ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") +} var log = logf.Log.WithName("leader") @@ -199,7 +200,7 @@ func isPodEvicted(pod corev1.Pod) bool { // getOperatorNamespace returns the namespace the operator should be running in. func getOperatorNamespace() (string, error) { - nsBytes, err := ioutil.ReadFile(defaultNamespaceLocation) + nsBytes, err := readNamespace() if err != nil { if os.IsNotExist(err) { return "", errNoNamespace diff --git a/leader/leader_test.go b/leader/leader_test.go index 4700e3b..f7036db 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -58,7 +58,9 @@ var _ = Describe("Leader", func() { Fail(err.Error()) } defer os.Remove(nsFile.Name()) - defaultNamespaceLocation = nsFile.Name() + readNamespace = func() ([]byte, error) { + return ioutil.ReadFile(nsFile.Name()) + } // test namespace, err := getOperatorNamespace() @@ -72,7 +74,9 @@ var _ = Describe("Leader", func() { Fail(err.Error()) } defer os.Remove(nsFile.Name()) - defaultNamespaceLocation = nsFile.Name() + readNamespace = func() ([]byte, error) { + return ioutil.ReadFile(nsFile.Name()) + } // test namespace, err := getOperatorNamespace() From ed7bb5a583e22fc5db25cafed38295682ca57e48 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 23 Jul 2020 12:00:27 -0400 Subject: [PATCH 04/12] Export ErrNoNamespace --- leader/leader.go | 10 +++------- leader/leader_test.go | 4 ++-- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/leader/leader.go b/leader/leader.go index 12565c4..1d1787a 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -31,9 +31,9 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" ) -// errNoNamespace indicates that a namespace could not be found for the current +// 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 = fmt.Errorf("namespace not found for current environment") // podNameEnvVar is the constant for env variable POD_NAME // which is the name of the current pod. @@ -61,10 +61,6 @@ func Become(ctx context.Context, lockName string) error { ns, err := getOperatorNamespace() if err != nil { - if err == errNoNamespace { - log.Info("Skipping leader election; not running in a cluster.") - return nil - } return err } @@ -203,7 +199,7 @@ func getOperatorNamespace() (string, error) { nsBytes, err := readNamespace() if err != nil { if os.IsNotExist(err) { - return "", errNoNamespace + return "", ErrNoNamespace } return "", err } diff --git a/leader/leader_test.go b/leader/leader_test.go index f7036db..941504d 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -16,7 +16,7 @@ var _ = Describe("Leader", func() { os.Unsetenv("POD_NAME") It("should return an error when POD_NAME is not set", func() { err := Become(context.TODO(), "leader-test") - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(BeNil()) }) // TODO: write a test to ensure Become works }) @@ -48,7 +48,7 @@ var _ = Describe("Leader", func() { Describe("getOperatorNamespace", func() { It("should return error when namespace not found", func() { namespace, err := getOperatorNamespace() - Expect(err).To(Equal(errNoNamespace)) + Expect(err).To(Equal(ErrNoNamespace)) Expect(namespace).To(Equal("")) }) It("should return namespace", func() { From 380e99fd3ba2f4bec7a619c2d0cb6b168a4f47cc Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 23 Jul 2020 13:01:18 -0400 Subject: [PATCH 05/12] Add test for getPod --- leader/leader_test.go | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/leader/leader_test.go b/leader/leader_test.go index 941504d..a51d340 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -8,6 +8,9 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + crclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) var _ = Describe("Leader", func() { @@ -93,12 +96,37 @@ var _ = Describe("Leader", func() { // TODO: write a test to ensure we get an OwnerReference }) Describe("getPod", func() { - os.Unsetenv("POD_NAME") + var ( + client crclient.Client + ) + BeforeEach(func() { + client = fake.NewFakeClient( + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mypod", + Namespace: "testns", + }, + }, + ) + }) It("should return an error when POD_NAME is not set", func() { + os.Unsetenv("POD_NAME") _, err := getPod(context.TODO(), nil, "") Expect(err).ShouldNot(BeNil()) }) - // TODO: write a test to ensure we get a Pod + It("should return an error if no pod is found", func() { + os.Setenv("POD_NAME", "thisisnotthepodyourelookingfor") + _, err := getPod(context.TODO(), client, "") + Expect(err).ShouldNot(BeNil()) + }) + It("should return the pod with the given name", func() { + os.Setenv("POD_NAME", "mypod") + pod, err := getPod(context.TODO(), client, "testns") + Expect(err).Should(BeNil()) + Expect(pod).ShouldNot(BeNil()) + Expect(pod.TypeMeta.APIVersion).To(Equal("v1")) + Expect(pod.TypeMeta.Kind).To(Equal("Pod")) + }) }) }) From 2aa15df1d5a343e7d2311747565cd16fdbc87f04 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 23 Jul 2020 13:09:03 -0400 Subject: [PATCH 06/12] Add myOwnerRef test --- leader/leader_test.go | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/leader/leader_test.go b/leader/leader_test.go index a51d340..4e5e480 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -88,12 +88,37 @@ var _ = Describe("Leader", func() { }) }) Describe("myOwnerRef", func() { - os.Unsetenv("POD_NAME") + var ( + client crclient.Client + ) + BeforeEach(func() { + client = fake.NewFakeClient( + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mypod", + Namespace: "testns", + }, + }, + ) + }) It("should return an error when POD_NAME is not set", func() { - _, err := myOwnerRef(context.TODO(), nil, "") + os.Unsetenv("POD_NAME") + _, err := myOwnerRef(context.TODO(), client, "") Expect(err).ShouldNot(BeNil()) }) - // TODO: write a test to ensure we get an OwnerReference + It("should return an error if no pod is found", func() { + os.Setenv("POD_NAME", "thisisnotthepodyourelookingfor") + _, err := myOwnerRef(context.TODO(), client, "") + Expect(err).ShouldNot(BeNil()) + }) + It("should return the owner reference without error", func() { + os.Setenv("POD_NAME", "mypod") + owner, err := myOwnerRef(context.TODO(), client, "testns") + Expect(err).Should(BeNil()) + Expect(owner.APIVersion).To(Equal("v1")) + Expect(owner.Kind).To(Equal("Pod")) + Expect(owner.Name).To(Equal("mypod")) + }) }) Describe("getPod", func() { var ( From 32ab0e9bcfa0cd96407f6bfb855c519093166a9d Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 23 Jul 2020 13:29:59 -0400 Subject: [PATCH 07/12] Move `Unsetenv` into the It test --- leader/leader_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/leader/leader_test.go b/leader/leader_test.go index 4e5e480..397d111 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -16,8 +16,8 @@ import ( var _ = Describe("Leader", func() { Describe("Become", func() { - os.Unsetenv("POD_NAME") It("should return an error when POD_NAME is not set", func() { + os.Unsetenv("POD_NAME") err := Become(context.TODO(), "leader-test") Expect(err).ShouldNot(BeNil()) }) From affe8aaf6e64b0b4e8b5d7d1cb1e5c6e2e537f8e Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 23 Jul 2020 16:27:23 -0400 Subject: [PATCH 08/12] Added options to Become to make it more testable --- leader/leader.go | 57 +++++++++++++++++++++++++++++++++---------- leader/leader_test.go | 48 +++++++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 14 deletions(-) diff --git a/leader/leader.go b/leader/leader.go index 1d1787a..c3cda0a 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -49,6 +49,35 @@ var log = logf.Log.WithName("leader") // attempts to become the leader. const maxBackoffInterval = time.Second * 16 +type Option func(*Config) error + +type Config struct { + Client crclient.Client +} + +func (c *Config) SetDefaults() error { + if c.Client == nil { + config, err := config.GetConfig() + if err != nil { + return err + } + + client, err := crclient.New(config, crclient.Options{}) + if err != nil { + return err + } + c.Client = client + } + return nil +} + +func WithClient(cl crclient.Client) Option { + return func(c *Config) error { + c.Client = cl + return nil + } +} + // Become ensures that the current pod is the leader within its namespace. If // run outside a cluster, it will skip leader election and return nil. It // continuously tries to create a ConfigMap with the provided name and the @@ -56,25 +85,27 @@ const maxBackoffInterval = time.Second * 16 // the same name, so the pod that successfully creates the ConfigMap is the // leader. Upon termination of that pod, the garbage collector will delete the // ConfigMap, enabling a different pod to become the leader. -func Become(ctx context.Context, lockName string) error { +func Become(ctx context.Context, lockName string, opts ...Option) error { log.Info("Trying to become the leader.") - ns, err := getOperatorNamespace() - if err != nil { - return err + config := Config{} + + for _, opt := range opts { + if err := opt(&config); err != nil { + return err + } } - config, err := config.GetConfig() - if err != nil { + if err := config.SetDefaults(); err != nil { return err } - client, err := crclient.New(config, crclient.Options{}) + ns, err := getOperatorNamespace() if err != nil { return err } - owner, err := myOwnerRef(ctx, client, ns) + owner, err := myOwnerRef(ctx, config.Client, ns) if err != nil { return err } @@ -82,7 +113,7 @@ func Become(ctx context.Context, lockName string) error { // check for existing lock from this pod, in case we got restarted existing := &corev1.ConfigMap{} key := crclient.ObjectKey{Namespace: ns, Name: lockName} - err = client.Get(ctx, key, existing) + err = config.Client.Get(ctx, key, existing) switch { case err == nil: @@ -112,7 +143,7 @@ func Become(ctx context.Context, lockName string) error { // try to create a lock backoff := time.Second for { - err := client.Create(ctx, cm) + err := config.Client.Create(ctx, cm) switch { case err == nil: log.Info("Became the leader.") @@ -120,7 +151,7 @@ func Become(ctx context.Context, lockName string) error { case apierrors.IsAlreadyExists(err): // refresh the lock so we use current leader key := crclient.ObjectKey{Namespace: ns, Name: lockName} - if err := client.Get(ctx, key, existing); err != nil { + if err := config.Client.Get(ctx, key, existing); err != nil { log.Info("Leader lock configmap not found.") continue // configmap got lost ... just wait a bit } @@ -134,7 +165,7 @@ func Become(ctx context.Context, lockName string) error { default: leaderPod := &corev1.Pod{} key = crclient.ObjectKey{Namespace: ns, Name: existingOwners[0].Name} - err = client.Get(ctx, key, leaderPod) + err = config.Client.Get(ctx, key, leaderPod) switch { case apierrors.IsNotFound(err): log.Info("Leader pod has been deleted, waiting for garbage collection to remove the lock.") @@ -144,7 +175,7 @@ func Become(ctx context.Context, lockName string) error { log.Info("Operator pod with leader lock has been evicted.", "leader", leaderPod.Name) log.Info("Deleting evicted leader.") // Pod may not delete immediately, continue with backoff - err := client.Delete(ctx, leaderPod) + err := config.Client.Delete(ctx, leaderPod) if err != nil { log.Error(err, "Leader pod could not be deleted.") } diff --git a/leader/leader_test.go b/leader/leader_test.go index 397d111..f8fa504 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -16,12 +16,58 @@ import ( var _ = Describe("Leader", func() { Describe("Become", func() { + var ( + client crclient.Client + ) + BeforeEach(func() { + client = fake.NewFakeClient( + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test", + Namespace: "testns", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "leader-test", + }, + }, + }, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test", + Namespace: "testns", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "leader-test", + }, + }, + }, + }, + ) + }) It("should return an error when POD_NAME is not set", func() { os.Unsetenv("POD_NAME") err := Become(context.TODO(), "leader-test") Expect(err).ShouldNot(BeNil()) }) - // TODO: write a test to ensure Become works + It("should not return an error", func() { + os.Setenv("POD_NAME", "leader-test") + nsFile, err := setupNamespace("testns") + if err != nil { + Fail(err.Error()) + } + defer os.Remove(nsFile.Name()) + readNamespace = func() ([]byte, error) { + return ioutil.ReadFile(nsFile.Name()) + } + + err = Become(context.TODO(), "leader-test", WithClient(client)) + Expect(err).Should(BeNil()) + }) }) Describe("isPodEvicted", func() { var ( From 312763859cc7d884729c3cd65d3dfea6191c4fe6 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 23 Jul 2020 21:36:13 -0400 Subject: [PATCH 09/12] simplify readNamespace --- leader/leader_test.go | 40 +++++++--------------------------------- 1 file changed, 7 insertions(+), 33 deletions(-) diff --git a/leader/leader_test.go b/leader/leader_test.go index f8fa504..31f8c59 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -2,7 +2,6 @@ package leader import ( "context" - "io/ioutil" "os" . "github.com/onsi/ginkgo" @@ -56,16 +55,11 @@ var _ = Describe("Leader", func() { }) It("should not return an error", func() { os.Setenv("POD_NAME", "leader-test") - nsFile, err := setupNamespace("testns") - if err != nil { - Fail(err.Error()) - } - defer os.Remove(nsFile.Name()) readNamespace = func() ([]byte, error) { - return ioutil.ReadFile(nsFile.Name()) + return []byte("testns"), nil } - err = Become(context.TODO(), "leader-test", WithClient(client)) + err := Become(context.TODO(), "leader-test", WithClient(client)) Expect(err).Should(BeNil()) }) }) @@ -96,19 +90,16 @@ var _ = Describe("Leader", func() { }) 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() { - - nsFile, err := setupNamespace("testnamespace") - if err != nil { - Fail(err.Error()) - } - defer os.Remove(nsFile.Name()) readNamespace = func() ([]byte, error) { - return ioutil.ReadFile(nsFile.Name()) + return []byte("testnamespace"), nil } // test @@ -117,14 +108,8 @@ var _ = Describe("Leader", func() { Expect(namespace).To(Equal("testnamespace")) }) It("should trim whitespace from namespace", func() { - - nsFile, err := setupNamespace(" testnamespace ") - if err != nil { - Fail(err.Error()) - } - defer os.Remove(nsFile.Name()) readNamespace = func() ([]byte, error) { - return ioutil.ReadFile(nsFile.Name()) + return []byte(" testnamespace "), nil } // test @@ -200,14 +185,3 @@ var _ = Describe("Leader", func() { }) }) }) - -func setupNamespace(ns string) (*os.File, error) { - nsFile, err := ioutil.TempFile("/tmp", "operator-ns-test") - if err != nil { - return nil, err - } - if _, err := nsFile.Write([]byte(ns)); err != nil { - return nil, err - } - return nsFile, nil -} From dac301ce73a3d4fed0fd2c986fde006bd925014c Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Fri, 24 Jul 2020 10:39:35 -0400 Subject: [PATCH 10/12] change description --- leader/leader_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/leader/leader_test.go b/leader/leader_test.go index 31f8c59..f695941 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -12,7 +12,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -var _ = Describe("Leader", func() { +var _ = Describe("Leader election", func() { Describe("Become", func() { var ( From 81cc900e8d230c369cdf61f42d5b136b36350d55 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Fri, 24 Jul 2020 10:42:48 -0400 Subject: [PATCH 11/12] unexport setDefaults --- leader/leader.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/leader/leader.go b/leader/leader.go index c3cda0a..55f3778 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -55,7 +55,7 @@ type Config struct { Client crclient.Client } -func (c *Config) SetDefaults() error { +func (c *Config) setDefaults() error { if c.Client == nil { config, err := config.GetConfig() if err != nil { @@ -96,7 +96,7 @@ func Become(ctx context.Context, lockName string, opts ...Option) error { } } - if err := config.SetDefaults(); err != nil { + if err := config.setDefaults(); err != nil { return err } From b98972b59678a19003d32eafc28d22dd9c55b39c Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Fri, 24 Jul 2020 11:07:15 -0400 Subject: [PATCH 12/12] Ensure Become will return ErrNoNamespace --- leader/leader_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/leader/leader_test.go b/leader/leader_test.go index f695941..acf85b4 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -2,6 +2,7 @@ package leader import ( "context" + "errors" "os" . "github.com/onsi/ginkgo" @@ -53,6 +54,16 @@ var _ = Describe("Leader election", func() { err := Become(context.TODO(), "leader-test") Expect(err).ShouldNot(BeNil()) }) + It("should return an ErrNoNamespace", func() { + os.Setenv("POD_NAME", "leader-test") + readNamespace = func() ([]byte, error) { + return nil, os.ErrNotExist + } + err := Become(context.TODO(), "leader-test", WithClient(client)) + Expect(err).ShouldNot(BeNil()) + Expect(err).To(Equal(ErrNoNamespace)) + Expect(errors.Is(err, ErrNoNamespace)).To(Equal(true)) + }) It("should not return an error", func() { os.Setenv("POD_NAME", "leader-test") readNamespace = func() ([]byte, error) {