From b61134b4a476c1cf53455eb1a35b5c9cbc51ca9d Mon Sep 17 00:00:00 2001 From: Jingtao Ren Date: Wed, 10 Jan 2018 19:39:46 -0800 Subject: [PATCH] refactoring and address comment --- Gopkg.lock | 6 +--- Gopkg.toml | 16 +++++++++ pkg/backup/reader/abs_reader.go | 4 +-- pkg/backup/writer/abs_writer.go | 12 +++---- pkg/controller/backup-operator/abs_backup.go | 13 ++----- pkg/controller/backup-operator/s3_backup.go | 13 ++----- pkg/controller/backup-operator/util.go | 38 ++++++++++++++++++++ pkg/util/azureutil/absfactory/client.go | 4 +-- test/pod/README.md | 2 ++ test/pod/test-pod-templ.yaml | 2 ++ 10 files changed, 71 insertions(+), 39 deletions(-) create mode 100644 pkg/controller/backup-operator/util.go diff --git a/Gopkg.lock b/Gopkg.lock index 2582077e3..af2575304 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -352,10 +352,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 -<<<<<<< HEAD - inputs-digest = "bc96c486c694ef6b8f178f9290a6e5aa4f41003968a3b9e62e289d67408b9b46" -======= - inputs-digest = "47a6cbc8ce90909306c936a49542b1be71a5da99af67986a04a1bc1bedb149c2" ->>>>>>> add ABS support for backup and restore + inputs-digest = "348ce70c891e3e31857fd45276c70cf2f54e6a63b76b889ee79525d8e8ea86a0" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index 2e251afbd..f155f5a3e 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -40,3 +40,19 @@ [[constraint]] name = "golang.org/x/time" + +[[constraint]] + name = "github.com/Azure/azure-sdk-for-go" + version = "v11.3.0-beta" + +[[constraint]] + name = "github.com/Azure/go-autorest" + version = "v9.6.0" + +[[constraint]] + name = "github.com/dgrijalva/jwt-go" + +[[constraint]] + name = "github.com/satori/uuid" + version = "v1.1.0" + diff --git a/pkg/backup/reader/abs_reader.go b/pkg/backup/reader/abs_reader.go index ed0c4a5dd..dcb51b0a9 100644 --- a/pkg/backup/reader/abs_reader.go +++ b/pkg/backup/reader/abs_reader.go @@ -31,6 +31,7 @@ type absReader struct { abs *storage.BlobStorageClient } +// NewABSReader return a Reader implementation to read a file from ABS in the form of absReader func NewABSReader(abs *storage.BlobStorageClient) Reader { return &absReader{abs} } @@ -53,6 +54,5 @@ func (absr *absReader) Open(path string) (io.ReadCloser, error) { } blob := containerRef.GetBlobReference(key) - getBlobOpts := &storage.GetBlobOptions{} - return blob.Get(getBlobOpts) + return blob.Get(&storage.GetBlobOptions{}) } diff --git a/pkg/backup/writer/abs_writer.go b/pkg/backup/writer/abs_writer.go index 1ebce4b90..5b9d3398b 100644 --- a/pkg/backup/writer/abs_writer.go +++ b/pkg/backup/writer/abs_writer.go @@ -20,8 +20,9 @@ import ( "fmt" "io" - "github.com/Azure/azure-sdk-for-go/storage" "github.com/coreos/etcd-operator/pkg/backup/util" + + "github.com/Azure/azure-sdk-for-go/storage" "github.com/pborman/uuid" ) @@ -38,7 +39,7 @@ func NewABSWriter(abs *storage.BlobStorageClient) Writer { const ( // AzureBlobBlockChunkLimitInBytes 100MiB is the limit - AzureBlobBlockChunkLimitInBytes = 104857600 + AzureBlobBlockChunkLimitInBytes = 100 * 1024 * 1024 ) // Write writes the backup file to the given abs path, "/". @@ -59,9 +60,7 @@ func (absw *absWriter) Write(path string, r io.Reader) (int64, error) { } blob := containerRef.GetBlobReference(key) - putBlobOpts := storage.PutBlobOptions{} - - err = blob.CreateBlockBlob(&putBlobOpts) + err = blob.CreateBlockBlob(&storage.PutBlobOptions{}) if err != nil { return 0, err } @@ -92,8 +91,7 @@ func (absw *absWriter) Write(path string, r io.Reader) (int64, error) { return 0, err } - getBlobOpts := &storage.GetBlobOptions{} - _, err = blob.Get(getBlobOpts) + _, err = blob.Get(&storage.GetBlobOptions{}) if err != nil { return 0, err } diff --git a/pkg/controller/backup-operator/abs_backup.go b/pkg/controller/backup-operator/abs_backup.go index 3df8b9af1..276a7f622 100644 --- a/pkg/controller/backup-operator/abs_backup.go +++ b/pkg/controller/backup-operator/abs_backup.go @@ -22,8 +22,6 @@ import ( "github.com/coreos/etcd-operator/pkg/backup" "github.com/coreos/etcd-operator/pkg/backup/writer" "github.com/coreos/etcd-operator/pkg/util/azureutil/absfactory" - "github.com/coreos/etcd-operator/pkg/util/etcdutil" - "github.com/coreos/etcd-operator/pkg/util/k8sutil" "k8s.io/client-go/kubernetes" ) @@ -37,15 +35,8 @@ func handleABS(kubecli kubernetes.Interface, s *api.ABSBackupSource, endpoints [ } var tlsConfig *tls.Config - if len(clientTLSSecret) != 0 { - d, err := k8sutil.GetTLSDataFromSecret(kubecli, namespace, clientTLSSecret) - if err != nil { - return nil, fmt.Errorf("failed to get TLS data from secret (%v): %v", clientTLSSecret, err) - } - tlsConfig, err = etcdutil.NewTLSConfig(d.CertData, d.KeyData, d.CAData) - if err != nil { - return nil, fmt.Errorf("failed to constructs tls config: %v", err) - } + if tlsConfig, err = generateTLSConfig(kubecli, clientTLSSecret, namespace); err != nil { + return nil, err } bm := backup.NewBackupManagerFromWriter(kubecli, writer.NewABSWriter(cli.ABS), tlsConfig, endpoints, namespace) diff --git a/pkg/controller/backup-operator/s3_backup.go b/pkg/controller/backup-operator/s3_backup.go index e2cbba476..48bfd66a1 100644 --- a/pkg/controller/backup-operator/s3_backup.go +++ b/pkg/controller/backup-operator/s3_backup.go @@ -22,8 +22,6 @@ import ( "github.com/coreos/etcd-operator/pkg/backup" "github.com/coreos/etcd-operator/pkg/backup/writer" "github.com/coreos/etcd-operator/pkg/util/awsutil/s3factory" - "github.com/coreos/etcd-operator/pkg/util/etcdutil" - "github.com/coreos/etcd-operator/pkg/util/k8sutil" "k8s.io/client-go/kubernetes" ) @@ -38,15 +36,8 @@ func handleS3(kubecli kubernetes.Interface, s *api.S3BackupSource, endpoints []s defer cli.Close() var tlsConfig *tls.Config - if len(clientTLSSecret) != 0 { - d, err := k8sutil.GetTLSDataFromSecret(kubecli, namespace, clientTLSSecret) - if err != nil { - return nil, fmt.Errorf("failed to get TLS data from secret (%v): %v", clientTLSSecret, err) - } - tlsConfig, err = etcdutil.NewTLSConfig(d.CertData, d.KeyData, d.CAData) - if err != nil { - return nil, fmt.Errorf("failed to constructs tls config: %v", err) - } + if tlsConfig, err = generateTLSConfig(kubecli, clientTLSSecret, namespace); err != nil { + return nil, err } bm := backup.NewBackupManagerFromWriter(kubecli, writer.NewS3Writer(cli.S3), tlsConfig, endpoints, namespace) diff --git a/pkg/controller/backup-operator/util.go b/pkg/controller/backup-operator/util.go new file mode 100644 index 000000000..cb3fcb5a8 --- /dev/null +++ b/pkg/controller/backup-operator/util.go @@ -0,0 +1,38 @@ +// Copyright 2017 The etcd-operator Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controller + +import ( + "crypto/tls" + "fmt" + + "github.com/coreos/etcd-operator/pkg/util/etcdutil" + "github.com/coreos/etcd-operator/pkg/util/k8sutil" +) + +func generateTLSConfig(kubecli kubernetes.Interface, clientTLSSecret, namespace string) (*tls.Config, error) { + var tlsConfig *tls.Config + if len(clientTLSSecret) != 0 { + d, err := k8sutil.GetTLSDataFromSecret(kubecli, namespace, clientTLSSecret) + if err != nil { + return nil, fmt.Errorf("failed to get TLS data from secret (%v): %v", clientTLSSecret, err) + } + tlsConfig, err = etcdutil.NewTLSConfig(d.CertData, d.KeyData, d.CAData) + if err != nil { + return nil, fmt.Errorf("failed to constructs tls config: %v", err) + } + } + return tlsConfig, nil +} diff --git a/pkg/util/azureutil/absfactory/client.go b/pkg/util/azureutil/absfactory/client.go index a03fc585f..7d6dafae1 100644 --- a/pkg/util/azureutil/absfactory/client.go +++ b/pkg/util/azureutil/absfactory/client.go @@ -41,7 +41,6 @@ func NewClientFromSecret(kubecli kubernetes.Interface, namespace, absSecret stri } }() - w = &ABSClient{} se, err := kubecli.CoreV1().Secrets(namespace).Get(absSecret, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("failed to get k8s secret: %v", err) @@ -58,6 +57,5 @@ func NewClientFromSecret(kubecli kubernetes.Interface, namespace, absSecret stri } abs := bc.GetBlobService() - w.ABS = &abs - return w, nil + return &ABSClient{ABS: &abs}, nil } diff --git a/test/pod/README.md b/test/pod/README.md index 2dfe1ecc0..a004da4a7 100644 --- a/test/pod/README.md +++ b/test/pod/README.md @@ -21,6 +21,7 @@ The [test-pod-tmpl.yaml](./test-pod-tmpl.yaml) can be used to define the test-po - `OPERATOR_IMAGE` is the etcd-operator image used for testing - `TEST_S3_BUCKET` is the S3 bucket name used for testing - `TEST_AWS_SECRET` is the secret name containing the aws credentials/config files. +- `TEST_ABS_SECRET` is the secret name containing the abs credentials - `E2E_TEST_SELECTOR` selects which e2e tests to run. Leave empty to run all tests. - `UPGRADE_TEST_SELECTOR` selects which upgrade tests to run. Leave empty to run all tests. - `PASSES` are the test passes to run (e2e, e2eslow, e2esh). See [hack/test](../../hack/test) @@ -35,6 +36,7 @@ sed -e "s||e2e-testing|g" \ -e "s|||g" \ -e "s||my-bucket|g" \ -e "s||aws-secret|g" \ + -e "s||abs-secret|g" \ -e "s|||g" \ -e "s||quay.io/coreos/etcd-operator:latest|g" \ -e "s||quay.io/coreos/etcd-operator:dev|g" \ diff --git a/test/pod/test-pod-templ.yaml b/test/pod/test-pod-templ.yaml index ef118bed1..9ab7f14e4 100644 --- a/test/pod/test-pod-templ.yaml +++ b/test/pod/test-pod-templ.yaml @@ -27,6 +27,8 @@ spec: value: - name: TEST_AWS_SECRET value: + - name: TEST_ABS_SECRET + value: - name: PARALLEL_TEST value: "true" - name: UPGRADE_TEST_SELECTOR