From 94d678ab16923a6a504e4b06815ece9e1688f264 Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Wed, 13 Nov 2019 14:17:50 +0000 Subject: [PATCH 01/10] Configure linter --- .devcontainer/Dockerfile | 3 +++ .golangci.toml | 42 ++++++++++++++++++++++++++++++++++++++++ Makefile | 16 +++++++-------- 3 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 .golangci.toml diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 2ae2840..d4db1b3 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -66,6 +66,9 @@ RUN apt-get update \ RUN apt-get -y install git procps wget nano zsh inotify-tools jq RUN wget https://github.com/robbyrussell/oh-my-zsh/raw/master/tools/install.sh -O - | zsh || true +# Install golangci-linter +RUN curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.21.0 + ENV PATH="/usr/local/kubebuilder/bin:${PATH}" ENV GO111MODULE=on diff --git a/.golangci.toml b/.golangci.toml new file mode 100644 index 0000000..40d462a --- /dev/null +++ b/.golangci.toml @@ -0,0 +1,42 @@ +[run] + deadline = "5m" + skip-files = [] + +[linters-settings] + + [linters-settings.govet] + check-shadowing = true + + [linters-settings.gocyclo] + min-complexity = 12.0 + + [linters-settings.maligned] + suggest-new = true + + [linters-settings.goconst] + min-len = 3.0 + min-occurrences = 3.0 + + [linters-settings.misspell] + locale = "US" + ignore-words = ["listend", "analyses"] + +[linters] + enable = ["vet", "golint", "gofmt", "deadcode", "varcheck", "structcheck", "misspell", "errcheck", "gosimple", "govet", "ineffassign"] + +[issues] + exclude-use-default = false + max-per-linter = 0 + max-same-issues = 0 + exclude = [] + +# Example ignore stuff +# [[issues.exclude-rules]] +# text = "Error return value of `g.DeleteView` is not checked" +# linters = ["errcheck"] +# [[issues.exclude-rules]] +# text = "Error return value of `g.SetCurrentView` is not checked" +# linters = ["errcheck"] +# [[issues.exclude-rules]] +# text = "Error return value of `*.SetCursor` is not checked" +# linters = ["errcheck"] \ No newline at end of file diff --git a/Makefile b/Makefile index 84d59ee..9584813 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ CRD_OPTIONS ?= "crd:trivialVersions=true" all: manager # Run tests -test: generate fmt vet manifests +test: generate fmt checks manifests rm -rf cover.* cover mkdir -p cover @@ -20,7 +20,7 @@ test: generate fmt vet manifests rm -f cover.out cover.out.tmp cover.json # Run tests with existing cluster -test-existing: generate fmt vet manifests +test-existing: generate fmt checks manifests rm -rf cover.* cover mkdir -p cover @@ -33,11 +33,11 @@ test-existing: generate fmt vet manifests rm -f cover.out cover.out.tmp cover.json # Build manager binary -manager: generate fmt vet +manager: generate fmt checks go build -o bin/manager main.go # Run against the configured Kubernetes cluster in ~/.kube/config -run: generate fmt vet +run: generate fmt checks go run ./main.go # Install CRDs into a cluster @@ -80,10 +80,10 @@ manifests: controller-gen fmt: go fmt ./... -# Run go vet against code -vet: - go vet ./... - +# Run linting +checks: + GO111MODULE=on golangci-lint run + # Generate code generate: controller-gen $(CONTROLLER_GEN) object:headerFile=./hack/boilerplate.go.txt paths=./api/... From 0686ee1b60a4b72c1f9bd10c79e2418fcc26622c Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Fri, 15 Nov 2019 10:03:30 +0000 Subject: [PATCH 02/10] Initial work to address linter feedback --- .golangci.toml | 2 +- Makefile | 2 +- api/v1alpha1/dbfsblock_types.go | 11 +++++++- api/v1alpha1/dbfsblock_types_test.go | 2 +- api/v1alpha1/dcluster_types.go | 7 +++++ api/v1alpha1/dcluster_types_test.go | 2 +- api/v1alpha1/djob_types.go | 7 +++++ api/v1alpha1/djob_types_test.go | 2 +- api/v1alpha1/groupversion_info.go | 2 +- api/v1alpha1/helpers.go | 1 + api/v1alpha1/helpers_test.go | 6 +--- api/v1alpha1/run_types.go | 6 ++++ api/v1alpha1/run_types_test.go | 2 +- api/v1alpha1/secretscope_types.go | 6 ++++ api/v1alpha1/secretscope_types_extra.go | 4 +++ api/v1alpha1/secretscope_types_test.go | 2 +- api/v1alpha1/workspaceitem_types.go | 11 +++++++- api/v1alpha1/workspaceitem_types_test.go | 2 +- controllers/dbfsblock_controller.go | 2 ++ .../dbfsblock_controller_databricks.go | 2 +- controllers/dbfsblock_controller_test.go | 2 +- controllers/dcluster_controller.go | 2 ++ controllers/dcluster_controller_test.go | 2 +- controllers/djob_controller.go | 2 ++ controllers/djob_controller_test.go | 6 ++-- controllers/run_controller.go | 2 ++ controllers/run_controller_databricks.go | 12 ++++---- controllers/run_controller_test.go | 6 ++-- controllers/secretscope_controller.go | 4 ++- .../secretscope_controller_databricks.go | 16 +++++------ controllers/secretscope_controller_test.go | 28 +++++++++---------- controllers/suite_test.go | 14 ++++------ controllers/workspaceitem_controller.go | 2 ++ controllers/workspaceitem_controller_test.go | 2 +- main.go | 2 +- 35 files changed, 117 insertions(+), 66 deletions(-) diff --git a/.golangci.toml b/.golangci.toml index 40d462a..2d5f3e5 100644 --- a/.golangci.toml +++ b/.golangci.toml @@ -19,7 +19,7 @@ [linters-settings.misspell] locale = "US" - ignore-words = ["listend", "analyses"] + ignore-words = ["listend", "analyses", "cancelling"] [linters] enable = ["vet", "golint", "gofmt", "deadcode", "varcheck", "structcheck", "misspell", "errcheck", "gosimple", "govet", "ineffassign"] diff --git a/Makefile b/Makefile index 9584813..81cf9a0 100644 --- a/Makefile +++ b/Makefile @@ -78,7 +78,7 @@ manifests: controller-gen # Run go fmt against code fmt: - go fmt ./... + find . -name '*.go' | grep -v vendor | xargs gofmt -s -w # Run linting checks: diff --git a/api/v1alpha1/dbfsblock_types.go b/api/v1alpha1/dbfsblock_types.go index 78e6ade..16195bd 100644 --- a/api/v1alpha1/dbfsblock_types.go +++ b/api/v1alpha1/dbfsblock_types.go @@ -52,10 +52,12 @@ type DbfsBlock struct { Status *DbfsBlockStatus `json:"status,omitempty"` } +// IsBeingDeleted returns true if a deletion timestamp is set func (dbfsBlock *DbfsBlock) IsBeingDeleted() bool { return !dbfsBlock.ObjectMeta.DeletionTimestamp.IsZero() } +// IsSubmitted returns true if the item has been submitted to DataBricks func (dbfsBlock *DbfsBlock) IsSubmitted() bool { if dbfsBlock.Status == nil || dbfsBlock.Status.FileInfo == nil || @@ -81,21 +83,28 @@ func (dbfsBlock *DbfsBlock) GetHash() string { return "" } h := sha1.New() - h.Write(data) + _, err = h.Write(data) + if err != nil { + return "" + } bs := h.Sum(nil) return fmt.Sprintf("%x", bs) } +// DbfsBlockFinalizerName is the name of the dbfs block finalizer const DbfsBlockFinalizerName = "dbfsBlock.finalizers.databricks.microsoft.com" +// HasFinalizer returns true if the item has the specified finalizer func (dbfsBlock *DbfsBlock) HasFinalizer(finalizerName string) bool { return containsString(dbfsBlock.ObjectMeta.Finalizers, finalizerName) } +// AddFinalizer adds the specified finalizer func (dbfsBlock *DbfsBlock) AddFinalizer(finalizerName string) { dbfsBlock.ObjectMeta.Finalizers = append(dbfsBlock.ObjectMeta.Finalizers, finalizerName) } +// RemoveFinalizer removes the specified finalizer func (dbfsBlock *DbfsBlock) RemoveFinalizer(finalizerName string) { dbfsBlock.ObjectMeta.Finalizers = removeString(dbfsBlock.ObjectMeta.Finalizers, finalizerName) } diff --git a/api/v1alpha1/dbfsblock_types_test.go b/api/v1alpha1/dbfsblock_types_test.go index c089eec..97610df 100644 --- a/api/v1alpha1/dbfsblock_types_test.go +++ b/api/v1alpha1/dbfsblock_types_test.go @@ -45,7 +45,7 @@ var _ = Describe("DbfsBlock", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/api/v1alpha1/dcluster_types.go b/api/v1alpha1/dcluster_types.go index 351accb..49f9cab 100644 --- a/api/v1alpha1/dcluster_types.go +++ b/api/v1alpha1/dcluster_types.go @@ -21,6 +21,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// DclusterStatus represents the status for a Dcluster type DclusterStatus struct { ClusterInfo *DclusterInfo `json:"cluster_info,omitempty"` } @@ -40,10 +41,12 @@ type Dcluster struct { Status *DclusterStatus `json:"status,omitempty"` } +// IsBeingDeleted returns true if a deletion timestamp is set func (dcluster *Dcluster) IsBeingDeleted() bool { return !dcluster.ObjectMeta.DeletionTimestamp.IsZero() } +// IsSubmitted returns true if the item has been submitted to DataBricks func (dcluster *Dcluster) IsSubmitted() bool { if dcluster.Status == nil || dcluster.Status.ClusterInfo == nil || @@ -53,16 +56,20 @@ func (dcluster *Dcluster) IsSubmitted() bool { return true } +// DclusterFinalizerName is the name of the finalizer for the Dcluster operator const DclusterFinalizerName = "dcluster.finalizers.databricks.microsoft.com" +// HasFinalizer returns true if the item has the specified finalizer func (dcluster *Dcluster) HasFinalizer(finalizerName string) bool { return containsString(dcluster.ObjectMeta.Finalizers, finalizerName) } +// AddFinalizer adds the specified finalizer func (dcluster *Dcluster) AddFinalizer(finalizerName string) { dcluster.ObjectMeta.Finalizers = append(dcluster.ObjectMeta.Finalizers, finalizerName) } +// RemoveFinalizer removes the specified finalizer func (dcluster *Dcluster) RemoveFinalizer(finalizerName string) { dcluster.ObjectMeta.Finalizers = removeString(dcluster.ObjectMeta.Finalizers, finalizerName) } diff --git a/api/v1alpha1/dcluster_types_test.go b/api/v1alpha1/dcluster_types_test.go index eadc271..5d878b5 100644 --- a/api/v1alpha1/dcluster_types_test.go +++ b/api/v1alpha1/dcluster_types_test.go @@ -45,7 +45,7 @@ var _ = Describe("Dcluster", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/api/v1alpha1/djob_types.go b/api/v1alpha1/djob_types.go index bd3874c..45bedfb 100644 --- a/api/v1alpha1/djob_types.go +++ b/api/v1alpha1/djob_types.go @@ -21,6 +21,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// DjobStatus is the status object for the Djob type DjobStatus struct { JobStatus *dbmodels.Job `json:"job_status,omitempty"` Last10Runs []dbmodels.Run `json:"last_10_runs,omitempty"` @@ -39,10 +40,12 @@ type Djob struct { Status *DjobStatus `json:"status,omitempty"` } +// IsBeingDeleted returns true if a deletion timestamp is set func (djob *Djob) IsBeingDeleted() bool { return !djob.ObjectMeta.DeletionTimestamp.IsZero() } +// IsSubmitted returns true if the item has been submitted to DataBricks func (djob *Djob) IsSubmitted() bool { if djob.Status == nil || djob.Status.JobStatus == nil || djob.Status.JobStatus.JobID == 0 { return false @@ -50,16 +53,20 @@ func (djob *Djob) IsSubmitted() bool { return djob.Status.JobStatus.JobID > 0 } +// DjobFinalizerName is the name of the djob finalizer const DjobFinalizerName = "djob.finalizers.databricks.microsoft.com" +// HasFinalizer returns true if the item has the specified finalizer func (djob *Djob) HasFinalizer(finalizerName string) bool { return containsString(djob.ObjectMeta.Finalizers, finalizerName) } +// AddFinalizer adds the specified finalizer func (djob *Djob) AddFinalizer(finalizerName string) { djob.ObjectMeta.Finalizers = append(djob.ObjectMeta.Finalizers, finalizerName) } +// RemoveFinalizer removes the specified finalizer func (djob *Djob) RemoveFinalizer(finalizerName string) { djob.ObjectMeta.Finalizers = removeString(djob.ObjectMeta.Finalizers, finalizerName) } diff --git a/api/v1alpha1/djob_types_test.go b/api/v1alpha1/djob_types_test.go index d7ac6f2..d95e5ab 100644 --- a/api/v1alpha1/djob_types_test.go +++ b/api/v1alpha1/djob_types_test.go @@ -45,7 +45,7 @@ var _ = Describe("Djob", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go index ac04ffd..561fd11 100644 --- a/api/v1alpha1/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -// package v1alpha1 contains API Schema definitions for the databricks v1 API group +// Package v1alpha1 contains API Schema definitions for the databricks v1 API group // +kubebuilder:object:generate=true // +groupName=databricks.microsoft.com package v1alpha1 diff --git a/api/v1alpha1/helpers.go b/api/v1alpha1/helpers.go index 23ffe3d..0c06629 100644 --- a/api/v1alpha1/helpers.go +++ b/api/v1alpha1/helpers.go @@ -51,6 +51,7 @@ func randomStringWithCharset(length int, charset string) string { return string(b) } +// RandomString generates a random string from a subset of characters func RandomString(length int) string { return randomStringWithCharset(length, charset) } diff --git a/api/v1alpha1/helpers_test.go b/api/v1alpha1/helpers_test.go index 046a7b4..716dfe9 100644 --- a/api/v1alpha1/helpers_test.go +++ b/api/v1alpha1/helpers_test.go @@ -17,16 +17,12 @@ limitations under the License. package v1alpha1 import ( - "time" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) var _ = Describe("Helpers", func() { - const timeout = time.Second * 5 - BeforeEach(func() { // Add any setup steps that needs to be executed before each test }) @@ -35,7 +31,7 @@ var _ = Describe("Helpers", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/api/v1alpha1/run_types.go b/api/v1alpha1/run_types.go index c084cf1..71643de 100644 --- a/api/v1alpha1/run_types.go +++ b/api/v1alpha1/run_types.go @@ -51,10 +51,12 @@ type Run struct { Status *dbazure.JobsRunsGetOutputResponse `json:"status,omitempty"` } +// IsBeingDeleted returns true if a deletion timestamp is set func (run *Run) IsBeingDeleted() bool { return !run.ObjectMeta.DeletionTimestamp.IsZero() } +// IsSubmitted returns true if the item has been submitted to DataBricks func (run *Run) IsSubmitted() bool { if run.Status == nil || run.Status.Metadata.JobID == 0 { return false @@ -62,16 +64,20 @@ func (run *Run) IsSubmitted() bool { return run.Status.Metadata.JobID > 0 } +// RunFinalizerName is the name of the run finalizer const RunFinalizerName = "run.finalizers.databricks.microsoft.com" +// HasFinalizer returns true if the item has the specified finalizer func (run *Run) HasFinalizer(finalizerName string) bool { return containsString(run.ObjectMeta.Finalizers, finalizerName) } +// AddFinalizer adds the specified finalizer func (run *Run) AddFinalizer(finalizerName string) { run.ObjectMeta.Finalizers = append(run.ObjectMeta.Finalizers, finalizerName) } +// RemoveFinalizer removes the specified finalizer func (run *Run) RemoveFinalizer(finalizerName string) { run.ObjectMeta.Finalizers = removeString(run.ObjectMeta.Finalizers, finalizerName) } diff --git a/api/v1alpha1/run_types_test.go b/api/v1alpha1/run_types_test.go index 51029c5..a7a9268 100644 --- a/api/v1alpha1/run_types_test.go +++ b/api/v1alpha1/run_types_test.go @@ -46,7 +46,7 @@ var _ = Describe("Run", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/api/v1alpha1/secretscope_types.go b/api/v1alpha1/secretscope_types.go index d4890c9..41c66f6 100644 --- a/api/v1alpha1/secretscope_types.go +++ b/api/v1alpha1/secretscope_types.go @@ -51,24 +51,30 @@ type SecretScope struct { Status SecretScopeStatus `json:"status,omitempty"` } +// IsSubmitted returns true if the item has been submitted to DataBricks func (ss *SecretScope) IsSubmitted() bool { return ss.Status.SecretScope != nil } +// IsBeingDeleted returns true if a deletion timestamp is set func (ss *SecretScope) IsBeingDeleted() bool { return !ss.ObjectMeta.DeletionTimestamp.IsZero() } +// SecretScopeFinalizerName is the name of the secretscope finalizer const SecretScopeFinalizerName = "secretscope.finalizers.databricks.microsoft.com" +// HasFinalizer returns true if the item has the specified finalizer func (ss *SecretScope) HasFinalizer(finalizerName string) bool { return containsString(ss.ObjectMeta.Finalizers, finalizerName) } +// AddFinalizer adds the specified finalizer func (ss *SecretScope) AddFinalizer(finalizerName string) { ss.ObjectMeta.Finalizers = append(ss.ObjectMeta.Finalizers, finalizerName) } +// RemoveFinalizer removes the specified finalizer func (ss *SecretScope) RemoveFinalizer(finalizerName string) { ss.ObjectMeta.Finalizers = removeString(ss.ObjectMeta.Finalizers, finalizerName) } diff --git a/api/v1alpha1/secretscope_types_extra.go b/api/v1alpha1/secretscope_types_extra.go index ed8d9fc..b88fd1e 100644 --- a/api/v1alpha1/secretscope_types_extra.go +++ b/api/v1alpha1/secretscope_types_extra.go @@ -16,6 +16,7 @@ limitations under the License. package v1alpha1 +// SecretScopeSecret represents a secret in a secret scope type SecretScopeSecret struct { Key string `json:"key,omitempty"` StringValue string `json:"string_value,omitempty"` @@ -23,15 +24,18 @@ type SecretScopeSecret struct { ValueFrom *SecretScopeValueFrom `json:"value_from,omitempty"` } +// SecretScopeACL represents ACLs for a secret scope type SecretScopeACL struct { Principal string `json:"principal,omitempty"` Permission string `json:"permission,omitempty"` } +// SecretScopeValueFrom references a secret scope type SecretScopeValueFrom struct { SecretKeyRef SecretScopeKeyRef `json:"secret_key_ref,omitempty"` } +// SecretScopeKeyRef refers to a secret scope Key type SecretScopeKeyRef struct { Name string `json:"name,omitempty"` Key string `json:"key,omitempty"` diff --git a/api/v1alpha1/secretscope_types_test.go b/api/v1alpha1/secretscope_types_test.go index 7086a51..8528f8a 100644 --- a/api/v1alpha1/secretscope_types_test.go +++ b/api/v1alpha1/secretscope_types_test.go @@ -44,7 +44,7 @@ var _ = Describe("SecretScope", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/api/v1alpha1/workspaceitem_types.go b/api/v1alpha1/workspaceitem_types.go index 58410e0..8707cec 100644 --- a/api/v1alpha1/workspaceitem_types.go +++ b/api/v1alpha1/workspaceitem_types.go @@ -55,10 +55,12 @@ type WorkspaceItem struct { Status *WorkspaceItemStatus `json:"status,omitempty"` } +// IsBeingDeleted returns true if a deletion timestamp is set func (wi *WorkspaceItem) IsBeingDeleted() bool { return !wi.ObjectMeta.DeletionTimestamp.IsZero() } +// IsSubmitted returns true if the item has been submitted to DataBricks func (wi *WorkspaceItem) IsSubmitted() bool { if wi.Status == nil || wi.Status.ObjectInfo == nil || wi.Status.ObjectInfo.Path == "" { return false @@ -82,21 +84,28 @@ func (wi *WorkspaceItem) GetHash() string { return "" } h := sha1.New() - h.Write(data) + _, err = h.Write(data) + if err != nil { + return "" + } bs := h.Sum(nil) return fmt.Sprintf("%x", bs) } +// WorkspaceItemFinalizerName is the name of the workspace item finalizer const WorkspaceItemFinalizerName = "workspaceitem.finalizers.databricks.microsoft.com" +// HasFinalizer returns true if the item has the specified finalizer func (wi *WorkspaceItem) HasFinalizer(finalizerName string) bool { return containsString(wi.ObjectMeta.Finalizers, finalizerName) } +// AddFinalizer adds the specified finalizer func (wi *WorkspaceItem) AddFinalizer(finalizerName string) { wi.ObjectMeta.Finalizers = append(wi.ObjectMeta.Finalizers, finalizerName) } +// RemoveFinalizer removes the specified finalizer func (wi *WorkspaceItem) RemoveFinalizer(finalizerName string) { wi.ObjectMeta.Finalizers = removeString(wi.ObjectMeta.Finalizers, finalizerName) } diff --git a/api/v1alpha1/workspaceitem_types_test.go b/api/v1alpha1/workspaceitem_types_test.go index 394699d..f305cff 100644 --- a/api/v1alpha1/workspaceitem_types_test.go +++ b/api/v1alpha1/workspaceitem_types_test.go @@ -45,7 +45,7 @@ var _ = Describe("WorkspaceItem", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/controllers/dbfsblock_controller.go b/controllers/dbfsblock_controller.go index a162f5d..faff3cd 100644 --- a/controllers/dbfsblock_controller.go +++ b/controllers/dbfsblock_controller.go @@ -42,6 +42,7 @@ type DbfsBlockReconciler struct { // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=dbfsblocks,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=dbfsblocks/status,verbs=get;update;patch +// Reconcile implements the reconciliation loop for the operator func (r *DbfsBlockReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { _ = context.Background() _ = r.Log.WithValues("dbfsblock", req.NamespacedName) @@ -88,6 +89,7 @@ func (r *DbfsBlockReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{}, nil } +// SetupWithManager adds the controller manager func (r *DbfsBlockReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&databricksv1alpha1.DbfsBlock{}). diff --git a/controllers/dbfsblock_controller_databricks.go b/controllers/dbfsblock_controller_databricks.go index 299217d..82e18e1 100644 --- a/controllers/dbfsblock_controller_databricks.go +++ b/controllers/dbfsblock_controller_databricks.go @@ -45,7 +45,7 @@ func (r *DbfsBlockReconciler) submit(instance *databricksv1alpha1.DbfsBlock) err if i+g <= len(data) { err = r.APIClient.Dbfs().AddBlock(createResponse.Handle, data[i:i+g]) } else { - err = r.APIClient.Dbfs().AddBlock(createResponse.Handle, data[i:len(data)]) + err = r.APIClient.Dbfs().AddBlock(createResponse.Handle, data[i:]) } if err != nil { return err diff --git a/controllers/dbfsblock_controller_test.go b/controllers/dbfsblock_controller_test.go index 121a1a0..d4eb240 100644 --- a/controllers/dbfsblock_controller_test.go +++ b/controllers/dbfsblock_controller_test.go @@ -42,7 +42,7 @@ var _ = Describe("DbfsBlock Controller", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/controllers/dcluster_controller.go b/controllers/dcluster_controller.go index 7aafe5f..4c98fbe 100644 --- a/controllers/dcluster_controller.go +++ b/controllers/dcluster_controller.go @@ -43,6 +43,7 @@ type DclusterReconciler struct { // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=dclusters,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=dclusters/status,verbs=get;update;patch +// Reconcile implements the reconciliation loop for the operator func (r *DclusterReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { _ = context.Background() _ = r.Log.WithValues("dcluster", req.NamespacedName) @@ -97,6 +98,7 @@ func (r *DclusterReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } +// SetupWithManager adds the controller manager func (r *DclusterReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&databricksv1alpha1.Dcluster{}). diff --git a/controllers/dcluster_controller_test.go b/controllers/dcluster_controller_test.go index ed234b2..19371a7 100644 --- a/controllers/dcluster_controller_test.go +++ b/controllers/dcluster_controller_test.go @@ -41,7 +41,7 @@ var _ = Describe("DbfsBlock Controller", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/controllers/djob_controller.go b/controllers/djob_controller.go index 80f505f..a963315 100644 --- a/controllers/djob_controller.go +++ b/controllers/djob_controller.go @@ -43,6 +43,7 @@ type DjobReconciler struct { // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=djobs,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=djobs/status,verbs=get;update;patch +// Reconcile implements the reconciliation loop for the operator func (r *DjobReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { _ = context.Background() _ = r.Log.WithValues("djob", req.NamespacedName) @@ -96,6 +97,7 @@ func (r *DjobReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } +// SetupWithManager adds the controller manager func (r *DjobReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&databricksv1alpha1.Djob{}). diff --git a/controllers/djob_controller_test.go b/controllers/djob_controller_test.go index 04005ca..5551927 100644 --- a/controllers/djob_controller_test.go +++ b/controllers/djob_controller_test.go @@ -42,7 +42,7 @@ var _ = Describe("Djob Controller", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. @@ -61,10 +61,10 @@ var _ = Describe("Djob Controller", func() { NumWorkers: 10, }, Libraries: []dbmodels.Library{ - dbmodels.Library{ + { Jar: "dbfs:/my-jar.jar", }, - dbmodels.Library{ + { Maven: &dbmodels.MavenLibrary{ Coordinates: "org.jsoup:jsoup:1.7.2", }, diff --git a/controllers/run_controller.go b/controllers/run_controller.go index edc5a98..9d5c6e1 100644 --- a/controllers/run_controller.go +++ b/controllers/run_controller.go @@ -43,6 +43,7 @@ type RunReconciler struct { // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=runs,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=runs/status,verbs=get;update;patch +// Reconcile implements the reconciliation loop for the operator func (r *RunReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { _ = context.Background() _ = r.Log.WithValues("run", req.NamespacedName) @@ -88,6 +89,7 @@ func (r *RunReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } +// SetupWithManager adds the controller manager func (r *RunReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&databricksv1alpha1.Run{}). diff --git a/controllers/run_controller_databricks.go b/controllers/run_controller_databricks.go index 6a3f1bf..914c031 100644 --- a/controllers/run_controller_databricks.go +++ b/controllers/run_controller_databricks.go @@ -18,11 +18,11 @@ package controllers import ( "context" + "errors" "fmt" "reflect" "strings" "time" - "errors" databricksv1alpha1 "github.com/microsoft/azure-databricks-operator/api/v1alpha1" dbmodels "github.com/xinsnake/databricks-sdk-golang/azure/models" @@ -53,11 +53,11 @@ func (r *RunReconciler) submit(instance *databricksv1alpha1.Run) error { // Here we set the owner attribute k8sJobNamespacedName := types.NamespacedName{Namespace: instance.GetNamespace(), Name: instance.Spec.JobName} var k8sJob databricksv1alpha1.Djob - if err := r.Client.Get(context.Background(), k8sJobNamespacedName, &k8sJob); err != nil { + if err = r.Client.Get(context.Background(), k8sJobNamespacedName, &k8sJob); err != nil { return err } instance.ObjectMeta.SetOwnerReferences([]metav1.OwnerReference{ - metav1.OwnerReference{ + { APIVersion: "v1alpha1", // TODO should this be a referenced value? Kind: "Djob", // TODO should this be a referenced value? Name: k8sJob.GetName(), @@ -104,8 +104,8 @@ func (r *RunReconciler) refresh(instance *databricksv1alpha1.Run) error { if err != nil { return err } - if (len(runOutput.Error) > 0){ - return errors.New(runOutput.Error) + if len(runOutput.Error) > 0 { + return errors.New(runOutput.Error) } err = r.Get(context.Background(), types.NamespacedName{ @@ -143,7 +143,7 @@ func (r *RunReconciler) delete(instance *databricksv1alpha1.Run) error { // We will not check for error when cancelling a job, // if it fails just let it be - r.APIClient.Jobs().RunsCancel(runID) + r.APIClient.Jobs().RunsCancel(runID) //nolint:errcheck // It takes time for DataBricks to cancel a run time.Sleep(15 * time.Second) diff --git a/controllers/run_controller_test.go b/controllers/run_controller_test.go index 18d02ab..e36bcf6 100644 --- a/controllers/run_controller_test.go +++ b/controllers/run_controller_test.go @@ -41,7 +41,7 @@ var _ = Describe("Run Controller", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. @@ -66,10 +66,10 @@ var _ = Describe("Run Controller", func() { NumWorkers: 10, }, Libraries: []dbmodels.Library{ - dbmodels.Library{ + { Jar: "dbfs:/my-jar.jar", }, - dbmodels.Library{ + { Maven: &dbmodels.MavenLibrary{ Coordinates: "org.jsoup:jsoup:1.7.2", }, diff --git a/controllers/secretscope_controller.go b/controllers/secretscope_controller.go index 0619be9..edaa328 100644 --- a/controllers/secretscope_controller.go +++ b/controllers/secretscope_controller.go @@ -44,6 +44,7 @@ type SecretScopeReconciler struct { // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=secretscopes,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=secretscopes/status,verbs=get;update;patch +// Reconcile implements the reconciliation loop for the operator func (r *SecretScopeReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { _ = context.Background() _ = r.Log.WithValues("secretscope", req.NamespacedName) @@ -63,7 +64,7 @@ func (r *SecretScopeReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) } if instance.IsBeingDeleted() { - err := r.handleFinalizer(instance) + err = r.handleFinalizer(instance) if err != nil { return reconcile.Result{}, fmt.Errorf("error when handling finalizer: %v", err) } @@ -91,6 +92,7 @@ func (r *SecretScopeReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } +// SetupWithManager adds the controller manager func (r *SecretScopeReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&databricksv1alpha1.SecretScope{}). diff --git a/controllers/secretscope_controller_databricks.go b/controllers/secretscope_controller_databricks.go index 8acb859..0736b52 100644 --- a/controllers/secretscope_controller_databricks.go +++ b/controllers/secretscope_controller_databricks.go @@ -43,9 +43,8 @@ func (r *SecretScopeReconciler) get(scope string) (*dbmodels.SecretScope, error) if (dbmodels.SecretScope{}) == matchingScope { return nil, fmt.Errorf("get for secret scope failed. scope not found: %s", scope) - } else { - return &matchingScope, nil } + return &matchingScope, nil } func (r *SecretScopeReconciler) submitSecrets(instance *databricksv1alpha1.SecretScope) error { @@ -109,21 +108,20 @@ func (r *SecretScopeReconciler) getSecretValueFrom(namespace string, scopeSecret value := string(secret.Data[scopeSecret.ValueFrom.SecretKeyRef.Key]) return value, nil - } else { - return "", fmt.Errorf("No ValueFrom present to extract secret") } + return "", fmt.Errorf("No ValueFrom present to extract secret") } func (r *SecretScopeReconciler) submitACLs(instance *databricksv1alpha1.SecretScope) error { scope := instance.ObjectMeta.Name - scopeSecretAcls, err := r.APIClient.Secrets().ListSecretACLs(scope) + scopeSecretACLs, err := r.APIClient.Secrets().ListSecretACLs(scope) if err != nil { return err } - if len(scopeSecretAcls) > 0 { - for _, existingAcl := range scopeSecretAcls { - err = r.APIClient.Secrets().DeleteSecretACL(scope, existingAcl.Principal) + if len(scopeSecretACLs) > 0 { + for _, existingACL := range scopeSecretACLs { + err = r.APIClient.Secrets().DeleteSecretACL(scope, existingACL.Principal) if err != nil { return err } @@ -190,7 +188,7 @@ func (r *SecretScopeReconciler) update(instance *databricksv1alpha1.SecretScope) } func (r *SecretScopeReconciler) delete(instance *databricksv1alpha1.SecretScope) error { - + if instance.Status.SecretScope != nil { scope := instance.Status.SecretScope.Name err := r.APIClient.Secrets().DeleteSecretScope(scope) diff --git a/controllers/secretscope_controller_test.go b/controllers/secretscope_controller_test.go index 9537b5e..c205d4f 100644 --- a/controllers/secretscope_controller_test.go +++ b/controllers/secretscope_controller_test.go @@ -49,7 +49,7 @@ var _ = Describe("SecretScope Controller", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. @@ -59,9 +59,9 @@ var _ = Describe("SecretScope Controller", func() { InitialManagePrincipal: "users", SecretScopeSecrets: make([]databricksv1alpha1.SecretScopeSecret, 0), SecretScopeACLs: []databricksv1alpha1.SecretScopeACL{ - databricksv1alpha1.SecretScopeACL{Principal: "admins", Permission: "WRITE"}, - databricksv1alpha1.SecretScopeACL{Principal: "admins", Permission: "READ"}, - databricksv1alpha1.SecretScopeACL{Principal: "admins", Permission: "MANAGE"}, + {Principal: "admins", Permission: "WRITE"}, + {Principal: "admins", Permission: "READ"}, + {Principal: "admins", Permission: "MANAGE"}, }, } @@ -90,7 +90,7 @@ var _ = Describe("SecretScope Controller", func() { By("Updating ACLs successfully") updatedACLs := []databricksv1alpha1.SecretScopeACL{ - databricksv1alpha1.SecretScopeACL{Principal: "admins", Permission: "READ"}, + {Principal: "admins", Permission: "READ"}, } updateSpec := databricksv1alpha1.SecretScopeSpec{ @@ -150,8 +150,8 @@ var _ = Describe("SecretScope Controller", func() { secretValue := "secretValue" byteSecretValue := "aGVsbG8=" initialSecrets := []databricksv1alpha1.SecretScopeSecret{ - databricksv1alpha1.SecretScopeSecret{Key: "secretKey", StringValue: secretValue}, - databricksv1alpha1.SecretScopeSecret{ + {Key: "secretKey", StringValue: secretValue}, + { Key: "secretFromSecret", ValueFrom: &databricksv1alpha1.SecretScopeValueFrom{ SecretKeyRef: databricksv1alpha1.SecretScopeKeyRef{ @@ -160,16 +160,16 @@ var _ = Describe("SecretScope Controller", func() { }, }, }, - databricksv1alpha1.SecretScopeSecret{Key: "byteSecretKey", ByteValue: byteSecretValue}, + {Key: "byteSecretKey", ByteValue: byteSecretValue}, } spec := databricksv1alpha1.SecretScopeSpec{ InitialManagePrincipal: "users", SecretScopeSecrets: initialSecrets, SecretScopeACLs: []databricksv1alpha1.SecretScopeACL{ - databricksv1alpha1.SecretScopeACL{Principal: "admins", Permission: "WRITE"}, - databricksv1alpha1.SecretScopeACL{Principal: "admins", Permission: "READ"}, - databricksv1alpha1.SecretScopeACL{Principal: "admins", Permission: "MANAGE"}, + {Principal: "admins", Permission: "WRITE"}, + {Principal: "admins", Permission: "READ"}, + {Principal: "admins", Permission: "MANAGE"}, }, } @@ -199,15 +199,15 @@ var _ = Describe("SecretScope Controller", func() { By("Updating secrets successfully") newSecretValue := "newSecretValue" updatedSecrets := []databricksv1alpha1.SecretScopeSecret{ - databricksv1alpha1.SecretScopeSecret{Key: "newSecretKey", StringValue: newSecretValue}, + {Key: "newSecretKey", StringValue: newSecretValue}, } updateSpec := databricksv1alpha1.SecretScopeSpec{ InitialManagePrincipal: "users", SecretScopeSecrets: updatedSecrets, SecretScopeACLs: []databricksv1alpha1.SecretScopeACL{ - databricksv1alpha1.SecretScopeACL{Principal: "admins", Permission: "WRITE"}, - databricksv1alpha1.SecretScopeACL{Principal: "admins", Permission: "READ"}, + {Principal: "admins", Permission: "WRITE"}, + {Principal: "admins", Permission: "READ"}, }, } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 101410d..7c26f15 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -30,7 +30,6 @@ import ( databricksv1alpha1 "github.com/microsoft/azure-databricks-operator/api/v1alpha1" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -42,7 +41,6 @@ import ( // These tests use Ginkgo (BDD-style Go testing framework). Refer to // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. -var cfg *rest.Config var k8sClient client.Client var k8sManager ctrl.Manager var testEnv *envtest.Environment @@ -93,13 +91,11 @@ var _ = BeforeSuite(func(done Done) { Fail("Missing environment variable required for tests. DATABRICKS_HOST and DATABRICKS_TOKEN must both be set.") } - apiClient = func() dbazure.DBClient { - var apiClient dbazure.DBClient - return apiClient.Init(db.DBClientOption{ - Host: host, - Token: token, - }) - }() + var apiClient dbazure.DBClient + apiClient.Init(db.DBClientOption{ + Host: host, + Token: token, + }) err = (&SecretScopeReconciler{ Client: k8sManager.GetClient(), diff --git a/controllers/workspaceitem_controller.go b/controllers/workspaceitem_controller.go index b60cbb6..626a97d 100644 --- a/controllers/workspaceitem_controller.go +++ b/controllers/workspaceitem_controller.go @@ -42,6 +42,7 @@ type WorkspaceItemReconciler struct { // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=workspaceitems,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=databricks.microsoft.com,resources=workspaceitems/status,verbs=get;update;patch +// Reconcile implements the reconciliation loop for the operator func (r *WorkspaceItemReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { _ = context.Background() _ = r.Log.WithValues("workspaceitem", req.NamespacedName) @@ -88,6 +89,7 @@ func (r *WorkspaceItemReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro return ctrl.Result{}, nil } +// SetupWithManager adds the controller manager func (r *WorkspaceItemReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&databricksv1alpha1.WorkspaceItem{}). diff --git a/controllers/workspaceitem_controller_test.go b/controllers/workspaceitem_controller_test.go index 86b8dd3..d02d8be 100644 --- a/controllers/workspaceitem_controller_test.go +++ b/controllers/workspaceitem_controller_test.go @@ -40,7 +40,7 @@ var _ = Describe("WorkspaceItem Controller", func() { // Add any teardown steps that needs to be executed after each test }) - // Add Tests for OpenAPI validation (or additonal CRD features) specified in + // Add Tests for OpenAPI validation (or additional CRD features) specified in // your API definition. // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. diff --git a/main.go b/main.go index 780d79f..584e440 100644 --- a/main.go +++ b/main.go @@ -68,7 +68,7 @@ func main() { host, token := os.Getenv("DATABRICKS_HOST"), os.Getenv("DATABRICKS_TOKEN") if len(host) < 10 && len(token) < 10 { err = fmt.Errorf("no valid databricks host / key configured") - setupLog.Error(err, "unable to initialise databricks api client") + setupLog.Error(err, "unable to initialize databricks api client") os.Exit(1) } var apiClient dbazure.DBClient From 5664e83b5697795e46e336f8ee62219085054f5b Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Fri, 15 Nov 2019 10:31:19 +0000 Subject: [PATCH 03/10] Continue on linter issues --- controllers/dbfsblock_controller_test.go | 12 ++++++------ controllers/dcluster_controller_test.go | 4 ++-- controllers/djob_controller_test.go | 4 ++-- controllers/run_controller_test.go | 4 ++-- controllers/secretscope_controller_test.go | 14 +++++++------- controllers/workspaceitem_controller_test.go | 4 ++-- main.go | 2 +- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/controllers/dbfsblock_controller_test.go b/controllers/dbfsblock_controller_test.go index d4eb240..60a7c45 100644 --- a/controllers/dbfsblock_controller_test.go +++ b/controllers/dbfsblock_controller_test.go @@ -50,11 +50,11 @@ var _ = Describe("DbfsBlock Controller", func() { It("Should create successfully", func() { data := make([]byte, 5000) - rand.Read(data) + _, _ = rand.Read(data) dataStr := base64.StdEncoding.EncodeToString(data) data2 := make([]byte, 5500) - rand.Read(data2) + _, _ = rand.Read(data2) dataStr2 := base64.StdEncoding.EncodeToString(data2) key := types.NamespacedName{ @@ -79,14 +79,14 @@ var _ = Describe("DbfsBlock Controller", func() { By("Expecting submitted") Eventually(func() bool { f := &databricksv1alpha1.DbfsBlock{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return f.IsSubmitted() }, timeout, interval).Should(BeTrue()) By("Expecting size to be 5000") Eventually(func() int64 { f := &databricksv1alpha1.DbfsBlock{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return f.Status.FileInfo.FileSize }, timeout, interval).Should(Equal(int64(5000))) @@ -100,7 +100,7 @@ var _ = Describe("DbfsBlock Controller", func() { By("Expecting size to be 5500") Eventually(func() int64 { f := &databricksv1alpha1.DbfsBlock{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return f.Status.FileInfo.FileSize }, timeout, interval).Should(Equal(int64(5500))) @@ -108,7 +108,7 @@ var _ = Describe("DbfsBlock Controller", func() { By("Expecting to delete successfully") Eventually(func() error { f := &databricksv1alpha1.DbfsBlock{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return k8sClient.Delete(context.Background(), f) }, timeout, interval).Should(Succeed()) diff --git a/controllers/dcluster_controller_test.go b/controllers/dcluster_controller_test.go index 19371a7..35d34dd 100644 --- a/controllers/dcluster_controller_test.go +++ b/controllers/dcluster_controller_test.go @@ -74,7 +74,7 @@ var _ = Describe("DbfsBlock Controller", func() { By("Expecting submitted") Eventually(func() bool { f := &databricksv1alpha1.Dcluster{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return f.IsSubmitted() }, timeout, interval).Should(BeTrue()) @@ -82,7 +82,7 @@ var _ = Describe("DbfsBlock Controller", func() { By("Expecting to delete successfully") Eventually(func() error { f := &databricksv1alpha1.Dcluster{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return k8sClient.Delete(context.Background(), f) }, timeout, interval).Should(Succeed()) diff --git a/controllers/djob_controller_test.go b/controllers/djob_controller_test.go index 5551927..ff5c798 100644 --- a/controllers/djob_controller_test.go +++ b/controllers/djob_controller_test.go @@ -95,7 +95,7 @@ var _ = Describe("Djob Controller", func() { By("Expecting submitted") Eventually(func() bool { f := &databricksv1alpha1.Djob{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return f.IsSubmitted() }, timeout, interval).Should(BeTrue()) @@ -103,7 +103,7 @@ var _ = Describe("Djob Controller", func() { By("Expecting to delete successfully") Eventually(func() error { f := &databricksv1alpha1.Djob{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return k8sClient.Delete(context.Background(), f) }, timeout, interval).Should(Succeed()) diff --git a/controllers/run_controller_test.go b/controllers/run_controller_test.go index e36bcf6..8b512d4 100644 --- a/controllers/run_controller_test.go +++ b/controllers/run_controller_test.go @@ -124,7 +124,7 @@ var _ = Describe("Run Controller", func() { By("Expecting run to be submitted") Eventually(func() bool { f := &databricksv1alpha1.Run{} - k8sClient.Get(context.Background(), runKey, f) + _ = k8sClient.Get(context.Background(), runKey, f) return f.IsSubmitted() }, timeout, interval).Should(BeTrue()) @@ -134,7 +134,7 @@ var _ = Describe("Run Controller", func() { By("Expecting run to be deleted successfully") Eventually(func() error { f := &databricksv1alpha1.Run{} - k8sClient.Get(context.Background(), runKey, f) + _ = k8sClient.Get(context.Background(), runKey, f) return k8sClient.Delete(context.Background(), f) }, timeout, interval).Should(Succeed()) diff --git a/controllers/secretscope_controller_test.go b/controllers/secretscope_controller_test.go index c205d4f..caaafe8 100644 --- a/controllers/secretscope_controller_test.go +++ b/controllers/secretscope_controller_test.go @@ -41,7 +41,7 @@ var _ = Describe("SecretScope Controller", func() { // failed test runs that don't clean up leave resources behind. keys := []string{aclKeyName, secretsKeyName} for _, value := range keys { - apiClient.Secrets().DeleteSecretScope(value) + _ = apiClient.Secrets().DeleteSecretScope(value) } }) @@ -84,7 +84,7 @@ var _ = Describe("SecretScope Controller", func() { fetched := &databricksv1alpha1.SecretScope{} Eventually(func() bool { - k8sClient.Get(context.Background(), key, fetched) + _ = k8sClient.Get(context.Background(), key, fetched) return fetched.IsSubmitted() }, timeout, interval).Should(BeTrue()) @@ -104,14 +104,14 @@ var _ = Describe("SecretScope Controller", func() { Expect(k8sClient.Update(context.Background(), fetched)).Should(Succeed()) fetchedUpdated := &databricksv1alpha1.SecretScope{} Eventually(func() []databricksv1alpha1.SecretScopeACL { - k8sClient.Get(context.Background(), key, fetchedUpdated) + _ = k8sClient.Get(context.Background(), key, fetchedUpdated) return fetchedUpdated.Spec.SecretScopeACLs }, timeout, interval).Should(Equal(updatedACLs)) By("Deleting the scope") Eventually(func() error { f := &databricksv1alpha1.SecretScope{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return k8sClient.Delete(context.Background(), f) }, timeout, interval).Should(Succeed()) @@ -192,7 +192,7 @@ var _ = Describe("SecretScope Controller", func() { fetched := &databricksv1alpha1.SecretScope{} Eventually(func() bool { - k8sClient.Get(context.Background(), key, fetched) + _ = k8sClient.Get(context.Background(), key, fetched) return fetched.IsSubmitted() }, timeout, interval).Should(BeTrue()) @@ -216,14 +216,14 @@ var _ = Describe("SecretScope Controller", func() { Expect(k8sClient.Update(context.Background(), fetched)).Should(Succeed()) fetchedUpdated := &databricksv1alpha1.SecretScope{} Eventually(func() []databricksv1alpha1.SecretScopeSecret { - k8sClient.Get(context.Background(), key, fetchedUpdated) + _ = k8sClient.Get(context.Background(), key, fetchedUpdated) return fetchedUpdated.Spec.SecretScopeSecrets }, timeout, interval).Should(Equal(updatedSecrets)) By("Deleting the scope") Eventually(func() error { f := &databricksv1alpha1.SecretScope{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return k8sClient.Delete(context.Background(), f) }, timeout, interval).Should(Succeed()) diff --git a/controllers/workspaceitem_controller_test.go b/controllers/workspaceitem_controller_test.go index d02d8be..d8b5886 100644 --- a/controllers/workspaceitem_controller_test.go +++ b/controllers/workspaceitem_controller_test.go @@ -71,7 +71,7 @@ var _ = Describe("WorkspaceItem Controller", func() { By("Expecting submitted") Eventually(func() bool { f := &databricksv1alpha1.WorkspaceItem{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return f.IsSubmitted() }, timeout, interval).Should(BeTrue()) @@ -86,7 +86,7 @@ var _ = Describe("WorkspaceItem Controller", func() { By("Expecting to delete successfully") Eventually(func() error { f := &databricksv1alpha1.WorkspaceItem{} - k8sClient.Get(context.Background(), key, f) + _ = k8sClient.Get(context.Background(), key, f) return k8sClient.Delete(context.Background(), f) }, timeout, interval).Should(Succeed()) diff --git a/main.go b/main.go index 584e440..32e7696 100644 --- a/main.go +++ b/main.go @@ -39,7 +39,7 @@ var ( ) func init() { - kscheme.AddToScheme(scheme) + _ = kscheme.AddToScheme(scheme) _ = databricksv1alpha1.AddToScheme(scheme) // +kubebuilder:scaffold:scheme } From e48db5fba8371df0dd884d09ebdb687adfe219b7 Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Fri, 15 Nov 2019 10:31:41 +0000 Subject: [PATCH 04/10] remove unused code --- controllers/secretscope_controller_databricks.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/controllers/secretscope_controller_databricks.go b/controllers/secretscope_controller_databricks.go index 0736b52..147eacd 100644 --- a/controllers/secretscope_controller_databricks.go +++ b/controllers/secretscope_controller_databricks.go @@ -178,15 +178,6 @@ func (r *SecretScopeReconciler) submit(instance *databricksv1alpha1.SecretScope) return r.Update(context.Background(), instance) } -func (r *SecretScopeReconciler) update(instance *databricksv1alpha1.SecretScope) error { - err := r.submitSecrets(instance) - if err != nil { - return err - } - - return r.submitACLs(instance) -} - func (r *SecretScopeReconciler) delete(instance *databricksv1alpha1.SecretScope) error { if instance.Status.SecretScope != nil { From d5379768e14ebc783a5f8c5d284faa947749ccf6 Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Fri, 15 Nov 2019 10:34:15 +0000 Subject: [PATCH 05/10] fmt --- controllers/dcluster_controller_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/dcluster_controller_test.go b/controllers/dcluster_controller_test.go index 935802d..4921b07 100644 --- a/controllers/dcluster_controller_test.go +++ b/controllers/dcluster_controller_test.go @@ -64,8 +64,8 @@ var _ = Describe("DbfsBlock Controller", func() { MaxWorkers: 5, }, AutoterminationMinutes: 15, - NodeTypeID: "Standard_D3_v2", - SparkVersion: "5.3.x-scala2.11", + NodeTypeID: "Standard_D3_v2", + SparkVersion: "5.3.x-scala2.11", }, } From 67c69ca832d59c33fa9ca37feba8c3373f8c14da Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Mon, 25 Nov 2019 09:40:45 +0000 Subject: [PATCH 06/10] Convert golangci-lint config to yaml --- .golangci.toml | 42 ------------------------------------------ .golangci.yml | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 42 deletions(-) delete mode 100644 .golangci.toml create mode 100644 .golangci.yml diff --git a/.golangci.toml b/.golangci.toml deleted file mode 100644 index 2d5f3e5..0000000 --- a/.golangci.toml +++ /dev/null @@ -1,42 +0,0 @@ -[run] - deadline = "5m" - skip-files = [] - -[linters-settings] - - [linters-settings.govet] - check-shadowing = true - - [linters-settings.gocyclo] - min-complexity = 12.0 - - [linters-settings.maligned] - suggest-new = true - - [linters-settings.goconst] - min-len = 3.0 - min-occurrences = 3.0 - - [linters-settings.misspell] - locale = "US" - ignore-words = ["listend", "analyses", "cancelling"] - -[linters] - enable = ["vet", "golint", "gofmt", "deadcode", "varcheck", "structcheck", "misspell", "errcheck", "gosimple", "govet", "ineffassign"] - -[issues] - exclude-use-default = false - max-per-linter = 0 - max-same-issues = 0 - exclude = [] - -# Example ignore stuff -# [[issues.exclude-rules]] -# text = "Error return value of `g.DeleteView` is not checked" -# linters = ["errcheck"] -# [[issues.exclude-rules]] -# text = "Error return value of `g.SetCurrentView` is not checked" -# linters = ["errcheck"] -# [[issues.exclude-rules]] -# text = "Error return value of `*.SetCursor` is not checked" -# linters = ["errcheck"] \ No newline at end of file diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..df8d614 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,44 @@ +run: + deadline: 5m + skip-files: [] + +linters-settings: + linters-settings.govet: + check-shadowing: true + + linters-settings.gocyclo: + min-complexity: 12.0 + + linters-settings.maligned: + suggest-new: true + + linters-settings.goconst: + min-len: 3.0 + min-occurrences: 3.0 + + linters-settings.misspell: + locale: "US" + ignore-words: + - listend + - analyses + - cancelling + +linters: + enable: + - vet + - golint + - gofmt + - deadcode + - varcheck + - structcheck + - misspell + - errcheck + - gosimple + - govet + - ineffassign + +issues: + exclude-use-default: false + max-per-linter: 0 + max-same-issues: 0 + exclude: [] From 6542be41d3c24971f2dbeaed70d6706f2fa50d6e Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Mon, 25 Nov 2019 09:41:28 +0000 Subject: [PATCH 07/10] Rename checks to lint --- Makefile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 81cf9a0..728828c 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ CRD_OPTIONS ?= "crd:trivialVersions=true" all: manager # Run tests -test: generate fmt checks manifests +test: generate fmt lint manifests rm -rf cover.* cover mkdir -p cover @@ -20,7 +20,7 @@ test: generate fmt checks manifests rm -f cover.out cover.out.tmp cover.json # Run tests with existing cluster -test-existing: generate fmt checks manifests +test-existing: generate fmt lint manifests rm -rf cover.* cover mkdir -p cover @@ -33,11 +33,11 @@ test-existing: generate fmt checks manifests rm -f cover.out cover.out.tmp cover.json # Build manager binary -manager: generate fmt checks +manager: generate fmt lint go build -o bin/manager main.go # Run against the configured Kubernetes cluster in ~/.kube/config -run: generate fmt checks +run: generate fmt lint go run ./main.go # Install CRDs into a cluster @@ -81,7 +81,7 @@ fmt: find . -name '*.go' | grep -v vendor | xargs gofmt -s -w # Run linting -checks: +lint: GO111MODULE=on golangci-lint run # Generate code From 2865b69ffe46cb9ec938bfc11e7c6432c5b30c0e Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Mon, 25 Nov 2019 09:50:59 +0000 Subject: [PATCH 08/10] re-add vet as per PR review comments --- Makefile | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 728828c..1106d7c 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ CRD_OPTIONS ?= "crd:trivialVersions=true" all: manager # Run tests -test: generate fmt lint manifests +test: generate fmt lint vet manifests rm -rf cover.* cover mkdir -p cover @@ -20,7 +20,7 @@ test: generate fmt lint manifests rm -f cover.out cover.out.tmp cover.json # Run tests with existing cluster -test-existing: generate fmt lint manifests +test-existing: generate fmt lint vet manifests rm -rf cover.* cover mkdir -p cover @@ -33,11 +33,11 @@ test-existing: generate fmt lint manifests rm -f cover.out cover.out.tmp cover.json # Build manager binary -manager: generate fmt lint +manager: generate fmt lint vet go build -o bin/manager main.go # Run against the configured Kubernetes cluster in ~/.kube/config -run: generate fmt lint +run: generate fmt lint vet go run ./main.go # Install CRDs into a cluster @@ -80,6 +80,10 @@ manifests: controller-gen fmt: find . -name '*.go' | grep -v vendor | xargs gofmt -s -w +# Run go vet against code +vet: + go vet ./... + # Run linting lint: GO111MODULE=on golangci-lint run From 8ed13f66d8247fad1d0e049ea7f7c5d597a59cd1 Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Mon, 25 Nov 2019 11:45:38 +0000 Subject: [PATCH 09/10] install golangci-lint in pipeline --- azure-pipelines.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/azure-pipelines.yaml b/azure-pipelines.yaml index 75e23e9..dc95421 100644 --- a/azure-pipelines.yaml +++ b/azure-pipelines.yaml @@ -58,6 +58,8 @@ steps: chmod a+x $(MODULE_PATH)/bin/* # export path export PATH=$PATH:$(MODULE_PATH)/bin + # Install golangci-linter + curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.21.0 # install test dependencies go mod download go get -u github.com/axw/gocov/gocov From 466cdee88376f0fdd3f32265c802e1a11b4409d7 Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Mon, 25 Nov 2019 14:59:29 +0000 Subject: [PATCH 10/10] update crd yaml --- config/crd/bases/databricks.microsoft.com_dclusters.yaml | 1 + config/crd/bases/databricks.microsoft.com_djobs.yaml | 1 + config/crd/bases/databricks.microsoft.com_secretscopes.yaml | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/config/crd/bases/databricks.microsoft.com_dclusters.yaml b/config/crd/bases/databricks.microsoft.com_dclusters.yaml index 0c2954e..c2667f9 100644 --- a/config/crd/bases/databricks.microsoft.com_dclusters.yaml +++ b/config/crd/bases/databricks.microsoft.com_dclusters.yaml @@ -107,6 +107,7 @@ spec: type: string type: object status: + description: DclusterStatus represents the status for a Dcluster properties: cluster_info: description: DclusterInfo is similar to dbmodels.ClusterInfo, the reason diff --git a/config/crd/bases/databricks.microsoft.com_djobs.yaml b/config/crd/bases/databricks.microsoft.com_djobs.yaml index fe6f0b5..b5210e7 100644 --- a/config/crd/bases/databricks.microsoft.com_djobs.yaml +++ b/config/crd/bases/databricks.microsoft.com_djobs.yaml @@ -218,6 +218,7 @@ spec: type: integer type: object status: + description: DjobStatus is the status object for the Djob properties: job_status: properties: diff --git a/config/crd/bases/databricks.microsoft.com_secretscopes.yaml b/config/crd/bases/databricks.microsoft.com_secretscopes.yaml index ac7761a..9af9ae1 100644 --- a/config/crd/bases/databricks.microsoft.com_secretscopes.yaml +++ b/config/crd/bases/databricks.microsoft.com_secretscopes.yaml @@ -32,6 +32,7 @@ spec: properties: acls: items: + description: SecretScopeACL represents ACLs for a secret scope properties: permission: type: string @@ -45,6 +46,7 @@ spec: type: string secrets: items: + description: SecretScopeSecret represents a secret in a secret scope properties: byte_value: type: string @@ -53,8 +55,10 @@ spec: string_value: type: string value_from: + description: SecretScopeValueFrom references a secret scope properties: secret_key_ref: + description: SecretScopeKeyRef refers to a secret scope Key properties: key: type: string