Skip to content
This repository has been archived by the owner on Mar 28, 2020. It is now read-only.

Commit

Permalink
backup: Use meta/v1.Time instead of time.Time
Browse files Browse the repository at this point in the history
After I generated the code based on k8s object
(zz_generated.deepcopy.go), we happened to be in failing to build.
This is because all k8s custom resource's fileds should implement
DeepCopyInto but time.Time we added doesn't implement it.
For this purpose we should have used meta/v1.Time which is the
implementation to implement all necessary function for k8s object
and same function of time.Time.

And also this commit include some refactoring which is pointed out
in code-review
  • Loading branch information
ukinau committed Dec 18, 2018
1 parent d519aa7 commit d1ae2c4
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 33 deletions.
4 changes: 1 addition & 3 deletions pkg/apis/etcd/v1beta2/backup_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
package v1beta2

import (
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -117,7 +115,7 @@ type BackupStatus struct {
// EtcdRevision is the revision of etcd's KV store where the backup is performed on.
EtcdRevision int64 `json:"etcdRevision,omitempty"`
// LastSuccessDate indicate the time to get snapshot last time
LastSuccessDate time.Time `json:"lastSuccessDate,omitempty"`
LastSuccessDate metav1.Time `json:"lastSuccessDate,omitempty"`
}

// S3BackupSource provides the spec how to store backups on S3.
Expand Down
14 changes: 8 additions & 6 deletions pkg/backup/backup_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/coreos/etcd/clientv3"
"github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
)

