CRD updates for the oc CLI fields for the NAR, NAB and NABSL objects #245

mpryc · 2025-02-28T10:59:35Z

Depends-On: openshift/oadp-operator#1648

Implements #232 with additional fields for:

NonAdminBackup
NonAdminRestore
NonAdminBackupStorageLocation

Why the changes were made

To fix #232 and add similar fields for the NAB and NAR objects.

How to test the changes made

Deployed with local oadp-operator deployment
Tested various objects and it's states:

Prior admin approval or rejections

Created couple of BSLs - names reflect the idea behind each of them

suspicious-nabsl (reject)
nonexistingsecretinns-nabsl (do nothing - no nabslrequest)
waitingapproval-nabsl (do nothing)
incorrect-bucket-nabsl (approve)
perfect-nabsl (approve)

Before any admin action:

$ oc get nabsl -n non-admin-bsl-test
NAME                          APPROVED   BSL-STATUS   STATUS       AGE
incorrect-bucket-nabsl        False                   New          2m11s
nonexistingsecretinns-nabsl                           BackingOff   39s
perfect-nabsl                 False                   New          2m40s
suspicious-sample             False                   New          10s
waitingapproval-nabsl         False                   New          94s

Note the nonexistingsecretinns-nabsl have not created request - the user must create secret to get to that state

$ oc get nabslrequest
NAME                                                              NABSL-NAMESPACE      NABSL-NAME               STATUS    AGE
non-admin-bsl-test-incorre-9aa22872-f39d-427c-b352-6924ff9c2175   non-admin-bsl-test   incorrect-bucket-nabsl   Pending   2m48s
non-admin-bsl-test-perfect-4413c67d-8eb8-42bc-8274-279a91895196   non-admin-bsl-test   perfect-nabsl            Pending   3m17s
non-admin-bsl-test-suspici-4fbdcebf-2277-4f44-890e-35d765815e1a   non-admin-bsl-test   suspicious-sample        Pending   47s
non-admin-bsl-test-waiting-fcae28c2-d8ea-48e2-a35c-8882bfd865e0   non-admin-bsl-test   waitingapproval-nabsl    Pending   2m11s

Approving or rejecting some:

$ oc get nabslrequest
NAME                                                              NABSL-NAMESPACE      NABSL-NAME               STATUS     AGE
non-admin-bsl-test-incorre-9aa22872-f39d-427c-b352-6924ff9c2175   non-admin-bsl-test   incorrect-bucket-nabsl   Approved   4m57s
non-admin-bsl-test-perfect-4413c67d-8eb8-42bc-8274-279a91895196   non-admin-bsl-test   perfect-nabsl            Approved   5m26s
non-admin-bsl-test-suspici-4fbdcebf-2277-4f44-890e-35d765815e1a   non-admin-bsl-test   suspicious-sample        Rejected   2m56s
non-admin-bsl-test-waiting-fcae28c2-d8ea-48e2-a35c-8882bfd865e0   non-admin-bsl-test   waitingapproval-nabsl    Pending    4m20s

Now the NABSL objects

$ oc get nabsl -n non-admin-bsl-test
NAME                          APPROVED   BSL-STATUS    STATUS       AGE
incorrect-bucket-nabsl        True       Unavailable   Created      8m25s
nonexistingsecretinns-nabsl                            BackingOff   6m53s
perfect-nabsl                 True       Available     Created      8m54s
suspicious-sample             False                    BackingOff   6m24s
waitingapproval-nabsl         False                    New          7m48s

Backup

Couple of backup objects were created

$ oc get nab -n non-admin-bsl-test
NAME                       BACKUP-STATUS         STATUS       AGE
backup-failed-sample       PartiallyFailed       Created      6s
incorrect-bsl-backup                             BackingOff   3m39s
some-backup                Completed             Created      13s

Restore

Nice InProgress and Completed status:

$ oc get nar -n non-admin-bsl-test
NAME               RESTORE-STATUS   STATUS       AGE
non-existing-nab                    BackingOff   46s

$ oc get nar -n non-admin-bsl-test
NAME               RESTORE-STATUS   STATUS       AGE
existing-nab       InProgress       Created      1s
non-existing-nab                    BackingOff   51s

$ oc get nar -n non-admin-bsl-test
NAME               RESTORE-STATUS   STATUS       AGE
existing-nab       Completed        Created      3s
non-existing-nab                    BackingOff   53s

Implements migtools#232 with additional fields for: - NonAdminBackup - NonAdminRestore - NonAdminBackupStorageLocation Signed-off-by: Michal Pryc <[email protected]>

openshift-ci · 2025-02-28T10:59:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mpryc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mpryc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mateusoliveira43 · 2025-03-06T12:35:32Z

/hold OADP PR

mateusoliveira43 · 2025-03-06T12:38:19Z

api/v1alpha1/nonadminbackup_types.go

@@ -98,6 +98,9 @@ type NonAdminBackupStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:path=nonadminbackups,shortName=nab
+// +kubebuilder:printcolumn:name="Backup-Status",type="string",JSONPath=".status.veleroBackup.status.phase"


name can not contain spaces?

from velero

