Skip to content

Commit 7c774d7

Browse files
suaas21hossainemruz
authored and
tamalsaha
committed
Fix backup-batch issues (#1004)
* Add tests for BackupBatch + ensure hook execution order for BackupBatch * Fix RC tests failure * Fix statefulset test failure issue due to cleanup Co-authored-by: Md. Emruz Hossain <hossainemruz@gmail.com>
1 parent 5bf1e2c commit 7c774d7

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+1056
-978
lines changed

‎pkg/backup/backupsession.go

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"os"
23+
"reflect"
2324
"strings"
2425
"time"
2526

@@ -145,6 +146,21 @@ func (c *BackupSessionController) initBackupSessionWatcher() error {
145146
queue.Enqueue(c.bsQueue.GetQueue(), backupsession)
146147
}
147148
},
149+
UpdateFunc: func(oldObj, newObj interface{}) {
150+
oldBS, ok := oldObj.(*api_v1beta1.BackupSession)
151+
if !ok {
152+
glog.Errorf("Invalid BackupSession Object")
153+
return
154+
}
155+
newBS, ok := newObj.(*api_v1beta1.BackupSession)
156+
if !ok {
157+
glog.Errorf("Invalid BackupSession Object")
158+
return
159+
}
160+
if !reflect.DeepEqual(&oldBS.Status, &newBS.Status) {
161+
queue.Enqueue(c.bsQueue.GetQueue(), newObj)
162+
}
163+
},
148164
}, selector))
149165
c.bsLister = c.StashInformerFactory.Stash().V1beta1().BackupSessions().Lister()
150166
return nil
@@ -199,7 +215,12 @@ func (c *BackupSessionController) startBackupProcess(backupSession *api_v1beta1.
199215

200216
// if BackupSession already has been processed for this host then skip further processing
201217
if c.isBackupTakenForThisHost(backupSession, targetInfo.Target) {
202-
log.Infof("Skip processing BackupSession %s/%s. Reason: BackupSession has been processed already for host %q\n", backupSession.Namespace, backupSession.Name, c.Host)
218+
log.Infof("Skip processing BackupSession %s/%s. Reason: BackupSession has been processed already for host %q", backupSession.Namespace, backupSession.Name, c.Host)
219+
return nil, nil
220+
}
221+
222+
if !isBackupInitiated(backupSession) {
223+
log.Infof("Skip processing BackupSession %s/%s. Reason: Backup process is not initiated by it's operator", backupSession.Namespace, backupSession.Name)
203224
return nil, nil
204225
}
205226

@@ -247,6 +268,7 @@ func (c *BackupSessionController) backup(invoker apis.Invoker, targetInfo apis.T
247268

248269
if c.InvokerType == api_v1beta1.ResourceKindBackupBatch {
249270
c.SetupOpt.Path = fmt.Sprintf("%s/%s/%s", c.SetupOpt.Path, strings.ToLower(c.BackupTargetKind), c.BackupTargetName)
271+
250272
}
251273

252274
// apply nice, ionice settings from env
@@ -265,8 +287,8 @@ func (c *BackupSessionController) backup(invoker apis.Invoker, targetInfo apis.T
265287
return nil, err
266288
}
267289

268-
// BackupOptions configuration
269-
backupOpt := util.BackupOptionsForBackupConfig(targetInfo.Target, invoker.RetentionPolicy, extraOpt)
290+
// BackupOptions backup target
291+
backupOpt := util.BackupOptionsForBackupTarget(targetInfo.Target, invoker.RetentionPolicy, extraOpt)
270292
// Run Backup
271293
// If there is an error during backup, don't return.
272294
// We will execute postBackup hook even if the backup failed.
@@ -507,3 +529,7 @@ func (c *BackupSessionController) isBackupTakenForThisHost(backupSession *api_v1
507529
}
508530
return false
509531
}
532+
533+
func isBackupInitiated(bs *api_v1beta1.BackupSession) bool {
534+
return bs.Status.Phase == api_v1beta1.BackupSessionRunning
535+
}

‎pkg/controller/backup_session.go

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -163,18 +163,6 @@ func (c *StashController) applyBackupSessionReconciliationLogic(backupSession *a
163163
return c.setBackupSessionSkipped(backupSession, fmt.Sprintf("backup invoker %s %s/%s is paused", invoker.ObjectRef.Kind, invoker.ObjectRef.Namespace, invoker.ObjectRef.Name))
164164
}
165165

166-
// skip if backup model is sidecar.
167-
// for sidecar model controller inside sidecar will take care of it.
168-
for _, targetInfo := range invoker.TargetsInfo {
169-
if targetInfo.Target != nil && invoker.Driver != api_v1beta1.VolumeSnapshotter && util.BackupModel(targetInfo.Target.Ref.Kind) == apis.ModelSidecar {
170-
log.Infof("Skipping processing BackupSession %s/%s. Reason: Backup model is sidecar. Controller inside sidecar will take care of it.", backupSession.Namespace, backupSession.Name)
171-
backupSession, err = c.setBackupSessionRunning(targetInfo.Target, invoker.Driver, backupSession)
172-
if err != nil {
173-
return err
174-
}
175-
}
176-
}
177-
178166
// if preBackup hook exist, then execute preBackupHook
179167
if invoker.Hooks != nil && invoker.Hooks.PreBackup != nil {
180168
err = util.ExecuteHook(c.clientConfig, invoker.Hooks.PreBackup, apis.PreBackupHook, os.Getenv("MY_POD_NAME"), os.Getenv("MY_POD_NAMESPACE"))
@@ -183,32 +171,32 @@ func (c *StashController) applyBackupSessionReconciliationLogic(backupSession *a
183171
}
184172
}
185173

186-
// if VolumeSnapshotter driver is used then ensure VolumeSnapshotter job and return
187-
if invoker.Driver == api_v1beta1.VolumeSnapshotter {
188-
for _, targetInfo := range invoker.TargetsInfo {
189-
if targetInfo.Target != nil {
174+
for i, targetInfo := range invoker.TargetsInfo {
175+
if targetInfo.Target != nil {
176+
// skip if backup model is sidecar.
177+
// for sidecar model controller inside sidecar will take care of it.
178+
if invoker.Driver != api_v1beta1.VolumeSnapshotter && util.BackupModel(targetInfo.Target.Ref.Kind) == apis.ModelSidecar {
179+
log.Infof("Skipping processing BackupSession %s/%s. Reason: Backup model is sidecar. Controller inside sidecar will take care of it.", backupSession.Namespace, backupSession.Name)
180+
}
181+
182+
// if VolumeSnapshotter driver is used then ensure VolumeSnapshotter job and return
183+
if invoker.Driver == api_v1beta1.VolumeSnapshotter {
190184
err = c.ensureVolumeSnapshotterJob(invoker, targetInfo, backupSession)
191185
if err != nil {
192186
return c.handleBackupJobCreationFailure(invoker, backupSession, err)
193187
}
194-
backupSession, err = c.setBackupSessionRunning(targetInfo.Target, invoker.Driver, backupSession)
188+
}
189+
190+
// Restic driver has been used. Now, create a backup job
191+
if util.BackupModel(targetInfo.Target.Ref.Kind) != apis.ModelSidecar {
192+
err = c.ensureBackupJob(invoker, targetInfo, backupSession, i)
195193
if err != nil {
196-
return err
194+
// failed to ensure backup job. set BackupSession phase "Failed" and send failure metrics.
195+
return c.handleBackupJobCreationFailure(invoker, backupSession, err)
197196
}
198197
}
199-
}
200-
return nil
201-
}
202198

203-
// Restic driver has been used. Now, create a backup job
204-
for i, targetInfo := range invoker.TargetsInfo {
205-
if targetInfo.Target != nil && util.BackupModel(targetInfo.Target.Ref.Kind) != apis.ModelSidecar {
206-
err = c.ensureBackupJob(invoker, targetInfo, backupSession, i)
207-
if err != nil {
208-
// failed to ensure backup job. set BackupSession phase "Failed" and send failure metrics.
209-
return c.handleBackupJobCreationFailure(invoker, backupSession, err)
210-
}
211-
// Backup job has been created successfully. Set BackupSession phase "Running"
199+
// Set BackupSession phase "Running"
212200
backupSession, err = c.setBackupSessionRunning(targetInfo.Target, invoker.Driver, backupSession)
213201
if err != nil {
214202
return err

‎pkg/util/options.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type ExtraOptions struct {
3333
EnableCache bool
3434
}
3535

36-
func BackupOptionsForBackupConfig(backupTarget *api.BackupTarget, retentionPolicy api_v1alpha1.RetentionPolicy, extraOpt ExtraOptions) restic.BackupOptions {
36+
func BackupOptionsForBackupTarget(backupTarget *api.BackupTarget, retentionPolicy api_v1alpha1.RetentionPolicy, extraOpt ExtraOptions) restic.BackupOptions {
3737
backupOpt := restic.BackupOptions{
3838
Host: extraOpt.Host,
3939
RetentionPolicy: retentionPolicy,

‎test/e2e/advance-configuration/runtimesettings.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ var _ = Describe("Advanced Configuration", func() {
7979
Expect(err).NotTo(HaveOccurred())
8080

8181
// Take an Instant Backup of the Sample Data
82-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
82+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
8383
Name: backupConfig.Name,
8484
Kind: v1beta1.ResourceKindBackupConfiguration,
8585
})

‎test/e2e/auto-backup/daemonset.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ var _ = Describe("Auto-Backup", func() {
8383
Expect(err).NotTo(HaveOccurred())
8484

8585
// Take an Instant Backup of the Sample Data
86-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
86+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
8787
Name: backupConfig.Name,
8888
Kind: v1beta1.ResourceKindBackupConfiguration,
8989
})
@@ -129,7 +129,7 @@ var _ = Describe("Auto-Backup", func() {
129129
Expect(err).NotTo(HaveOccurred())
130130

131131
// Take an Instant Backup of the Sample Data
132-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
132+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
133133
Name: backupConfig.Name,
134134
Kind: v1beta1.ResourceKindBackupConfiguration,
135135
})
@@ -169,7 +169,7 @@ var _ = Describe("Auto-Backup", func() {
169169
Expect(err).NotTo(HaveOccurred())
170170

171171
// Take an Instant Backup of the Sample Data
172-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
172+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
173173
Name: backupConfig.Name,
174174
Kind: v1beta1.ResourceKindBackupConfiguration,
175175
})
@@ -228,7 +228,7 @@ var _ = Describe("Auto-Backup", func() {
228228
Expect(err).NotTo(HaveOccurred())
229229

230230
// Take an Instant Backup of the Sample Data
231-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
231+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
232232
Name: backupConfig.Name,
233233
Kind: v1beta1.ResourceKindBackupConfiguration,
234234
})

‎test/e2e/auto-backup/deployment.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ var _ = Describe("Auto-Backup", func() {
8282
Expect(err).NotTo(HaveOccurred())
8383

8484
// Take an Instant Backup of the Sample Data
85-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
85+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
8686
Name: backupConfig.Name,
8787
Kind: v1beta1.ResourceKindBackupConfiguration,
8888
})
@@ -128,7 +128,7 @@ var _ = Describe("Auto-Backup", func() {
128128
Expect(err).NotTo(HaveOccurred())
129129

130130
// Take an Instant Backup of the Sample Data
131-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
131+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
132132
Name: backupConfig.Name,
133133
Kind: v1beta1.ResourceKindBackupConfiguration,
134134
})
@@ -168,7 +168,7 @@ var _ = Describe("Auto-Backup", func() {
168168
Expect(err).NotTo(HaveOccurred())
169169

170170
// Take an Instant Backup of the Sample Data
171-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
171+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
172172
Name: backupConfig.Name,
173173
Kind: v1beta1.ResourceKindBackupConfiguration,
174174
})
@@ -226,7 +226,7 @@ var _ = Describe("Auto-Backup", func() {
226226
Expect(err).NotTo(HaveOccurred())
227227

228228
// Take an Instant Backup of the Sample Data
229-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
229+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
230230
Name: backupConfig.Name,
231231
Kind: v1beta1.ResourceKindBackupConfiguration,
232232
})

‎test/e2e/auto-backup/pvc.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ var _ = Describe("Auto-Backup", func() {
8585
Expect(err).NotTo(HaveOccurred())
8686

8787
// Take an Instant Backup of the Sample Data
88-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
88+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
8989
Name: backupConfig.Name,
9090
Kind: v1beta1.ResourceKindBackupConfiguration,
9191
})
@@ -135,7 +135,7 @@ var _ = Describe("Auto-Backup", func() {
135135
Expect(err).NotTo(HaveOccurred())
136136

137137
// Take an Instant Backup of the Sample Data
138-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
138+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
139139
Name: backupConfig.Name,
140140
Kind: v1beta1.ResourceKindBackupConfiguration,
141141
})
@@ -179,7 +179,7 @@ var _ = Describe("Auto-Backup", func() {
179179
Expect(err).NotTo(HaveOccurred())
180180

181181
// Take an Instant Backup of the Sample Data
182-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
182+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
183183
Name: backupConfig.Name,
184184
Kind: v1beta1.ResourceKindBackupConfiguration,
185185
})

‎test/e2e/auto-backup/replicaset.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ var _ = Describe("Auto-Backup", func() {
8383
Expect(err).NotTo(HaveOccurred())
8484

8585
// Take an Instant Backup of the Sample Data
86-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
86+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
8787
Name: backupConfig.Name,
8888
Kind: v1beta1.ResourceKindBackupConfiguration,
8989
})
@@ -129,7 +129,7 @@ var _ = Describe("Auto-Backup", func() {
129129
Expect(err).NotTo(HaveOccurred())
130130

131131
// Take an Instant Backup of the Sample Data
132-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
132+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
133133
Name: backupConfig.Name,
134134
Kind: v1beta1.ResourceKindBackupConfiguration,
135135
})
@@ -169,7 +169,7 @@ var _ = Describe("Auto-Backup", func() {
169169
Expect(err).NotTo(HaveOccurred())
170170

171171
// Take an Instant Backup of the Sample Data
172-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
172+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
173173
Name: backupConfig.Name,
174174
Kind: v1beta1.ResourceKindBackupConfiguration,
175175
})
@@ -227,7 +227,7 @@ var _ = Describe("Auto-Backup", func() {
227227
Expect(err).NotTo(HaveOccurred())
228228

229229
// Take an Instant Backup of the Sample Data
230-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
230+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
231231
Name: backupConfig.Name,
232232
Kind: v1beta1.ResourceKindBackupConfiguration,
233233
})

‎test/e2e/auto-backup/replicationcontroller.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ var _ = Describe("Auto-Backup", func() {
8383
Expect(err).NotTo(HaveOccurred())
8484

8585
// Take an Instant Backup of the Sample Data
86-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
86+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
8787
Name: backupConfig.Name,
8888
Kind: v1beta1.ResourceKindBackupConfiguration,
8989
})
@@ -129,7 +129,7 @@ var _ = Describe("Auto-Backup", func() {
129129
Expect(err).NotTo(HaveOccurred())
130130

131131
// Take an Instant Backup of the Sample Data
132-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
132+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
133133
Name: backupConfig.Name,
134134
Kind: v1beta1.ResourceKindBackupConfiguration,
135135
})
@@ -169,7 +169,7 @@ var _ = Describe("Auto-Backup", func() {
169169
Expect(err).NotTo(HaveOccurred())
170170

171171
// Take an Instant Backup of the Sample Data
172-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
172+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
173173
Name: backupConfig.Name,
174174
Kind: v1beta1.ResourceKindBackupConfiguration,
175175
})
@@ -227,7 +227,7 @@ var _ = Describe("Auto-Backup", func() {
227227
Expect(err).NotTo(HaveOccurred())
228228

229229
// Take an Instant Backup of the Sample Data
230-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
230+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
231231
Name: backupConfig.Name,
232232
Kind: v1beta1.ResourceKindBackupConfiguration,
233233
})

‎test/e2e/auto-backup/statefulset.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ var _ = Describe("Auto-Backup", func() {
8383
Expect(err).NotTo(HaveOccurred())
8484

8585
// Take an Instant Backup of the Sample Data
86-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
86+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
8787
Name: backupConfig.Name,
8888
Kind: v1beta1.ResourceKindBackupConfiguration,
8989
})
@@ -129,7 +129,7 @@ var _ = Describe("Auto-Backup", func() {
129129
Expect(err).NotTo(HaveOccurred())
130130

131131
// Take an Instant Backup of the Sample Data
132-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
132+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
133133
Name: backupConfig.Name,
134134
Kind: v1beta1.ResourceKindBackupConfiguration,
135135
})
@@ -169,7 +169,7 @@ var _ = Describe("Auto-Backup", func() {
169169
Expect(err).NotTo(HaveOccurred())
170170

171171
// Take an Instant Backup of the Sample Data
172-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
172+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
173173
Name: backupConfig.Name,
174174
Kind: v1beta1.ResourceKindBackupConfiguration,
175175
})
@@ -227,7 +227,7 @@ var _ = Describe("Auto-Backup", func() {
227227
Expect(err).NotTo(HaveOccurred())
228228

229229
// Take an Instant Backup of the Sample Data
230-
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.TargetRef{
230+
backupSession, err := f.TakeInstantBackup(backupConfig.ObjectMeta, v1beta1.BackupInvokerRef{
231231
Name: backupConfig.Name,
232232
Kind: v1beta1.ResourceKindBackupConfiguration,
233233
})

0 commit comments

Comments
 (0)