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
`VersionMismatch` 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 21, 2023
1 parent 6f79345 commit a73ddb7
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 5 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,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
4 changes: 4 additions & 0 deletions api/v1/storagecluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,10 @@ const (
// ConditionExternalClusterConnecting type indicates that rook is still trying for
// an external connection
ConditionExternalClusterConnecting conditionsv1.ConditionType = "ExternalClusterConnecting"

// ConditionVersionMismatch type indicates that there is a mismatch in the storagecluster
// and the operator version
ConditionVersionMismatch conditionsv1.ConditionType = "VersionMismatch"
)

// List of constants to show different different reconciliation messages and statuses.
Expand Down
9 changes: 8 additions & 1 deletion controllers/storagecluster/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,20 @@ 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
reason := statusutil.EventReasonValidationFailed
message := err.Error()
statusutil.SetVersionMismatchCondition(&instance.Status.Conditions, reason, message)
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
}
return err
}

reason := statusutil.VersionValidReason
message := "Version check successful"
statusutil.SetVersionMismatchCondition(&instance.Status.Conditions, reason, message)

if !instance.Spec.ExternalStorage.Enable {
if err := r.validateStorageDeviceSets(instance); err != nil {
r.Log.Error(err, "Failed to validate StorageDeviceSets.", "StorageCluster", klog.KRef(instance.Namespace, instance.Name))
Expand Down Expand Up @@ -558,7 +565,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
32 changes: 32 additions & 0 deletions controllers/util/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ const (
ExternalClusterUnknownReason = "ExternalClusterStateUnknownCondition"
// ExternalClusterErrorReason indicates an error state
ExternalClusterErrorReason = "ExternalClusterStateError"
// VersionValidReason indicates version in SC CR and operator is equal
VersionValidReason = "VersionMatched"
)

// SetProgressingCondition sets the ProgressingCondition to True and other conditions to
Expand Down Expand Up @@ -239,6 +241,16 @@ func setStatusConditionIfNotPresent(conditions *[]conditionsv1.Condition, condit
conditionsv1.SetStatusCondition(conditions, condition)
}

// update an existing condtion only if its already present and the status is different
func setStatusConditionIfDifferentPresent(conditions *[]conditionsv1.Condition, condition conditionsv1.Condition) {

foundCondition := conditionsv1.FindStatusCondition(*conditions, condition.Type)
if foundCondition != nil && foundCondition.Status != condition.Status {
// update
conditionsv1.SetStatusCondition(conditions, condition)
}
}

// MapNoobaaNegativeConditions records noobaa related conditions
// This will only look for negative conditions: !Available, Degraded, Progressing
func MapNoobaaNegativeConditions(conditions *[]conditionsv1.Condition, found *nbv1.NooBaa) {
Expand Down Expand Up @@ -280,3 +292,23 @@ func MapNoobaaNegativeConditions(conditions *[]conditionsv1.Condition, found *nb
}

}

// SetVersionMismatchCondition sets the ConditionReconcileComplete to True and other Conditions
// to indicate that the reconciliation process has completed successfully.
func SetVersionMismatchCondition(conditions *[]conditionsv1.Condition, reason string, message string) {
if reason != VersionValidReason {
conditionsv1.SetStatusCondition(conditions, conditionsv1.Condition{
Type: ocsv1.ConditionVersionMismatch,
Status: corev1.ConditionTrue,
Reason: reason,
Message: message,
})
} else {
setStatusConditionIfDifferentPresent(conditions, conditionsv1.Condition{
Type: ocsv1.ConditionVersionMismatch,
Status: corev1.ConditionFalse,
Reason: reason,
Message: message,
})
}
}
3 changes: 2 additions & 1 deletion hack/build-functest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ else
echo "GINKO binary found at $GINKGO"
fi

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

"${GINKGO}" build "functests/${suite}/"
"${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=${CSV_VERSION}"

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

function help_txt() {
echo "Environment Variables"
Expand Down
2 changes: 1 addition & 1 deletion hack/go-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ source hack/common.sh
mkdir -p ${OUTDIR_BIN}

LDFLAGS="-s -w -X github.com/red-hat-storage/ocs-operator/controllers/defaults.IsUnsupportedCephVersionAllowed=${OCS_ALLOW_UNSUPPORTED_CEPH_VERSION} \
-X github.com/red-hat-storage/ocs-operator/version.Version=${VERSION}"
-X github.com/red-hat-storage/ocs-operator/version.Version=${CSV_VERSION}"

go build -tags 'netgo osusergo' -ldflags="${LDFLAGS}" -o ${OUTDIR_BIN}/ocs-operator ./main.go
go build -tags 'netgo osusergo' -ldflags="${LDFLAGS}" -o ${OUTDIR_BIN}/metrics-exporter ./metrics/main.go
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=${CSV_VERSION}"

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

0 comments on commit a73ddb7

Please sign in to comment.