$ velero backup-location get NAME PROVIDER BUCKET/PREFIX PHASE LAST VALIDATED ACCESS MODE DEFAULT velero-sample-1 aws my-bucket-name/velero Unavailable 2025-03-06 12:54:30 +0000 UTC ReadWrite true $ velero backup get NAME STATUS ERRORS WARNINGS CREATED EXPIRES STORAGE LOCATION SELECTOR backup Failed 0 0 2025-03-06 12:56:14 +0000 UTC 29d velero-sample-1 <none> $ velero restore get NAME BACKUP STATUS STARTED COMPLETED ERRORS WARNINGS CREATED SELECTOR restore backup Failed 2025-03-06 12:56:45 +0000 UTC 2025-03-06 12:56:46 +0000 UTC 0 0 2025-03-06 12:56:45 +0000 UTC <none>

it makes sense to call Backup Status and Status, but I think it would be better Backup Phase and Phase,

How about this?

openshift/oadp-operator#1648 (comment)

mateusoliveira43 · 2025-03-06T13:05:34Z

api/v1alpha1/nonadminbackupstoragelocation_types.go

@@ -71,6 +71,10 @@ type NonAdminBackupStorageLocationStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:path=nonadminbackupstoragelocations,shortName=nabsl
+// +kubebuilder:printcolumn:name="Approved",type="string",JSONPath=".status.conditions[?(@.type=='ClusterAdminApproved')].status"
+// +kubebuilder:printcolumn:name="BSL-Status",type="string",JSONPath=".status.veleroBackupStorageLocation.status.phase"


from velero

$ velero backup-location get NAME PROVIDER BUCKET/PREFIX PHASE LAST VALIDATED ACCESS MODE DEFAULT velero-sample-1 aws my-bucket-name/velero Unavailable 2025-03-06 12:54:30 +0000 UTC ReadWrite true $ velero backup get NAME STATUS ERRORS WARNINGS CREATED EXPIRES STORAGE LOCATION SELECTOR backup Failed 0 0 2025-03-06 12:56:14 +0000 UTC 29d velero-sample-1 <none>

would change BSL-Status to Storage Location Phase (if spaces are allowed) and Status to Phase

does the order matter?

currently is APPROVED -> BSL-Phase -> Phase (NABSL)

would be better to change to NABSL phase -> Approved -> Storage Location Phase?

I would really not like to have spaces as this isn't a good CLI/Shell practice in my opinion. It's hard to use awk/sed/cut for those as well they make have affect on the alignment in various terminal systems.

mateusoliveira43 · 2025-03-06T13:23:01Z

api/v1alpha1/nonadminbackupstoragelocationrequest_types.go

@@ -84,6 +84,10 @@ type NonAdminBackupStorageLocationRequestStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:path=nonadminbackupstoragelocationrequests,shortName=nabslrequest
+// +kubebuilder:printcolumn:name="NABSL-Namespace",type="string",JSONPath=".status.nonAdminBackupStorageLocation.namespace"
+// +kubebuilder:printcolumn:name="NABSL-Name",type="string",JSONPath=".status.nonAdminBackupStorageLocation.name"


would drop NABSL prefix in namespace title. having just namespace I think is enough m(this is a request for NAMESPACE X)

would also drop NAME suffix for NABSL

This object is mostly viewable by the cluster admin to interact with and the namespace and name is the one that requested the BSL creation, we can drop it, but would this be easy enough to understand without reading documentation ?

CRD updates for the oc CLI fields for the NAR, NAB and NABSL objects

bdb2859

Implements migtools#232 with additional fields for: - NonAdminBackup - NonAdminRestore - NonAdminBackupStorageLocation Signed-off-by: Michal Pryc <[email protected]>

openshift-ci bot requested review from mrnold and sseago February 28, 2025 10:59

openshift-ci bot added the approved label Feb 28, 2025

mpryc requested review from weshayutin and shubham-pampattiwar February 28, 2025 10:59

mpryc mentioned this pull request Feb 28, 2025

CRD updates for the oc CLI fields for the NAR, NAB and NABSL objects openshift/oadp-operator#1648

Open

openshift-ci bot added the do-not-merge/hold label Mar 6, 2025

mateusoliveira43 reviewed Mar 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CRD updates for the oc CLI fields for the NAR, NAB and NABSL objects #245

CRD updates for the oc CLI fields for the NAR, NAB and NABSL objects #245

mpryc commented Feb 28, 2025 •

edited by mateusoliveira43

Loading

openshift-ci bot commented Feb 28, 2025

mateusoliveira43 commented Mar 6, 2025

mateusoliveira43 Mar 6, 2025

mateusoliveira43 Mar 6, 2025

mpryc Mar 7, 2025

mateusoliveira43 Mar 6, 2025

mateusoliveira43 Mar 6, 2025

mpryc Mar 7, 2025

mateusoliveira43 Mar 6, 2025

mpryc Mar 7, 2025

CRD updates for the oc CLI fields for the NAR, NAB and NABSL objects #245

Are you sure you want to change the base?

CRD updates for the oc CLI fields for the NAR, NAB and NABSL objects #245

Conversation

mpryc commented Feb 28, 2025 • edited by mateusoliveira43 Loading

Why the changes were made

How to test the changes made

Prior admin approval or rejections

Created couple of BSLs - names reflect the idea behind each of them

Before any admin action:

Backup

Restore

openshift-ci bot commented Feb 28, 2025

mateusoliveira43 commented Mar 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpryc commented Feb 28, 2025 •

edited by mateusoliveira43

Loading