Apply suggestions from code review

Co-Authored-By: Shreyas Rao <[email protected]>
Signed-off-by: Swapnil Mhamane <[email protected]>

.ci/integration_test

@@ -27,7 +27,7 @@ VCS="github.com"
 ORGANIZATION="gardener"
 PROJECT="etcd-backup-restore"
 REPOSITORY=${VCS}/${ORGANIZATION}/${PROJECT}
-VERSION_FILE="$(readlink  -f "${SOURCE_PATH}/VERSION")"
+VERSION_FILE="$(readlink -f "${SOURCE_PATH}/VERSION")"
 VERSION="$(cat "${VERSION_FILE}")"
 
 export GOBIN="${SOURCE_PATH}/bin"
@@ -45,7 +45,7 @@ TEST_DIR=
 function setup_test_enviornment() {
   setup_ginkgo
   setup_etcd
-  setup_etcbrctl
+  setup_etcdbrctl
   setup_awscli
 }
 
@@ -69,9 +69,9 @@ function setup_etcd(){
     echo "Successfully installed etcd."
 }
 
-function setup_etcbrctl(){
+function setup_etcdbrctl(){
     echo "Installing etcdbrctl..."
-    go build \
+    GO111MODULE=on go build \
     -v \
     -o ${GOBIN}/etcdbrctl \
     -ldflags "-w -X ${REPOSITORY}/pkg/version.Version=${VERSION}" \
@@ -199,4 +199,4 @@ echo "Deleting test enviornment..."
 cleanup_test_environment
 echo "Successfully completed all tests."
 
-exit $TEST_RESULT
+exit $TEST_RESULT

.ci/unit_test

@@ -30,7 +30,7 @@ REPOSITORY=${VCS}/${ORGANIZATION}/${PROJECT}
 cd "${SOURCE_PATH}"
 
 # Install Ginkgo (test framework) to be able to execute the tests.
-go get -u github.com/onsi/ginkgo/ginkgo
+GO111MODULE=off go get github.com/onsi/ginkgo/ginkgo
 
 ###############################################################################
 

README.md

@@ -15,13 +15,13 @@ Etcd-backup-restore is collection of components to backup and restore the [etcd]
 
 ### Design and Proposals
 * [Core design](doc/proposals/design.md)
-* [Etcd data Validation ](doc/proposals/validation.md)
-* [High watch events ingress rate issue](doc.proposals/high_watch_event_ingress_rate.md)
+* [Etcd data validation ](doc/proposals/validation.md)
+* [High watch events ingress rate issue](doc/proposals/high_watch_event_ingress_rate.md)
 
 ### Development
 
-* [Setting up a local development environment](doc.development/local_setup.md)
-* [Testing and Dependency Management](doc.development/testing_and_dependencies.md)
+* [Setting up a local development environment](doc/development/local_setup.md)
+* [Testing and Dependency Management](doc/development/testing_and_dependencies.md)
 * [Adding support for a new cloud provider](doc/development/new_cp_support.md)
 
 

doc/development/local_setup.md

@@ -1,20 +1,20 @@
 ## Prerequisites
 
-Although the following installation instructions are for Mac OS X, similar alternate commands could be found for any Linux distribution
+Although the following installation instructions are for Mac OS X, similar alternate commands can be found for any Linux distribution.
 
 ### Installing [Golang](https://golang.org/) environment
 
-Install the latest version of Golang (at least `v1.12` is required). For Mac OS, you could use [Homebrew](https://brew.sh/):
+Install the latest version of Golang (at least `v1.12` is required). For Mac OS, you may use [Homebrew](https://brew.sh/):
 
 ```sh
 brew install golang
 ```
 
-For other OS, please check [Go installation documentation](https://golang.org/doc/install).
+For other OSes, please check [Go installation documentation](https://golang.org/doc/install).
 
 Make sure to set your `$GOPATH` environment variable properly (conventionally, it points to `$HOME/go`).
 
-For your convenience, you can add the `bin` directory of the `$GOPATH` to your `$PATH`: `PATH=$PATH:$GOPATH/bin`, but it is not necessarily required.
+For your convenience, you can add the `bin` directory of the `$GOPATH` to your `$PATH`: `PATH=$PATH:$GOPATH/bin`, but it is not mandatory.
 
 ### [Golint](https://github.com/golang/lint)
 
@@ -45,12 +45,11 @@ brew install git
 
 ### Installing `gcloud` SDK (Optional)
 
-In case you have to create a new release or a new hotfix, you have to push the resulting Docker image into a Docker registry. Currently, we are using the Google Container Registry (this could change in the future). Please follow the official [installation instructions from Google](https://cloud.google.com/sdk/downloads).
-
+In case you have to create a new release or a new hotfix, you have to push the resulting Docker image into a Docker registry. Currently, we use the Google Container Registry (this could change in the future). Please follow the official [installation instructions from Google](https://cloud.google.com/sdk/downloads).
 
 ## Build
 
-Currently there are no binary build available, but it is pretty straight forward to build it by following the steps mentioned below.
+Currently there are no binary builds available, but it is fairly simple to build it by following the steps mentioned below.
 
 * First, you need to create a target folder structure before cloning and building `etcdbrctl`.
 
@@ -65,12 +64,13 @@ Currently there are no binary build available, but it is pretty straight forward
     make build-local
     ```
 
-* Next you can make it available to use as shell command by moving the executable to `/usr/local/bin`.
+* Next you can make it available to use as shell command by moving the executable to `/usr/local/bin`, or by optionally including the `bin` directory in your `$PATH` environment variable.
 You can verify the installation by running following command:
+
     ```console
     $ etcdbrctl -v
     INFO[0000] etcd-backup-restore Version: v0.7.0-dev
     INFO[0000] Git SHA: 38979f0
     INFO[0000] Go Version: go1.12
     INFO[0000] Go OS/Arch: darwin/amd64
-    ```
+    ```

doc/development/testing_and_dependencies.md

@@ -1,16 +1,13 @@
 # Dependency management
 
-We use golang modules to manage golang dependencies. In order to add a new package dependency to the project, you can perform `go get <PACKAGE>@<VERSION>` or edit the `go.mod` file and append the package along with the version you want to use.
+We use go-modules to manage golang dependencies. In order to add a new package dependency to the project, you can perform `go get <PACKAGE>@<VERSION>` or edit the `go.mod` file and append the package along with the version you want to use.
 
 ### Updating dependencies
 
 The `Makefile` contains a rule called `revendor` which performs `go mod vendor` and `go mod tidy`.
 * `go mod vendor` resets the main module's vendor directory to include all packages needed to build and test all the main module's packages. It does not include test code for vendored packages.
-It does not include test code for vendored packages.
 * `go mod tidy` makes sure go.mod matches the source code in the module.
-It adds any missing modules necessary to build the current module's
-packages and dependencies, and it removes unused modules that
-don't provide any relevant packages.
+It adds any missing modules necessary to build the current module's packages and dependencies, and it removes unused modules that don't provide any relevant packages.
 
 ```sh
 make revendor
@@ -22,10 +19,10 @@ The dependencies are installed into the `vendor` folder which **should be added*
 
 # Testing
 
-We have created `make` target `verify` which will internally run different rule like `fmt` for formatting, `lint` for linting check and most importantly `test` which will check the code against predefined unit tests. Although, currently there are not enough test cases written to cover entire code, hence one should check for failure cases manually before raising pull request. We will eventually add the test cases for complete code coverage.
+We have created `make` target `verify` which will internally run different rules like `fmt` for formatting, `lint` for linting check and most importantly `test` which will check the code against predefined unit tests. As currently there aren't enough test cases written to cover the entire code, you must check for failure cases manually and include test cases before raising pull request. We will eventually add more test cases for complete code coverage.
 
 ```sh
 make verify
 ```
 
-By default, we try to run test in parallel without computing code coverage. To get the code coverage, you will have to set environment variable `COVER` to `true`. This will log the code coverage percentage at the end of test logs. Also, all cover profile files will accumulated under `test/output/coverprofile.out` directory. You can visualize exact code coverage using `make show-coverage`.
+By default, we run tests without computing code coverage. To get the code coverage, you can set the environment variable `COVER` to `true`. This will log the code coverage percentage at the end of test logs. Also, all cover profile files will be accumulated under `test/output/coverprofile.out` directory. You can visualize the exact code coverage by running `make show-coverage` after running `make verify` with code coverage enabled.

doc/proposals/design.md

@@ -39,7 +39,7 @@ Main goal of this project to provide a solution to make [etcd] instance backing
 
 ## Architecture
 
-![architecture](images/etcd-backup-restore.jpg)
+![architecture](../images/etcd-backup-restore.jpg)
 
 We will have a StatefulSet for etcd with two containers in it.
 
@@ -50,7 +50,7 @@ We will have a StatefulSet for etcd with two containers in it.
 ### ETCD Container
 
 - Request the sidecar to validate/initialize the data directory.
-- The etcd process is started only if the `initialize` request to sidecar returns a success. 
+- The etcd process is started only if the `initialize` request to sidecar returns a success.
 
 ### Sidecar Container
 
@@ -71,12 +71,12 @@ Sidecar container has two components
 - Probe is required to ensure that etcd is live before backups are triggered.
 - Schedule the backup operation (probably using cron library) which triggers full snapshot at regular intervals.
 - Store the snapshot in the configured cloud object store.
-  
+
 **Init container is not used for the validation/restoration of etcd data directory. The rationale behind the decision was to avoid baking in pod restart logic in sidecar container in the event etcd process had died. In case etcd container died, init-container had to be run before etcd container was run to ensure that data directory was valid. This required the pod to be restarted. With the current design, the sidecar handles the data directory validation/restoration and periodic backups. Pod restart is not required.**
 
 ## Workflow
 
-![sequence-diagram](images/etcd-backup-restore-sequence-diagram.jpg)
+![sequence-diagram](../images/etcd-backup-restore-sequence-diagram.jpg)
 
 ### Etcd container
 
@@ -92,15 +92,15 @@ Sidecar container has two components
     1. In case of data directory corruption, restore data directory from the latest cloud snapshot. Return success.
     2. In case data directory is valid, return success.
     3. In all other cases, return failure.
-3. Once the `initialize` request returns success, etcd process can be expected to start up in some time. The prober would then receive a successful probe of etcd's liveliness. 
-4. On successful probe, start taking periodic backup of etcd and store the snapshot to the cloud object store. Stop prober. 
+3. Once the `initialize` request returns success, etcd process can be expected to start up in some time. The prober would then receive a successful probe of etcd's liveliness.
+4. On successful probe, start taking periodic backup of etcd and store the snapshot to the cloud object store. Stop prober.
     - In case of a failure to take a backup, exit with error. (Container restarts)
 
 ### Handling of Different Scenarios/Issues
 
 - DNS latency: Should not matter for single member Etcd cluster.
 - Etcd upgrade and downgrade for K8s compatibility: Should not be issue for v3.* series released so far. Simply restart pod. No data format change.
-- Iaas issue: Issues like unreachable object store, will be taken care by init container and backup container. Both container will keep retrying to reach out object store with exponential timeouts.
+- IaaS issue: Issues like unreachable object store, will be taken care by init container and backup container. Both container will keep retrying to reach out object store with exponential timeouts.
 - Corrupt backup: StatefulSet go in restart loop, and human operator will with customers concern delete the last corrupt backup from object store manually. So that, in next iteration it will recover from previous non-corrupt backup.
 
 ## Outlook