Expand Down Expand Up @@ -53,31 +54,32 @@ func NewBackupManagerFromWriter(kubecli kubernetes.Interface, bw writer.Writer,

// SaveSnap uses backup writer to save etcd snapshot to a specified S3 path
// and returns backup etcd server's kv store revision and its version.
func (bm *BackupManager) SaveSnap(ctx context.Context, s3Path string, now time.Time, isPeriodic bool) (int64, string, error) {
func (bm *BackupManager) SaveSnap(ctx context.Context, s3Path string, isPeriodic bool) (int64, string, *metav1.Time, error) {
now := time.Now().UTC()
etcdcli, rev, err := bm.etcdClientWithMaxRevision(ctx)
if err != nil {
return 0, "", fmt.Errorf("create etcd client failed: %v", err)
return 0, "", nil, fmt.Errorf("create etcd client failed: %v", err)
}
defer etcdcli.Close()

resp, err := etcdcli.Status(ctx, etcdcli.Endpoints()[0])
if err != nil {
return 0, "", fmt.Errorf("failed to retrieve etcd version from the status call: %v", err)
return 0, "", nil, fmt.Errorf("failed to retrieve etcd version from the status call: %v", err)
}

rc, err := etcdcli.Snapshot(ctx)
if err != nil {
return 0, "", fmt.Errorf("failed to receive snapshot (%v)", err)
return 0, "", nil, fmt.Errorf("failed to receive snapshot (%v)", err)
}
defer rc.Close()
if isPeriodic {
s3Path = fmt.Sprintf(s3Path+"_v%d_%s", rev, now.Format("2006-01-02-15:04:05"))
}
_, err = bm.bw.Write(ctx, s3Path, rc)
if err != nil {
return 0, "", fmt.Errorf("failed to write snapshot (%v)", err)
return 0, "", nil, fmt.Errorf("failed to write snapshot (%v)", err)
}
return rev, resp.Version, nil
return rev, resp.Version, &metav1.Time{now}, nil
}

// EnsureMaxBackup to ensure the number of snapshot is under maxcount
Expand Down
8 changes: 3 additions & 5 deletions pkg/controller/backup-operator/abs_backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"crypto/tls"
"fmt"
"time"

api "github.com/coreos/etcd-operator/pkg/apis/etcd/v1beta2"
"github.com/coreos/etcd-operator/pkg/backup"
Expand All @@ -41,18 +40,17 @@ func handleABS(ctx context.Context, kubecli kubernetes.Interface, s *api.ABSBack
if tlsConfig, err = generateTLSConfig(kubecli, clientTLSSecret, namespace); err != nil {
return nil, err
}
now := time.Now().UTC()
bm := backup.NewBackupManagerFromWriter(kubecli, writer.NewABSWriter(cli.ABS), tlsConfig, endpoints, namespace)

rev, etcdVersion, err := bm.SaveSnap(ctx, s.Path, now, isPeriodic)
rev, etcdVersion, now, err := bm.SaveSnap(ctx, s.Path, isPeriodic)
if err != nil {
return nil, fmt.Errorf("failed to save snapshot (%v)", err)
}
if maxBackup > 0 {
err := bm.EnsureMaxbackup(ctx, s.Path, maxBackup)
err := bm.EnsureMaxBackup(ctx, s.Path, maxBackup)
if err != nil {
return nil, fmt.Errorf("succeeded in saving snapshot but failed to delete old snapshot (%v)", err)
}
}
return &api.BackupStatus{EtcdVersion: etcdVersion, EtcdRevision: rev, LastSuccessDate: now}, nil
return &api.BackupStatus{EtcdVersion: etcdVersion, EtcdRevision: rev, LastSuccessDate: *now}, nil
}
8 changes: 3 additions & 5 deletions pkg/controller/backup-operator/gcs_backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"crypto/tls"
"fmt"
"time"

api "github.com/coreos/etcd-operator/pkg/apis/etcd/v1beta2"
"github.com/coreos/etcd-operator/pkg/backup"
Expand All @@ -42,18 +41,17 @@ func handleGCS(ctx context.Context, kubecli kubernetes.Interface, s *api.GCSBack
if tlsConfig, err = generateTLSConfig(kubecli, clientTLSSecret, namespace); err != nil {
return nil, err
}
now := time.Now().UTC()
bm := backup.NewBackupManagerFromWriter(kubecli, writer.NewGCSWriter(cli.GCS), tlsConfig, endpoints, namespace)

rev, etcdVersion, err := bm.SaveSnap(ctx, s.Path, now, isPeriodic)
rev, etcdVersion, now, err := bm.SaveSnap(ctx, s.Path, isPeriodic)
if err != nil {
return nil, fmt.Errorf("failed to save snapshot (%v)", err)
}
if maxBackup > 0 {
err := bm.EnsureMaxbackup(ctx, s.Path, maxBackup)
err := bm.EnsureMaxBackup(ctx, s.Path, maxBackup)
if err != nil {
return nil, fmt.Errorf("succeeded in saving snapshot but failed to delete old snapshot (%v)", err)
}
}
return &api.BackupStatus{EtcdVersion: etcdVersion, EtcdRevision: rev, LastSuccessDate: now}, nil
return &api.BackupStatus{EtcdVersion: etcdVersion, EtcdRevision: rev, LastSuccessDate: *now}, nil
}
8 changes: 3 additions & 5 deletions pkg/controller/backup-operator/s3_backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"crypto/tls"
"fmt"
"time"

api "github.com/coreos/etcd-operator/pkg/apis/etcd/v1beta2"
"github.com/coreos/etcd-operator/pkg/backup"
Expand All @@ -43,18 +42,17 @@ func handleS3(ctx context.Context, kubecli kubernetes.Interface, s *api.S3Backup
if tlsConfig, err = generateTLSConfig(kubecli, clientTLSSecret, namespace); err != nil {
return nil, err
}
now := time.Now().UTC()
bm := backup.NewBackupManagerFromWriter(kubecli, writer.NewS3Writer(cli.S3), tlsConfig, endpoints, namespace)

rev, etcdVersion, err := bm.SaveSnap(ctx, s.Path, now, isPeriodic)
rev, etcdVersion, now, err := bm.SaveSnap(ctx, s.Path, isPeriodic)
if err != nil {
return nil, fmt.Errorf("failed to save snapshot (%v)", err)
}
if maxBackup > 0 {
err := bm.EnsureMaxbackup(ctx, s.Path, maxBackup)
err := bm.EnsureMaxBackup(ctx, s.Path, maxBackup)
if err != nil {
return nil, fmt.Errorf("succeeded in saving snapshot but failed to delete old snapshot (%v)", err)
}
}
return &api.BackupStatus{EtcdVersion: etcdVersion, EtcdRevision: rev, LastSuccessDate: now}, nil
return &api.BackupStatus{EtcdVersion: etcdVersion, EtcdRevision: rev, LastSuccessDate: *now}, nil
}
11 changes: 2 additions & 9 deletions pkg/controller/backup-operator/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,7 @@ func (b *Backup) processItem(key string) error {

if eb.DeletionTimestamp != nil {
b.deletePeriodicBackupRunner(eb.ObjectMeta.UID)
err := b.removeFinalizerOfPeriodicBackup(eb)
if err != nil {
return err
}
return nil
return b.removeFinalizerOfPeriodicBackup(eb)
}
isPeriodic := isPeriodicBackup(&eb.Spec)

Expand Down Expand Up @@ -168,10 +164,7 @@ func (b *Backup) removeFinalizerOfPeriodicBackup(eb *api.EtcdBackup) error {
}
metadata.SetFinalizers(finalizers)
_, err = b.backupCRCli.EtcdV1beta2().EtcdBackups(b.namespace).Update(ebNew.(*api.EtcdBackup))
if err != nil {
return err
}
return nil
return err
}

func (b *Backup) periodicRunnerFunc(ctx context.Context, t *time.Ticker, eb *api.EtcdBackup) {
Expand Down

0 comments on commit d1ae2c4

Please sign in to comment.