Skip to content

Commit

Permalink
Report versioning errors in StorageCluster's Status
Browse files Browse the repository at this point in the history
Update StorageCluster status on versioning error. Append an error status
condition to StorageCluster status field when an error arises during `versionCheck`.

And, modified the hack scripts to update the VERSION env during the
binary build.

Co-authored-by: Pranshu Srivastava <rexagod@gmail.com>
Signed-off-by: Nikhil-Ladha <nikhilladha1999@gmail.com>
  • Loading branch information
Nikhil-Ladha and rexagod committed Feb 7, 2023
1 parent c46ffed commit d05194d
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ lint: ## Run golangci-lint inside a container
# use 'make functest' to run just the functional tests
unit-test:
@echo "Executing unit tests"
go test -v -cover `go list ./... | grep -v "functest"`
hack/unit-test.sh

ocs-operator-ci: shellcheck-test golangci-lint unit-test verify-deps verify-generated verify-latest-deploy-yaml

Expand Down
8 changes: 7 additions & 1 deletion controllers/storagecluster/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ func (r *StorageClusterReconciler) validateStorageClusterSpec(instance *ocsv1.St
r.Log.Error(err, "Failed to validate StorageCluster version.", "StorageCluster", klog.KRef(instance.Namespace, instance.Name))
r.recorder.ReportIfNotPresent(instance, corev1.EventTypeWarning, statusutil.EventReasonValidationFailed, err.Error())
instance.Status.Phase = statusutil.PhaseError
statusutil.SetStatusConditionIfNotPresent(&instance.Status.Conditions, conditionsv1.Condition{
Type: conditionsv1.ConditionDegraded,
Status: corev1.ConditionTrue,
Reason: statusutil.EventReasonValidationFailed,
Message: err.Error(),
})
if updateErr := r.Client.Status().Update(context.TODO(), instance); updateErr != nil {
r.Log.Error(updateErr, "Failed to update StorageCluster.", "StorageCluster", klog.KRef(instance.Namespace, instance.Name))
return updateErr
Expand Down Expand Up @@ -558,7 +564,7 @@ func (r *StorageClusterReconciler) reconcilePhases(
return reconcile.Result{}, nil
}

// versionCheck populates the `.Spec.Version` field
// versionCheck populates the `.Status.Version` field
func versionCheck(sc *ocsv1.StorageCluster, reqLogger logr.Logger) error {
if sc.Status.Version == "" {
sc.Status.Version = version.Version
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ func TestVersionCheck(t *testing.T) {
for _, tc := range testcases {
reconciler := createFakeStorageClusterReconciler(t, tc.storageCluster)
err := versionCheck(tc.storageCluster, reconciler.Log)
fmt.Printf("Version: %s, version.Version: %s", tc.storageCluster.Status.Version, version.Version)
if tc.errorExpected {
assert.Errorf(t, err, "[%q]: failed to assert error when higher version is provided in the storagecluster spec", tc.label)
continue
Expand Down
10 changes: 5 additions & 5 deletions controllers/util/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func MapCephClusterNoConditions(conditions *[]conditionsv1.Condition, reason str
}

// won't override a status condition of the same type and status
func setStatusConditionIfNotPresent(conditions *[]conditionsv1.Condition, condition conditionsv1.Condition) {
func SetStatusConditionIfNotPresent(conditions *[]conditionsv1.Condition, condition conditionsv1.Condition) {

foundCondition := conditionsv1.FindStatusCondition(*conditions, condition.Type)
if foundCondition != nil && foundCondition.Status == condition.Status {
Expand All @@ -244,7 +244,7 @@ func setStatusConditionIfNotPresent(conditions *[]conditionsv1.Condition, condit
func MapNoobaaNegativeConditions(conditions *[]conditionsv1.Condition, found *nbv1.NooBaa) {

if found == nil {
setStatusConditionIfNotPresent(conditions, conditionsv1.Condition{
SetStatusConditionIfNotPresent(conditions, conditionsv1.Condition{
Type: conditionsv1.ConditionDegraded,
Status: corev1.ConditionTrue,
Reason: "NoobaaNotFound",
Expand All @@ -255,14 +255,14 @@ func MapNoobaaNegativeConditions(conditions *[]conditionsv1.Condition, found *nb

switch found.Status.Phase {
case nbv1.SystemPhaseRejected:
setStatusConditionIfNotPresent(conditions, conditionsv1.Condition{
SetStatusConditionIfNotPresent(conditions, conditionsv1.Condition{
Type: conditionsv1.ConditionDegraded,
Status: corev1.ConditionTrue,
Reason: "NoobaaSpecRejected",
Message: fmt.Sprintf("Noobaa object's configuration is rejected by the noobaa operator"), //nolint:gosimple
})
case "", nbv1.SystemPhaseVerifying, nbv1.SystemPhaseCreating, nbv1.SystemPhaseConnecting, nbv1.SystemPhaseConfiguring:
setStatusConditionIfNotPresent(conditions, conditionsv1.Condition{
SetStatusConditionIfNotPresent(conditions, conditionsv1.Condition{
Type: conditionsv1.ConditionProgressing,
Status: corev1.ConditionTrue,
Reason: "NoobaaInitializing",
Expand All @@ -271,7 +271,7 @@ func MapNoobaaNegativeConditions(conditions *[]conditionsv1.Condition, found *nb
case nbv1.SystemPhaseReady:
// no-op. Ready isn't a negative case
default:
setStatusConditionIfNotPresent(conditions, conditionsv1.Condition{
SetStatusConditionIfNotPresent(conditions, conditionsv1.Condition{
Type: conditionsv1.ConditionDegraded,
Status: corev1.ConditionTrue,
Reason: "NoobaaPhaseUnknown",
Expand Down
3 changes: 2 additions & 1 deletion hack/build-functest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ else
echo "GINKO binary found at $GINKGO"
fi

LDFLAGS="-X github.com/red-hat-storage/ocs-operator/version.Version=${VERSION}"

"${GINKGO}" build "functests/${suite}/"
"$GOBIN"/ginkgo build --ldflags "${LDFLAGS}" "functests/${suite}/"

mkdir -p $OUTDIR_BIN
mv "functests/${suite}/${suite}.test" "${OUTDIR_BIN}/${suite}_tests"
4 changes: 3 additions & 1 deletion hack/generate-unified-csv.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ set -e
source hack/common.sh

CSV_MERGER="tools/csv-merger/csv-merger"
(cd tools/csv-merger/ && go build)
LDFLAGS="-X github.com/red-hat-storage/ocs-operator/version.Version=${VERSION}"

(cd tools/csv-merger/ && go build -ldflags="${LDFLAGS}")

function help_txt() {
echo "Environment Variables"
Expand Down
10 changes: 10 additions & 0 deletions hack/unit-test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/bin/bash

set -e

source hack/common.sh

LDFLAGS="-X github.com/red-hat-storage/ocs-operator/version.Version=${VERSION}"

# shellcheck disable=SC2046
go test -ldflags="${LDFLAGS}" -v -cover $(go list ./... | grep -v "functest")

0 comments on commit d05194d

Please sign in to comment.