Skip to content

Address review comments for operator conditions helpers #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 2, 2020
Merged

Address review comments for operator conditions helpers #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddc803b
Address review comments for opertor conditions helpers
varshaprasad96 Nov 21, 2020
934e0e6
Update GetOperatorNamespace
varshaprasad96 Dec 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 15 additions & 27 deletions conditions/operator_conditions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@
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"
Expand All @@ -32,26 +30,29 @@ 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")
}
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
// 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 := 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
}
Expand Down Expand Up @@ -118,16 +119,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
}
168 changes: 66 additions & 102 deletions conditions/operator_conditions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() (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"))
})
})

Describe("SetOperatorCondition", func() {
It("should set condition status", func() {
newCond := metav1.Condition{
Expand Down Expand Up @@ -110,222 +151,145 @@ 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,
Reason: "foo",
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))
})
})

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 {
Expand Down
Loading