diff --git a/conditions/conditions.go b/conditions/conditions.go index 645065e..7dc365d 100644 --- a/conditions/conditions.go +++ b/conditions/conditions.go @@ -17,10 +17,8 @@ package conditions import ( "context" "fmt" - "os" apiv2 "github.com/operator-framework/api/pkg/operators/v2" - "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" @@ -28,15 +26,8 @@ import ( ) var ( - // readNamespace gets the namespacedName of the operator. - readNamespace = utils.GetOperatorNamespace -) - -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" + // ErrNoOperatorCondition indicates that the operator condition CRD is nil + ErrNoOperatorCondition = fmt.Errorf("operator Condition CRD is nil") ) // condition is a Condition that gets and sets a specific @@ -49,21 +40,6 @@ type condition struct { var _ Condition = &condition{} -// NewCondition returns a new Condition interface using the provided client -// for the specified conditionType. The condition will internally fetch the namespacedName -// of the operatorConditionCRD. -func NewCondition(cl client.Client, condType apiv2.ConditionType) (Condition, error) { - objKey, err := GetNamespacedName() - if err != nil { - return nil, err - } - return &condition{ - namespacedName: *objKey, - condType: condType, - client: cl, - }, nil -} - // Get implements conditions.Get func (c *condition) Get(ctx context.Context) (*metav1.Condition, error) { operatorCond := &apiv2.OperatorCondition{} @@ -92,33 +68,9 @@ func (c *condition) Set(ctx context.Context, status metav1.ConditionStatus, opti Status: status, } - if len(option) != 0 { - for _, opt := range option { - opt(newCond) - } + for _, opt := range option { + opt(newCond) } meta.SetStatusCondition(&operatorCond.Spec.Conditions, *newCond) - err = c.client.Update(ctx, operatorCond) - if err != nil { - return err - } - return nil -} - -// 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. 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("could not determine operator condition name: environment variable %s not set", operatorCondEnvVar) - } - operatorNs, err := readNamespace() - if err != nil { - return nil, fmt.Errorf("could not determine operator namespace: %v", err) - } - return &types.NamespacedName{Name: conditionName, Namespace: operatorNs}, nil + return c.client.Update(ctx, operatorCond) } diff --git a/conditions/conditions_test.go b/conditions/conditions_test.go index 579863a..d74a1b5 100644 --- a/conditions/conditions_test.go +++ b/conditions/conditions_test.go @@ -32,48 +32,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -const ( - conditionFoo apiv2.ConditionType = "conditionFoo" - conditionBar apiv2.ConditionType = "conditionBar" -) - var _ = Describe("Condition", func() { var ns = "default" ctx := context.TODO() var clock kubeclock.Clock = &kubeclock.RealClock{} - var transitionTime metav1.Time = metav1.Time{Time: clock.Now()} + var transitionTime = metav1.Time{Time: clock.Now()} var cl client.Client - var err error - - BeforeEach(func() { - sch := runtime.NewScheme() - err = apiv2.AddToScheme(sch) - Expect(err).NotTo(HaveOccurred()) - cl = fake.NewClientBuilder().WithScheme(sch).Build() - }) - - Describe("NewCondition", func() { - It("should create a new condition", func() { - err := os.Setenv(operatorCondEnvVar, "test-operator-condition") - Expect(err).NotTo(HaveOccurred()) - readNamespace = func() (string, error) { - return ns, nil - } - - c, err := NewCondition(cl, conditionFoo) - Expect(err).NotTo(HaveOccurred()) - Expect(c).NotTo(BeNil()) - }) - - It("should error when namespacedName cannot be found", func() { - err := os.Unsetenv(operatorCondEnvVar) - Expect(err).NotTo(HaveOccurred()) - - c, err := NewCondition(cl, conditionFoo) - Expect(err).To(HaveOccurred()) - Expect(c).To(BeNil()) - }) - }) Describe("Get/Set", func() { var operatorCond *apiv2.OperatorCondition @@ -160,7 +124,6 @@ var _ = Describe("Condition", func() { Expect(apierrors.IsNotFound(err)).To(BeTrue()) Expect(con).To(BeNil()) }) - }) Context("Set", func() { @@ -211,53 +174,6 @@ var _ = Describe("Condition", func() { Expect(err).To(HaveOccurred()) Expect(apierrors.IsNotFound(err)).To(BeTrue()) }) - - }) - - }) - - 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() (string, error) { - return "", 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() (string, error) { - return "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 deleteCondition(ctx context.Context, client client.Client, obj client.Object) { - err := client.Delete(ctx, obj) - Expect(err).NotTo(HaveOccurred()) -} diff --git a/conditions/factory.go b/conditions/factory.go new file mode 100644 index 0000000..64ff9bb --- /dev/null +++ b/conditions/factory.go @@ -0,0 +1,119 @@ +// Copyright 2021 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 conditions + +import ( + "fmt" + "os" + + apiv2 "github.com/operator-framework/api/pkg/operators/v2" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/operator-lib/internal/utils" +) + +// Factory define the interface for building Conditions. +type Factory interface { + NewCondition(apiv2.ConditionType) (Condition, error) + GetNamespacedName() (*types.NamespacedName, error) +} + +// InClusterFactory is a conditions factory that can build conditions and get +// the namespaced name of the operator's condition based on an in-cluster +// configuration. +type InClusterFactory struct { + Client client.Client +} + +// NewCondition creates a new Condition using the provided client and condition +// type. The condition's name and namespace are determined by the Factory's GetName +// and GetNamespace functions. +func (f InClusterFactory) NewCondition(condType apiv2.ConditionType) (Condition, error) { + objKey, err := f.GetNamespacedName() + if err != nil { + return nil, err + } + return &condition{ + namespacedName: *objKey, + condType: condType, + client: f.Client, + }, nil +} + +// 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. Hence, GetNamespacedName() can provide the NamespacedName when the operator +// is running on cluster and is being managed by OLM. +func (f InClusterFactory) GetNamespacedName() (*types.NamespacedName, error) { + conditionName, err := f.getConditionName() + if err != nil { + return nil, fmt.Errorf("get operator condition name: %v", err) + } + conditionNamespace, err := f.getConditionNamespace() + if err != nil { + return nil, fmt.Errorf("get operator condition namespace: %v", err) + } + + return &types.NamespacedName{Name: conditionName, Namespace: conditionNamespace}, nil +} + +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" +) + +// getConditionName reads and returns the OPERATOR_CONDITION_NAME environment +// variable. If the variable is unset or empty, it returns an error. +func (f InClusterFactory) getConditionName() (string, error) { + name := os.Getenv(operatorCondEnvVar) + if name == "" { + return "", fmt.Errorf("could not determine operator condition name: environment variable %s not set", operatorCondEnvVar) + } + return name, nil +} + +// readNamespace gets the namespacedName of the operator. +var readNamespace = utils.GetOperatorNamespace + +// getConditionNamespace reads the namespace file mounted into a pod in a +// cluster via its service account volume. If the file is not found or cannot be +// read, this function returns an error. +func (f InClusterFactory) getConditionNamespace() (string, error) { + return readNamespace() +} + +// NewCondition returns a new Condition interface using the provided client +// for the specified conditionType. The condition will internally fetch the namespacedName +// of the operatorConditionCRD. +// +// Deprecated: Use InClusterFactory{cl}.NewCondition() instead. +func NewCondition(cl client.Client, condType apiv2.ConditionType) (Condition, error) { + return InClusterFactory{cl}.NewCondition(condType) +} + +// 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. 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. +// +// Deprecated: InClusterFactory{}.GetNamespacedName(). +func GetNamespacedName() (*types.NamespacedName, error) { + return InClusterFactory{}.GetNamespacedName() +} diff --git a/conditions/factory_test.go b/conditions/factory_test.go new file mode 100644 index 0000000..6f92077 --- /dev/null +++ b/conditions/factory_test.go @@ -0,0 +1,140 @@ +// Copyright 2021 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 conditions + +import ( + "context" + "os" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + apiv2 "github.com/operator-framework/api/pkg/operators/v2" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const ( + conditionFoo apiv2.ConditionType = "conditionFoo" + conditionBar apiv2.ConditionType = "conditionBar" +) + +var _ = Describe("NewCondition", func() { + var cl client.Client + BeforeEach(func() { + sch := runtime.NewScheme() + err := apiv2.AddToScheme(sch) + Expect(err).NotTo(HaveOccurred()) + cl = fake.NewClientBuilder().WithScheme(sch).Build() + }) + + testNewCondition(func(ct apiv2.ConditionType) (Condition, error) { + return NewCondition(cl, ct) + }) +}) + +var _ = Describe("GetNamespacedName", func() { + testGetNamespacedName(GetNamespacedName) +}) + +var _ = Describe("InClusterFactory", func() { + var cl client.Client + var f InClusterFactory + + BeforeEach(func() { + sch := runtime.NewScheme() + err := apiv2.AddToScheme(sch) + Expect(err).NotTo(HaveOccurred()) + cl = fake.NewClientBuilder().WithScheme(sch).Build() + f = InClusterFactory{cl} + }) + + Describe("NewCondition", func() { + testNewCondition(f.NewCondition) + }) + + Describe("GetNamespacedName", func() { + testGetNamespacedName(f.GetNamespacedName) + }) +}) + +func testNewCondition(fn func(apiv2.ConditionType) (Condition, error)) { + It("should create a new condition", func() { + err := os.Setenv(operatorCondEnvVar, "test-operator-condition") + Expect(err).NotTo(HaveOccurred()) + readNamespace = func() (string, error) { + return "default", nil + } + + c, err := fn(conditionFoo) + Expect(err).NotTo(HaveOccurred()) + Expect(c).NotTo(BeNil()) + }) + + It("should error when namespacedName cannot be found", func() { + err := os.Unsetenv(operatorCondEnvVar) + Expect(err).NotTo(HaveOccurred()) + + c, err := fn(conditionFoo) + Expect(err).To(HaveOccurred()) + Expect(c).To(BeNil()) + }) +} + +func testGetNamespacedName(fn func() (*types.NamespacedName, error)) { + It("should error when name of the operator condition cannot be found", func() { + err := os.Unsetenv(operatorCondEnvVar) + Expect(err).NotTo(HaveOccurred()) + + objKey, err := fn() + 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() (string, error) { + return "", os.ErrNotExist + } + + objKey, err := fn() + Expect(err).To(HaveOccurred()) + Expect(objKey).To(BeNil()) + Expect(err.Error()).To(ContainSubstring("get operator condition namespace: file does not exist")) + }) + + It("should return the right namespaced name from SA namespace file", func() { + err := os.Setenv(operatorCondEnvVar, "test") + Expect(err).NotTo(HaveOccurred()) + + readNamespace = func() (string, error) { + return "testns", nil + } + objKey, err := fn() + Expect(err).NotTo(HaveOccurred()) + Expect(objKey).NotTo(BeNil()) + Expect(objKey.Name).To(BeEquivalentTo("test")) + Expect(objKey.Namespace).To(BeEquivalentTo("testns")) + }) +} + +func deleteCondition(ctx context.Context, client client.Client, obj client.Object) { + err := client.Delete(ctx, obj) + Expect(err).NotTo(HaveOccurred()) +} diff --git a/go.sum b/go.sum index 2d195b1..f97a510 100644 --- a/go.sum +++ b/go.sum @@ -36,7 +36,6 @@ github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb0 github.com/NYTimes/gziphandler v1.1.1/go.mod h1:n/CVRwUEOgIxrgPvAQhUUr9oeUtvrhMomdKFjzJNB0c= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= -github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=