Skip to content
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.

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

Update NAB object status appropriately if VeleroBackup already exists for the NAB object #59

Open
shubham-pampattiwar opened this issue May 6, 2024 · 7 comments
Assignees

Comments

@shubham-pampattiwar
Copy link
Member

No description provided.

@mateusoliveira43
Copy link
Contributor

Are we able to reproduce this error constantly ❓ I think we talked about this being a corner case

If it is a corner case, would not prioritize it now

@mpryc
Copy link
Collaborator

mpryc commented May 6, 2024

Agree this is corner case as we generate names based on the NAB name, namespace and hash, so it's really corner case to expect VeleroBackup to be there beforehand.

@shubham-pampattiwar
Copy link
Member Author

shubham-pampattiwar commented May 6, 2024

Right now I get the following status on NAB object if the VeleroBackup object already exists:

status:
  conditions:
  - lastTransitionTime: "2024-05-06T19:11:02Z"
    message: backup accepted
    reason: BackupAccepted
    status: "True"
    type: Accepted
  phase: New
  veleroBackupName: nab-tests-nac-7fbca55475f247
  veleroBackupNamespace: openshift-adp
  veleroBackupStatus:
    completionTimestamp: "2024-05-06T19:09:45Z"
    expiration: "2024-06-05T19:09:35Z"
    formatVersion: 1.1.0
    phase: Completed
    progress:
      itemsBackedUp: 86
      totalItems: 86
    startTimestamp: "2024-05-06T19:09:35Z"
    version: 1

I think we should add some msg related to backup already exists etc

@mateusoliveira43
Copy link
Contributor

@shubham-pampattiwar now I am confused

is not the bug you are worried, but only a feedback message that NAB created Velero Backup ❓

@shubham-pampattiwar
Copy link
Member Author

We want to keep NAB : VeleroBackup :: 1:1

@mpryc mpryc self-assigned this May 14, 2024
@shubham-pampattiwar
Copy link
Member Author

I will try to reproduce this and comment again

@mateusoliveira43
Copy link
Contributor

@shubham-pampattiwar can you test this in #73 ?

I think the error was related to the race condition I fixed there

On your example #59 (comment)

it failed here

_, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "Validated", "Valid Backup config")
if errUpdate != nil {
logger.Error(errUpdate, "Unable to set BackupAccepted Condition: True", nameField, nab.Name, constant.NameSpaceString, nab.Namespace)
return true, false, errUpdate
}
_, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionQueued, metav1.ConditionTrue, "BackupScheduled", "Created Velero Backup object")
if errUpdate != nil {
logger.Error(errUpdate, "Unable to set BackupQueued Condition: True", nameField, nab.Name, constant.NameSpaceString, nab.Namespace)
return true, false, errUpdate
}

and on the next reconcile it went here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready for Review
Development

No branches or pull requests

3 participants