Skip to content
This repository was archived by the owner on Mar 28, 2020. It is now read-only.
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions pkg/controller/backup-operator/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,18 +175,22 @@ func (b *Backup) periodicRunnerFunc(ctx context.Context, t *time.Ticker, eb *api
break
case <-t.C:
var latestEb *api.EtcdBackup
var bs *api.BackupStatus
var err error
for {
for i := 1; i < 6; i++ {
latestEb, err = b.backupCRCli.EtcdV1beta2().EtcdBackups(b.namespace).Get(eb.Name, metav1.GetOptions{})
if err != nil {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ukinau Sorry for the delay. Yes think this should fix the infinite loop.

One more thing that I would improve on this is that we should not retry 5 times if the EtcdBackup CR is deleted or not found because that's when we want to stop the periodic backup. We only want to retry if it's some transient error.

  for i := 1; i < 6; i++ {
    latestEb, err = b.backupCRCli.EtcdV1beta2().EtcdBackups(b.namespace).Get(eb.Name, metav1.GetOptions{})
    if err != nil {
      // Stop backup if CR not found
      if apierrors.IsNotFound(err) {
        b.logger.Infof("Could not find EtcdBackup. Stopping periodic backup for EtcdBackup CR %v",
        eb.Name)
        break
      }
      
      b.logger.Warningf("[Attempt: %d/5] Failed to get latest EtcdBackup %v : (%v)",
        i, eb.Name, err)
      time.Sleep(1)
      continue
    }
    break
  }
  if err == nil {
    // Perform backup
    bs, err = b.handleBackup(&ctx, &latestEb.Spec, true)
  }

But that's just a minor change and since this PR has been out for a while I can make the above change as a follow up if you won't get the time for it anytime soon.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review. I'm gonna fix it

b.logger.Warningf("Failed to get latest EtcdBackup %v : (%v)", eb.Name, err)
b.logger.Warningf("[Attempt: %d/5] Failed to get latest EtcdBackup %v : (%v)",
i, eb.Name, err)
time.Sleep(1)
continue
}
break
}
// Perform backup
bs, err := b.handleBackup(&ctx, &latestEb.Spec, true)
if err == nil {
// Perform backup
bs, err = b.handleBackup(&ctx, &latestEb.Spec, true)
}
// Report backup status
b.reportBackupStatus(bs, err, latestEb)
}
Expand Down