-
Notifications
You must be signed in to change notification settings - Fork 89
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
PB-4991: Update ApplicationBackupController to execute the post-exec … #1602
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -265,13 +265,40 @@ func (a *ApplicationBackupController) createBackupLocationPath(backup *stork_api | |
func (a *ApplicationBackupController) handle(ctx context.Context, backup *stork_api.ApplicationBackup) error { | ||
if backup.DeletionTimestamp != nil { | ||
if controllers.ContainsFinalizer(backup, controllers.FinalizerCleanup) { | ||
// Run the post exec rules if the backup is in ApplicationBackupStageVolumes stage(After the ApplicationBackupStagePreExecRule Stage) AND execRulesCompleted check is negative | ||
if backup.Status.Stage == stork_api.ApplicationBackupStageVolumes { | ||
log.ApplicationBackupLog(backup).WithField("Event", "Finalizer Cleanup").Infof("BackpCR is marked for deletion during the %v stage", backup.Status.Stage) | ||
if terminationChannels, ok := a.terminationChannels[string(backup.UID)]; ok { | ||
for _, channel := range terminationChannels { | ||
log.ApplicationBackupLog(backup).WithField("Event", "Finalizer Cleanup").Info("Sending termination commands to kill pre-exec pod") | ||
channel <- true | ||
} | ||
} | ||
if isexecRulesCompleted, isExists := a.execRulesCompleted[string(backup.UID)]; backup.Spec.PostExecRule != "" && isExists && !isexecRulesCompleted { | ||
log.ApplicationBackupLog(backup).WithField("Event", "Finalizer Cleanup").Info("Starting post-exec rule during the backup cr finalizer cleanup") | ||
err := a.runPostExecRule(backup) | ||
if err != nil && !k8s_errors.IsNotFound(err) { | ||
message := fmt.Sprintf("Error running PostExecRule during the finalizer cleanup: %v", err) | ||
log.ApplicationBackupLog(backup).WithField("Event", "Finalizer Cleanup").Errorf(message) | ||
a.recorder.Event(backup, | ||
v1.EventTypeWarning, | ||
string(stork_api.ApplicationBackupStatusFailed), | ||
message) | ||
return fmt.Errorf("%v", message) | ||
} | ||
a.execRulesCompleted[string(backup.UID)] = true | ||
} | ||
} | ||
|
||
canDelete, err := a.deleteBackup(backup) | ||
if err != nil { | ||
logrus.Errorf("%s: cleanup: %s", reflect.TypeOf(a), err) | ||
} | ||
if !canDelete { | ||
log.ApplicationBackupLog(backup).Infof("Is backup [%v/%v] can be deleted: value is %v ", backup.Namespace, backup.Name, canDelete) | ||
return nil | ||
} | ||
|
||
// get-rid of map entry for termination channel and rule flag | ||
if _, ok := a.terminationChannels[string(backup.UID)]; ok { | ||
delete(a.terminationChannels, string(backup.UID)) | ||
|
@@ -289,17 +316,23 @@ func (a *ApplicationBackupController) handle(ctx context.Context, backup *stork_ | |
delete(a.vmIncludeResourceMap, string(backup.UID)) | ||
log.ApplicationBackupLog(backup).Infof("cleaning up vmIncludeResources for VM backupObjectType") | ||
} | ||
// Calling cleanupResources which will cleanup the resources created by applicationbackup controller. | ||
// Calling cleanupResources which will cleanup the resources created by applicationbackup controller. Including the post exec rule CR for manual backup when created through the px-backup | ||
// In the case of kdmp driver, it will cleanup the dataexport CRs. | ||
err = a.cleanupResources(backup) | ||
if err != nil { | ||
log.ApplicationBackupLog(backup).Errorf("Error while cleanupResources which will cleanup the resources created by applicationbackup controller [%v/%v]: %v", backup.Namespace, backup.Name, err) | ||
return err | ||
} | ||
} | ||
|
||
if backup.GetFinalizers() != nil { | ||
controllers.RemoveFinalizer(backup, controllers.FinalizerCleanup) | ||
return a.client.Update(ctx, backup) | ||
err := a.client.Update(ctx, backup) | ||
if err != nil { | ||
log.ApplicationBackupLog(backup).Errorf("Error while updating applicationbackup [%v/%v]: %v", backup.Namespace, backup.Name, err) | ||
return err | ||
} | ||
return nil | ||
} | ||
|
||
return nil | ||
|
@@ -309,6 +342,12 @@ func (a *ApplicationBackupController) handle(ctx context.Context, backup *stork_ | |
if backup.Status.Stage == stork_api.ApplicationBackupStageFinal { | ||
return nil | ||
} | ||
|
||
// Initialize execRulesCompleted for the backup UID | ||
if _, isExists := a.execRulesCompleted[string(backup.UID)]; backup.Spec.PostExecRule != "" && !isExists { | ||
a.execRulesCompleted[string(backup.UID)] = false | ||
} | ||
|
||
if labelSelector := backup.Spec.NamespaceSelector; len(labelSelector) != 0 { | ||
var pxNs string | ||
namespaces, err := core.Instance().ListNamespacesV2(labelSelector) | ||
|
@@ -620,8 +659,21 @@ func (a *ApplicationBackupController) backupVolumes(backup *stork_api.Applicatio | |
backupStatusVolMap[statusVolume.Namespace+"-"+statusVolume.PersistentVolumeClaim] = "" | ||
} | ||
|
||
namespacedName := types.NamespacedName{Namespace: backup.Namespace, Name: backup.Name} | ||
backup.Status.Stage = stork_api.ApplicationBackupStageVolumes | ||
namespacedName := types.NamespacedName{} | ||
|
||
backup, err = a.updateBackupCRInVolumeStage( | ||
namespacedName, | ||
stork_api.ApplicationBackupStatusInProgress, | ||
backup.Status.Stage, | ||
"Starting the Volume backups", | ||
nil, | ||
) | ||
if err != nil { | ||
logrus.Errorf("Error while updateBackupCRInVolumeStage: %v", err) | ||
return err | ||
} | ||
|
||
if a.IsVolsToBeBackedUp(backup) { | ||
isResourceTypePVC := IsResourceTypePVC(backup) | ||
var objectMap map[stork_api.ObjectInfo]bool | ||
|
@@ -710,8 +762,6 @@ func (a *ApplicationBackupController) backupVolumes(backup *stork_api.Applicatio | |
backup.Status.Volumes = make([]*stork_api.ApplicationBackupVolumeInfo, 0) | ||
} | ||
|
||
namespacedName.Namespace = backup.Namespace | ||
namespacedName.Name = backup.Name | ||
if len(backup.Status.Volumes) != pvcCount { | ||
for driverName, pvcs := range pvcMappings { | ||
var driver volume.Driver | ||
|
@@ -938,38 +988,41 @@ func (a *ApplicationBackupController) backupVolumes(backup *stork_api.Applicatio | |
driverCombo := a.checkVolumeDriverCombination(backup.Status.Volumes) | ||
// If the driver combination of volumes onlykdmp or mixed of both kdmp and non-kdmp, call post exec rule | ||
// backup of volume is success. | ||
if driverCombo == kdmpDriverOnly || driverCombo == mixedDriver { | ||
// Let's kill the pre-exec rule pod here so that application specific | ||
// data stream freezing logic works. Certain app actually unleash the WRITE when session ends. | ||
// At this point we are dead sure that volume snapshot for all PVCs in the APP is done.. if not then | ||
// there is issue... | ||
// For detail refer pb-3823 | ||
for _, channel := range terminationChannels { | ||
logrus.Infof("Sending termination commands to kill pre-exec pod in kdmp or mixed driver path") | ||
channel <- true | ||
} | ||
//terminationChannels = nil | ||
if backup.Spec.PostExecRule != "" { | ||
log.ApplicationBackupLog(backup).Infof("Starting post-exec rule for kdmp and mixed driver path") | ||
err = a.runPostExecRule(backup) | ||
if err != nil { | ||
message := fmt.Sprintf("Error running PostExecRule for kdmp and mixed driver scenario: %v", err) | ||
log.ApplicationBackupLog(backup).Errorf(message) | ||
a.recorder.Event(backup, | ||
v1.EventTypeWarning, | ||
string(stork_api.ApplicationBackupStatusFailed), | ||
message) | ||
|
||
backup.Status.Stage = stork_api.ApplicationBackupStageFinal | ||
backup.Status.FinishTimestamp = metav1.Now() | ||
backup.Status.LastUpdateTimestamp = metav1.Now() | ||
backup.Status.Status = stork_api.ApplicationBackupStatusFailed | ||
backup.Status.Reason = message | ||
err = a.client.Update(context.TODO(), backup) | ||
if !a.execRulesCompleted[string(backup.UID)] { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lalat-das Was there any reason, why we did not had it check in the previous fix? Is it because, it is the last step? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was the comment I had given earlier, guess it was missed incorporating. |
||
if driverCombo == kdmpDriverOnly || driverCombo == mixedDriver { | ||
// Let's kill the pre-exec rule pod here so that application specific | ||
// data stream freezing logic works. Certain app actually unleash the WRITE when session ends. | ||
// At this point we are dead sure that volume snapshot for all PVCs in the APP is done.. if not then | ||
// there is issue... | ||
// For detail refer pb-3823 | ||
for _, channel := range terminationChannels { | ||
logrus.Infof("Sending termination commands to kill pre-exec pod in kdmp or mixed driver path") | ||
channel <- true | ||
} | ||
//terminationChannels = nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the commented code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This commented code is an existing one not part of the this PR change. |
||
if backup.Spec.PostExecRule != "" { | ||
log.ApplicationBackupLog(backup).Infof("Starting post-exec rule for kdmp and mixed driver path") | ||
err = a.runPostExecRule(backup) | ||
if err != nil { | ||
return err | ||
message := fmt.Sprintf("Error running PostExecRule for kdmp and mixed driver scenario: %v", err) | ||
log.ApplicationBackupLog(backup).Errorf(message) | ||
a.recorder.Event(backup, | ||
v1.EventTypeWarning, | ||
string(stork_api.ApplicationBackupStatusFailed), | ||
message) | ||
|
||
backup.Status.Stage = stork_api.ApplicationBackupStageFinal | ||
backup.Status.FinishTimestamp = metav1.Now() | ||
backup.Status.LastUpdateTimestamp = metav1.Now() | ||
backup.Status.Status = stork_api.ApplicationBackupStatusFailed | ||
backup.Status.Reason = message | ||
err = a.client.Update(context.TODO(), backup) | ||
if err != nil { | ||
return err | ||
} | ||
return fmt.Errorf("%v", message) | ||
} | ||
return fmt.Errorf("%v", message) | ||
a.execRulesCompleted[string(backup.UID)] = true | ||
} | ||
} | ||
} | ||
|
@@ -2141,6 +2194,18 @@ func IsResourceTypePVC(backup *stork_api.ApplicationBackup) bool { | |
func (a *ApplicationBackupController) cleanupResources( | ||
backup *stork_api.ApplicationBackup, | ||
) error { | ||
// Delete post-exec rule CR only for the manual backup as it is managed and created through the px-backup | ||
// And also here the deletion will be skipped if the backupCR is created by the backup schedule or for the restore from the px-backup. | ||
if len(backup.Spec.PostExecRule) != 0 && backup.Annotations[utils.PxbackupAnnotationCreateByKey] == utils.PxbackupAnnotationCreateByValue && backup.Annotations["portworx.io/backup-by"] != "backup-schedule" && backup.Annotations["portworx.io/backup-by"] != "restore" { | ||
log.ApplicationBackupLog(backup).WithField("Event", "Finalizer Cleanup").Info("Delete post-exec rule CR as it is a manual backup created by the px-backup") | ||
|
||
err := storkops.Instance().DeleteRule(backup.Spec.PostExecRule, backup.Namespace) | ||
if err != nil && !k8s_errors.IsNotFound(err) { | ||
log.ApplicationBackupLog(backup).WithField("Event", "Finalizer Cleanup").Errorf("Error while deleting post exec rule CR: %v", err) | ||
return err | ||
} | ||
} | ||
|
||
drivers := a.getDriversForBackup(backup) | ||
for driverName := range drivers { | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is redundant code. You already checking ifExists..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is in finalizer here it is during initialization.