update RBDPVCBackup.status

Update RBDPVCBackup status to follow the offical convention: ref. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties Signed-off-by: Satoru Takeuchi <[email protected]>
cybozu-go · Apr 3, 2024 · 81dacee · 81dacee
1 parent 160b328
commit 81dacee
Show file tree

Hide file tree

Showing 7 changed files with 124 additions and 97 deletions.
diff --git a/Makefile b/Makefile
@@ -2,7 +2,7 @@
 # Image URL to use all building/pushing image targets
 IMG ?= controller:latest
 # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.28.0
+ENVTEST_K8S_VERSION = 1.28.3
 
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 ifeq (,$(shell go env GOBIN))

diff --git a/api/v1/rbdpvcbackup_types.go b/api/v1/rbdpvcbackup_types.go
@@ -30,15 +30,11 @@ type RBDPVCBackupStatus struct {
 	CreatedAt metav1.Time `json:"createdAt,omitempty"`
 
 	// 'conditions' specifies current backup conditions
-	// +kubebuilder:validation:Enum=Creating;Bound;Failed;Deleting
-	Conditions string `json:"conditions,omitempty"`
+	Conditions []metav1.Condition `json:"conditions,omitempty"`
 }
 
 const (
-	RBDPVCBackupConditionsCreating = "Creating"
-	RBDPVCBackupConditionsBound    = "Bound"
-	RBDPVCBackupConditionsFailed   = "Failed"
-	RBDPVCBackupConditionsDeleting = "Deleting"
+	RBDPVCBackupConditionTypeReadyToUse = "ReadyToUse"
 )
 
 //+kubebuilder:object:root=true

diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/backup.cybozu.com_rbdpvcbackups.yaml b/config/crd/bases/backup.cybozu.com_rbdpvcbackups.yaml
@@ -46,12 +46,73 @@ spec:
             properties:
               conditions:
                 description: '''conditions'' specifies current backup conditions'
-                enum:
-                - Creating
-                - Bound
-                - Failed
-                - Deleting
-                type: string
+                items:
+                  description: "Condition contains details for one aspect of the current
+                    state of this API Resource. --- This struct is intended for direct
+                    use as an array at the field path .status.conditions.  For example,
+                    \n type FooStatus struct{ // Represents the observations of a
+                    foo's current state. // Known .status.conditions.type are: \"Available\",
+                    \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge
+                    // +listType=map // +listMapKey=type Conditions []metav1.Condition
+                    `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\"
+                    protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }"
+                  properties:
+                    lastTransitionTime:
+                      description: lastTransitionTime is the last time the condition
+                        transitioned from one status to another. This should be when
+                        the underlying condition changed.  If that is not known, then
+                        using the time when the API field changed is acceptable.
+                      format: date-time
+                      type: string
+                    message:
+                      description: message is a human readable message indicating
+                        details about the transition. This may be an empty string.
+                      maxLength: 32768
+                      type: string
+                    observedGeneration:
+                      description: observedGeneration represents the .metadata.generation
+                        that the condition was set based upon. For instance, if .metadata.generation
+                        is currently 12, but the .status.conditions[x].observedGeneration
+                        is 9, the condition is out of date with respect to the current
+                        state of the instance.
+                      format: int64
+                      minimum: 0
+                      type: integer
+                    reason:
+                      description: reason contains a programmatic identifier indicating
+                        the reason for the condition's last transition. Producers
+                        of specific condition types may define expected values and
+                        meanings for this field, and whether the values are considered
+                        a guaranteed API. The value should be a CamelCase string.
+                        This field may not be empty.
+                      maxLength: 1024
+                      minLength: 1
+                      pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
+                      type: string
+                    status:
+                      description: status of the condition, one of True, False, Unknown.
+                      enum:
+                      - "True"
+                      - "False"
+                      - Unknown
+                      type: string
+                    type:
+                      description: type of condition in CamelCase or in foo.example.com/CamelCase.
+                        --- Many .condition.type values are consistent across resources
+                        like Available, but because arbitrary conditions can be useful
+                        (see .node.status.conditions), the ability to deconflict is
+                        important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
+                      maxLength: 316
+                      pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
+                      type: string
+                  required:
+                  - lastTransitionTime
+                  - message
+                  - reason
+                  - status
+                  - type
+                  type: object
+                type: array
               createdAt:
                 description: '''createdAt'' specifies the creation date and time'
                 format: date-time

diff --git a/e2e/suite_test.go b/e2e/suite_test.go
@@ -16,6 +16,9 @@ import (
 	. "github.com/onsi/gomega"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"sigs.k8s.io/yaml"
 )
 
