From 9a6488224cb6758210a7288a75abce7df7849eb0 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Mon, 5 Feb 2018 15:25:12 -0800 Subject: [PATCH 1/3] apis: update Timeout comment --- pkg/apis/etcd/v1beta2/backup_types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/etcd/v1beta2/backup_types.go b/pkg/apis/etcd/v1beta2/backup_types.go index c7dc173a3..ba2424cb2 100644 --- a/pkg/apis/etcd/v1beta2/backup_types.go +++ b/pkg/apis/etcd/v1beta2/backup_types.go @@ -84,8 +84,8 @@ type BackupSource struct { // BackupPolicy defines backup policy. type BackupPolicy struct { - // timeout is the maximal time of retriving plus saving an etcd backup. - Timeout int64 `json:"timeout,omitempty"` + // TimeoutInSecond is the maximal allowed time in second of the entire backup process. + TimeoutInSecond int64 `json:"timeoutInSecond,omitempty"` } // BackupStatus represents the status of the EtcdBackup Custom Resource. From a641328b651ee8d752111af7ccfe0840f58c7506 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Mon, 5 Feb 2018 15:26:03 -0800 Subject: [PATCH 2/3] pkg/backup: use context to controll SaveSnap() --- pkg/backup/backup_manager.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/pkg/backup/backup_manager.go b/pkg/backup/backup_manager.go index 0c962381d..6853665e5 100644 --- a/pkg/backup/backup_manager.go +++ b/pkg/backup/backup_manager.go @@ -20,7 +20,6 @@ import ( "fmt" "github.com/coreos/etcd-operator/pkg/backup/writer" - "github.com/coreos/etcd-operator/pkg/util/constants" "github.com/coreos/etcd/clientv3" "github.com/sirupsen/logrus" @@ -51,22 +50,18 @@ 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(s3Path string) (int64, string, error) { - etcdcli, rev, err := bm.etcdClientWithMaxRevision() +func (bm *BackupManager) SaveSnap(ctx context.Context, s3Path string) (int64, string, error) { + etcdcli, rev, err := bm.etcdClientWithMaxRevision(ctx) if err != nil { return 0, "", fmt.Errorf("create etcd client failed: %v", err) } defer etcdcli.Close() - ctx, cancel := context.WithTimeout(context.Background(), constants.DefaultRequestTimeout) resp, err := etcdcli.Status(ctx, etcdcli.Endpoints()[0]) - cancel() if err != nil { return 0, "", fmt.Errorf("failed to retrieve etcd version from the status call: %v", err) } - ctx, cancel = context.WithTimeout(context.Background(), constants.DefaultSnapshotTimeout) - defer cancel() // Can't cancel() after Snapshot() because that will close the reader. rc, err := etcdcli.Snapshot(ctx) if err != nil { return 0, "", fmt.Errorf("failed to receive snapshot (%v)", err) @@ -82,24 +77,24 @@ func (bm *BackupManager) SaveSnap(s3Path string) (int64, string, error) { // etcdClientWithMaxRevision gets the etcd endpoint with the maximum kv store revision // and returns the etcd client of that member. -func (bm *BackupManager) etcdClientWithMaxRevision() (*clientv3.Client, int64, error) { - etcdcli, rev, err := getClientWithMaxRev(bm.endpoints, bm.etcdTLSConfig) +func (bm *BackupManager) etcdClientWithMaxRevision(ctx context.Context) (*clientv3.Client, int64, error) { + etcdcli, rev, err := getClientWithMaxRev(ctx, bm.endpoints, bm.etcdTLSConfig) if err != nil { return nil, 0, fmt.Errorf("failed to get etcd client with maximum kv store revision: %v", err) } return etcdcli, rev, nil } -func getClientWithMaxRev(endpoints []string, tc *tls.Config) (*clientv3.Client, int64, error) { +func getClientWithMaxRev(ctx context.Context, endpoints []string, tc *tls.Config) (*clientv3.Client, int64, error) { mapEps := make(map[string]*clientv3.Client) var maxClient *clientv3.Client maxRev := int64(0) errors := make([]string, 0) for _, endpoint := range endpoints { cfg := clientv3.Config{ - Endpoints: []string{endpoint}, - DialTimeout: constants.DefaultDialTimeout, - TLS: tc, + Endpoints: []string{endpoint}, + Context: ctx, + TLS: tc, } etcdcli, err := clientv3.New(cfg) if err != nil { @@ -108,9 +103,7 @@ func getClientWithMaxRev(endpoints []string, tc *tls.Config) (*clientv3.Client, } mapEps[endpoint] = etcdcli - ctx, cancel := context.WithTimeout(context.Background(), constants.DefaultRequestTimeout) resp, err := etcdcli.Get(ctx, "/", clientv3.WithSerializable()) - cancel() if err != nil { errors = append(errors, fmt.Sprintf("failed to get revision from endpoint (%s)", endpoint)) continue From aabcd453c1ab873869a443d6ae1ff3482af31e03 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Mon, 5 Feb 2018 15:27:06 -0800 Subject: [PATCH 3/3] *: support backup timeout in backup operator --- pkg/backup/backup_manager.go | 8 +++++--- pkg/controller/backup-operator/abs_backup.go | 7 +++++-- pkg/controller/backup-operator/s3_backup.go | 7 +++++-- pkg/controller/backup-operator/sync.go | 15 +++++++++++++-- pkg/util/constants/constants.go | 7 ++++--- 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/pkg/backup/backup_manager.go b/pkg/backup/backup_manager.go index 6853665e5..217906a56 100644 --- a/pkg/backup/backup_manager.go +++ b/pkg/backup/backup_manager.go @@ -20,6 +20,7 @@ import ( "fmt" "github.com/coreos/etcd-operator/pkg/backup/writer" + "github.com/coreos/etcd-operator/pkg/util/constants" "github.com/coreos/etcd/clientv3" "github.com/sirupsen/logrus" @@ -91,10 +92,11 @@ func getClientWithMaxRev(ctx context.Context, endpoints []string, tc *tls.Config maxRev := int64(0) errors := make([]string, 0) for _, endpoint := range endpoints { + // TODO: update clientv3 to 3.2.x and then use ctx as in clientv3.Config. cfg := clientv3.Config{ - Endpoints: []string{endpoint}, - Context: ctx, - TLS: tc, + Endpoints: []string{endpoint}, + DialTimeout: constants.DefaultDialTimeout, + TLS: tc, } etcdcli, err := clientv3.New(cfg) if err != nil { diff --git a/pkg/controller/backup-operator/abs_backup.go b/pkg/controller/backup-operator/abs_backup.go index 1d339e20b..d38af1cc8 100644 --- a/pkg/controller/backup-operator/abs_backup.go +++ b/pkg/controller/backup-operator/abs_backup.go @@ -15,6 +15,7 @@ package controller import ( + "context" "crypto/tls" "fmt" @@ -27,7 +28,8 @@ import ( ) // handleABS saves etcd cluster's backup to specificed ABS path. -func handleABS(kubecli kubernetes.Interface, s *api.ABSBackupSource, endpoints []string, clientTLSSecret, namespace string) (*api.BackupStatus, error) { +func handleABS(ctx context.Context, kubecli kubernetes.Interface, s *api.ABSBackupSource, endpoints []string, clientTLSSecret, namespace string) (*api.BackupStatus, error) { + // TODO: controls NewClientFromSecret with ctx. This depends on upstream kubernetes to support API calls with ctx. cli, err := absfactory.NewClientFromSecret(kubecli, namespace, s.ABSSecret) if err != nil { return nil, err @@ -39,7 +41,8 @@ func handleABS(kubecli kubernetes.Interface, s *api.ABSBackupSource, endpoints [ } bm := backup.NewBackupManagerFromWriter(kubecli, writer.NewABSWriter(cli.ABS), tlsConfig, endpoints, namespace) - rev, etcdVersion, err := bm.SaveSnap(s.Path) + + rev, etcdVersion, err := bm.SaveSnap(ctx, s.Path) if err != nil { return nil, fmt.Errorf("failed to save snapshot (%v)", err) } diff --git a/pkg/controller/backup-operator/s3_backup.go b/pkg/controller/backup-operator/s3_backup.go index 48bfd66a1..caf0c18ce 100644 --- a/pkg/controller/backup-operator/s3_backup.go +++ b/pkg/controller/backup-operator/s3_backup.go @@ -15,6 +15,7 @@ package controller import ( + "context" "crypto/tls" "fmt" @@ -28,7 +29,8 @@ import ( // TODO: replace this with generic backend interface for other options (PV, Azure) // handleS3 saves etcd cluster's backup to specificed S3 path. -func handleS3(kubecli kubernetes.Interface, s *api.S3BackupSource, endpoints []string, clientTLSSecret, namespace string) (*api.BackupStatus, error) { +func handleS3(ctx context.Context, kubecli kubernetes.Interface, s *api.S3BackupSource, endpoints []string, clientTLSSecret, namespace string) (*api.BackupStatus, error) { + // TODO: controls NewClientFromSecret with ctx. This depends on upstream kubernetes to support API calls with ctx. cli, err := s3factory.NewClientFromSecret(kubecli, namespace, s.Endpoint, s.AWSSecret) if err != nil { return nil, err @@ -41,7 +43,8 @@ func handleS3(kubecli kubernetes.Interface, s *api.S3BackupSource, endpoints []s } bm := backup.NewBackupManagerFromWriter(kubecli, writer.NewS3Writer(cli.S3), tlsConfig, endpoints, namespace) - rev, etcdVersion, err := bm.SaveSnap(s.Path) + + rev, etcdVersion, err := bm.SaveSnap(ctx, s.Path) if err != nil { return nil, fmt.Errorf("failed to save snapshot (%v)", err) } diff --git a/pkg/controller/backup-operator/sync.go b/pkg/controller/backup-operator/sync.go index 45d652d19..02a298727 100644 --- a/pkg/controller/backup-operator/sync.go +++ b/pkg/controller/backup-operator/sync.go @@ -15,9 +15,12 @@ package controller import ( + "context" "errors" + "time" api "github.com/coreos/etcd-operator/pkg/apis/etcd/v1beta2" + "github.com/coreos/etcd-operator/pkg/util/constants" "github.com/sirupsen/logrus" ) @@ -119,15 +122,23 @@ func (b *Backup) handleBackup(spec *api.BackupSpec) (*api.BackupStatus, error) { return nil, err } + // When BackupPolicy.Timeout <= 0, use default DefaultBackupTimeout. + backupTimeout := time.Duration(constants.DefaultBackupTimeout) + if spec.BackupPolicy != nil && spec.BackupPolicy.TimeoutInSecond > 0 { + backupTimeout = time.Duration(spec.BackupPolicy.TimeoutInSecond) * time.Second + } + + ctx, cancel := context.WithTimeout(context.Background(), backupTimeout) + defer cancel() switch spec.StorageType { case api.BackupStorageTypeS3: - bs, err := handleS3(b.kubecli, spec.S3, spec.EtcdEndpoints, spec.ClientTLSSecret, b.namespace) + bs, err := handleS3(ctx, b.kubecli, spec.S3, spec.EtcdEndpoints, spec.ClientTLSSecret, b.namespace) if err != nil { return nil, err } return bs, nil case api.BackupStorageTypeABS: - bs, err := handleABS(b.kubecli, spec.ABS, spec.EtcdEndpoints, spec.ClientTLSSecret, b.namespace) + bs, err := handleABS(ctx, b.kubecli, spec.ABS, spec.EtcdEndpoints, spec.ClientTLSSecret, b.namespace) if err != nil { return nil, err } diff --git a/pkg/util/constants/constants.go b/pkg/util/constants/constants.go index c42f74c28..e992dba39 100644 --- a/pkg/util/constants/constants.go +++ b/pkg/util/constants/constants.go @@ -17,9 +17,10 @@ package constants import "time" const ( - DefaultDialTimeout = 5 * time.Second - DefaultRequestTimeout = 5 * time.Second - DefaultSnapshotTimeout = 1 * time.Minute + DefaultDialTimeout = 5 * time.Second + DefaultRequestTimeout = 5 * time.Second + // DefaultBackupTimeout is the default maximal allowed time of the entire backup process. + DefaultBackupTimeout = 1 * time.Minute DefaultSnapshotInterval = 1800 * time.Second DefaultBackupPodHTTPPort = 19999