@@ -308,7 +311,7 @@ var _ = Describe("rbd backup system", func() {
 		var backup backupv1.RBDPVCBackup
 		err = yaml.Unmarshal(stdout, &backup)
 		Expect(err).NotTo(HaveOccurred())
-		Expect(backup.Status.Conditions).To(Equal(backupv1.RBDPVCBackupConditionsBound))
+		Expect(meta.FindStatusCondition(backup.Status.Conditions, backupv1.ConditionTypeReadyToUse).Status).To(Equal(metav1.ConditionTrue))
 	})
 
 	It("should delete RBDPVCBackup resource", func() {

diff --git a/internal/controller/rbdpvcbackup_controller.go b/internal/controller/rbdpvcbackup_controller.go
@@ -11,6 +11,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -96,8 +97,8 @@ func executeCommandImpl(command []string, input io.Reader) ([]byte, error) {
 
 var executeCommand = executeCommandImpl
 
-func (r *RBDPVCBackupReconciler) updateConditions(ctx context.Context, backup *backupv1.RBDPVCBackup, conditions string) error {
-	backup.Status.Conditions = conditions
+func (r *RBDPVCBackupReconciler) updateCondition(ctx context.Context, backup *backupv1.RBDPVCBackup, condition metav1.Condition) error {
+	meta.SetStatusCondition(&backup.Status.Conditions, condition)
 	err := r.Status().Update(ctx, backup)
 	if err != nil {
 		logger.Error("failed to update status", "conditions", backup.Status.Conditions, "error", err)
@@ -114,7 +115,7 @@ func (r *RBDPVCBackupReconciler) createRBDSnapshot(ctx context.Context, poolName
 		out, err := executeCommand(command, nil)
 		if err != nil {
 			logger.Info("failed to run `rbd snap ls`", "poolName", poolName, "imageName", imageName, "error", err)
-			err2 := r.updateConditions(ctx, backup, backupv1.RBDPVCBackupConditionsFailed)
+			err2 := r.updateCondition(ctx, backup, metav1.Condition{Type: backupv1.ConditionTypeReadyToUse, Status: metav1.ConditionFalse, Reason: backupv1.ConditionReasonFailedToCreateBackup})
 			if err2 != nil {
 				return ctrl.Result{}, err2
 			}
@@ -124,7 +125,7 @@ func (r *RBDPVCBackupReconciler) createRBDSnapshot(ctx context.Context, poolName
 		err = json.Unmarshal(out, &snapshots)
 		if err != nil {
 			logger.Error("failed to unmarshal json", "json", out, "error", err)
-			err2 := r.updateConditions(ctx, backup, backupv1.RBDPVCBackupConditionsFailed)
+			err2 := r.updateCondition(ctx, backup, metav1.Condition{Type: backupv1.ConditionTypeReadyToUse, Status: metav1.ConditionFalse, Reason: backupv1.ConditionReasonFailedToCreateBackup})
 			if err2 != nil {
 				return ctrl.Result{}, err2
 			}
@@ -138,8 +139,8 @@ func (r *RBDPVCBackupReconciler) createRBDSnapshot(ctx context.Context, poolName
 			}
 		}
 		if !existSnapshot {
-			logger.Info("snapshot not exists", "snapshotName", backup.Name)
-			err2 := r.updateConditions(ctx, backup, backupv1.RBDPVCBackupConditionsFailed)
+			logger.Info("snapshot does not exists", "snapshotName", backup.Name)
+			err2 := r.updateCondition(ctx, backup, metav1.Condition{Type: backupv1.ConditionTypeReadyToUse, Status: metav1.ConditionFalse, Reason: backupv1.ConditionReasonFailedToCreateBackup})
 			if err2 != nil {
 				return ctrl.Result{}, err2
 			}
@@ -183,6 +184,7 @@ func (r *RBDPVCBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	var pvc corev1.PersistentVolumeClaim
 	err = r.Get(ctx, types.NamespacedName{Namespace: pvcNamespace, Name: pvcName}, &pvc)
 	if err != nil {
+		r.updateCondition(ctx, &backup, metav1.Condition{Status: metav1.ConditionFalse, Reason: backupv1.ConditionReasonCouldNotAccessVolume})
 		logger.Error("failed to get PVC", "namespace", pvcNamespace, "name", pvcName, "error", err)
 		if errors.IsNotFound(err) {
 			if !backup.ObjectMeta.DeletionTimestamp.IsZero() {
@@ -202,19 +204,21 @@ func (r *RBDPVCBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	if pvc.Status.Phase != corev1.ClaimBound {
+		r.updateCondition(ctx, &backup, metav1.Condition{Status: metav1.ConditionFalse, Reason: backupv1.ConditionReasonCouldNotAccessVolume})
 		if pvc.Status.Phase == corev1.ClaimPending {
 			logger.Info("waiting for PVC bound.")
 			return ctrl.Result{Requeue: true}, nil
 		} else {
-			logger.Error("failed to bound PVC", "status.phase", pvc.Status.Phase)
-			return ctrl.Result{}, fmt.Errorf("failed to bound PVC (status.phase: %s)", pvc.Status.Phase)
+			logger.Error("PVC is not bound", "status.phase", pvc.Status.Phase)
+			return ctrl.Result{}, fmt.Errorf("PVC is not bound (status.phase: %s)", pvc.Status.Phase)
 		}
 	}
 
 	pvName := pvc.Spec.VolumeName
 	var pv corev1.PersistentVolume
 	err = r.Get(ctx, types.NamespacedName{Namespace: req.NamespacedName.Namespace, Name: pvName}, &pv)
 	if err != nil {
+		r.updateCondition(ctx, &backup, metav1.Condition{Status: metav1.ConditionFalse, Reason: backupv1.ConditionReasonCouldNotAccessVolume})
 		logger.Error("failed to get PV", "namespace", req.NamespacedName.Namespace, "name", pvName, "error", err)
 		return ctrl.Result{}, err
 	}
@@ -229,14 +233,6 @@ func (r *RBDPVCBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	if !backup.ObjectMeta.DeletionTimestamp.IsZero() {
-		if backup.Status.Conditions != backupv1.RBDPVCBackupConditionsDeleting {
-			err = r.updateConditions(ctx, &backup, backupv1.RBDPVCBackupConditionsDeleting)
-			if err != nil {
-				return ctrl.Result{}, err
-			}
-			return ctrl.Result{Requeue: true}, nil
-		}
-
 		if controllerutil.ContainsFinalizer(&backup, RBDPVCBackupFinalizerName) {
 			command := []string{"rbd", "snap", "rm", poolName + "/" + imageName + "@" + backup.Name}
 			_, err = executeCommand(command, nil)
@@ -264,7 +260,9 @@ func (r *RBDPVCBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	if !controllerutil.ContainsFinalizer(&backup, RBDPVCBackupFinalizerName) {
+		r.updateCondition(ctx, &backup, metav1.Condition{Status: metav1.ConditionFalse, Reason: backupv1.ConditionReasonCreatingBackup})
 		controllerutil.AddFinalizer(&backup, RBDPVCBackupFinalizerName)
+		meta.SetStatusCondition(&backup.Status.Conditions, metav1.Condition{Type: backupv1.ConditionTypeReadyToUse, Status: metav1.ConditionFalse})
 		err = r.Update(ctx, &backup)
 		if err != nil {
 			logger.Error("failed to add finalizer", "finalizer", RBDPVCBackupFinalizerName, "error", err)
@@ -273,25 +271,17 @@ func (r *RBDPVCBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return ctrl.Result{Requeue: true}, nil
 	}
 
-	if backup.Status.Conditions == backupv1.RBDPVCBackupConditionsBound || backup.Status.Conditions == backupv1.RBDPVCBackupConditionsDeleting {
+	if meta.FindStatusCondition(backup.Status.Conditions, backupv1.ConditionTypeReadyToUse).Status == metav1.ConditionTrue {
 		return ctrl.Result{}, nil
 	}
 
-	if backup.Status.Conditions != backupv1.RBDPVCBackupConditionsCreating {
-		err := r.updateConditions(ctx, &backup, backupv1.RBDPVCBackupConditionsCreating)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-		return ctrl.Result{Requeue: true}, nil
-	}
-
 	result, err := r.createRBDSnapshot(ctx, poolName, imageName, &backup)
 	if err != nil {
 		return result, err
 	}
 
 	backup.Status.CreatedAt = metav1.NewTime(time.Now())
-	backup.Status.Conditions = backupv1.RBDPVCBackupConditionsBound
+	meta.SetStatusCondition(&backup.Status.Conditions, metav1.Condition{Type: backupv1.ConditionTypeReadyToUse, Status: metav1.ConditionTrue})
 	err = r.Status().Update(ctx, &backup)
 	if err != nil {
 		logger.Error("failed to update status", "createdAt", backup.Status.CreatedAt, "conditions", backup.Status.Conditions, "error", err)