From 9f27818e1cd7e101bc35ac828c1f5a7d9337dbe5 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Mon, 3 Dec 2018 15:53:46 +0100 Subject: [PATCH 1/3] Automatically format imports Signed-off-by: Pavol Loffay --- Makefile | 1 + pkg/service/query.go | 3 +- pkg/storage/dependency_test.go | 3 +- pkg/strategy/controller.go | 5 +- pkg/util/util.go | 3 +- scripts/import-order-cleanup.py | 90 +++++++++++++++++++++++++++++++++ scripts/import-order-cleanup.sh | 5 ++ test/e2e/all_in_one_test.go | 7 +-- test/e2e/cassandra.go | 5 +- test/e2e/daemonset.go | 9 ++-- test/e2e/jaeger_test.go | 2 +- test/e2e/production_test.go | 5 +- test/e2e/sidecar.go | 9 ++-- test/e2e/simplest_test.go | 5 +- 14 files changed, 128 insertions(+), 24 deletions(-) create mode 100644 scripts/import-order-cleanup.py create mode 100755 scripts/import-order-cleanup.sh diff --git a/Makefile b/Makefile index d6da7e084..09edad0fb 100644 --- a/Makefile +++ b/Makefile @@ -29,6 +29,7 @@ ensure-generate-is-noop: generate .PHONY: format format: @echo Formatting code... + @./scripts/import-order-cleanup.sh inplace @go fmt $(PACKAGES) .PHONY: lint diff --git a/pkg/service/query.go b/pkg/service/query.go index 705c941fa..c75836709 100644 --- a/pkg/service/query.go +++ b/pkg/service/query.go @@ -3,10 +3,9 @@ package service import ( "fmt" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" ) diff --git a/pkg/storage/dependency_test.go b/pkg/storage/dependency_test.go index 94d28c18e..0daf29706 100644 --- a/pkg/storage/dependency_test.go +++ b/pkg/storage/dependency_test.go @@ -3,8 +3,9 @@ package storage import ( "testing" - "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" "github.com/stretchr/testify/assert" + + "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" ) func TestDefaultDependencies(t *testing.T) { diff --git a/pkg/strategy/controller.go b/pkg/strategy/controller.go index 2656b6d37..0bee1e9c4 100644 --- a/pkg/strategy/controller.go +++ b/pkg/strategy/controller.go @@ -4,12 +4,13 @@ import ( "context" "strings" - "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" - "github.com/jaegertracing/jaeger-operator/pkg/storage" "github.com/sirupsen/logrus" "github.com/spf13/viper" batchv1 "k8s.io/api/batch/v1" "k8s.io/apimachinery/pkg/runtime" + + "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" + "github.com/jaegertracing/jaeger-operator/pkg/storage" ) // S knows what type of deployments to build based on a given spec diff --git a/pkg/util/util.go b/pkg/util/util.go index 7609290fe..248b4743e 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -1,8 +1,9 @@ package util import ( - "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" "k8s.io/api/core/v1" + + "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" ) // removeDuplicatedVolumes returns a unique list of Volumes based on Volume names. Only the first item is kept. diff --git a/scripts/import-order-cleanup.py b/scripts/import-order-cleanup.py new file mode 100644 index 000000000..bfbe5cab5 --- /dev/null +++ b/scripts/import-order-cleanup.py @@ -0,0 +1,90 @@ +import argparse + +def cleanup_imports_and_return(imports): + os_packages = [] + jaeger_packages = [] + thirdparty_packages = [] + for i in imports: + if i.strip() == "": + continue + if i.find("github.com/jaegertracing/jaeger-operator/") != -1: + jaeger_packages.append(i) + elif i.find(".com") != -1 or i.find(".net") != -1 or i.find(".org") != -1 or i.find(".in") != -1 or i.find("k8s.") != -1: + thirdparty_packages.append(i) + else: + os_packages.append(i) + + l = [] + needs_new_line = False + if os_packages: + l.extend(os_packages) + needs_new_line = True + if thirdparty_packages: + if needs_new_line: + l.append("") + l.extend(thirdparty_packages) + needs_new_line = True + if jaeger_packages: + if needs_new_line: + l.append("") + l.extend(jaeger_packages) + + imports_reordered = imports != l + l.insert(0, "import (") + l.append(")") + return l, imports_reordered + +def parse_go_file(f): + with open(f, 'r') as go_file: + lines = [i.rstrip() for i in go_file.readlines()] + in_import_block = False + imports_reordered = False + imports = [] + output_lines = [] + for line in lines: + if in_import_block: + endIdx = line.find(")") + if endIdx != -1: + in_import_block = False + ordered_imports, imports_reordered = cleanup_imports_and_return(imports) + output_lines.extend(ordered_imports) + imports = [] + continue + imports.append(line) + else: + importIdx = line.find("import (") + if importIdx != -1: + in_import_block = True + continue + output_lines.append(line) + output_lines.append("") + return "\n".join(output_lines), imports_reordered + + +def main(): + parser = argparse.ArgumentParser( + description='Tool to make cleaning up import orders easily') + + parser.add_argument('-o', '--output', default='stdout', + choices=['inplace', 'stdout'], + help='output target [default: stdout]') + + parser.add_argument('-t', '--target', + help='list of filenames to operate upon', + nargs='+', + required=True) + + args = parser.parse_args() + output = args.output + go_files = args.target + + for f in go_files: + parsed, imports_reordered = parse_go_file(f) + if output == "stdout" and imports_reordered: + print(f + " imports out of order") + else: + with open(f, 'w') as ofile: + ofile.write(parsed) + +if __name__ == '__main__': + main() diff --git a/scripts/import-order-cleanup.sh b/scripts/import-order-cleanup.sh new file mode 100755 index 000000000..033a70f50 --- /dev/null +++ b/scripts/import-order-cleanup.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +set -e + +python scripts/import-order-cleanup.py -o $1 -t $(git ls-files "*\.go" | grep -v -e vendor) diff --git a/test/e2e/all_in_one_test.go b/test/e2e/all_in_one_test.go index c96f9eaff..9e6e16376 100644 --- a/test/e2e/all_in_one_test.go +++ b/test/e2e/all_in_one_test.go @@ -9,12 +9,13 @@ import ( "testing" "time" - "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" framework "github.com/operator-framework/operator-sdk/pkg/test" "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" "github.com/sirupsen/logrus" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/wait" + + "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" ) const TrackingID = "MyTrackingId" diff --git a/test/e2e/cassandra.go b/test/e2e/cassandra.go index 8d624fc87..89c99daf3 100644 --- a/test/e2e/cassandra.go +++ b/test/e2e/cassandra.go @@ -5,11 +5,12 @@ import ( "fmt" "testing" - "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" framework "github.com/operator-framework/operator-sdk/pkg/test" "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" "github.com/sirupsen/logrus" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" ) // Cassandra runs a test with Cassandra as the backing storage diff --git a/test/e2e/daemonset.go b/test/e2e/daemonset.go index 4a5a153aa..59d30fed4 100644 --- a/test/e2e/daemonset.go +++ b/test/e2e/daemonset.go @@ -9,15 +9,16 @@ import ( "testing" "time" - "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" - framework "github.com/operator-framework/operator-sdk/pkg/test" - "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" - "github.com/sirupsen/logrus" appsv1 "k8s.io/api/apps/v1" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" + framework "github.com/operator-framework/operator-sdk/pkg/test" + "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" + "github.com/sirupsen/logrus" + + "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" ) // DaemonSet runs a test with the agent as DaemonSet diff --git a/test/e2e/jaeger_test.go b/test/e2e/jaeger_test.go index 8bae44950..336855d91 100644 --- a/test/e2e/jaeger_test.go +++ b/test/e2e/jaeger_test.go @@ -4,9 +4,9 @@ import ( "testing" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" framework "github.com/operator-framework/operator-sdk/pkg/test" "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/jaegertracing/jaeger-operator/pkg/apis" "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" diff --git a/test/e2e/production_test.go b/test/e2e/production_test.go index 183ba228c..175655f65 100644 --- a/test/e2e/production_test.go +++ b/test/e2e/production_test.go @@ -5,10 +5,11 @@ import ( "fmt" "testing" - "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" framework "github.com/operator-framework/operator-sdk/pkg/test" "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" ) func SimpleProd(t *testing.T) { diff --git a/test/e2e/sidecar.go b/test/e2e/sidecar.go index 7d01e24c0..59555b1bd 100644 --- a/test/e2e/sidecar.go +++ b/test/e2e/sidecar.go @@ -9,15 +9,16 @@ import ( "testing" "time" - "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" - "github.com/jaegertracing/jaeger-operator/pkg/inject" - framework "github.com/operator-framework/operator-sdk/pkg/test" - "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" appsv1 "k8s.io/api/apps/v1" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" + framework "github.com/operator-framework/operator-sdk/pkg/test" + "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" + + "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" + "github.com/jaegertracing/jaeger-operator/pkg/inject" ) // Sidecar runs a test with the agent as sidecar diff --git a/test/e2e/simplest_test.go b/test/e2e/simplest_test.go index 4bd6f052f..443501269 100644 --- a/test/e2e/simplest_test.go +++ b/test/e2e/simplest_test.go @@ -5,10 +5,11 @@ import ( "fmt" "testing" - "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" framework "github.com/operator-framework/operator-sdk/pkg/test" "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" ) func SimplestJaeger(t *testing.T) { From c28e02cf6e8850696f1a8cd28cd7da1cfe20caf4 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Mon, 3 Dec 2018 16:57:26 +0100 Subject: [PATCH 2/3] Move to .travis Signed-off-by: Pavol Loffay --- {scripts => .travis}/import-order-cleanup.py | 0 .travis/import-order-cleanup.sh | 5 +++++ Makefile | 2 +- scripts/import-order-cleanup.sh | 5 ----- 4 files changed, 6 insertions(+), 6 deletions(-) rename {scripts => .travis}/import-order-cleanup.py (100%) create mode 100755 .travis/import-order-cleanup.sh delete mode 100755 scripts/import-order-cleanup.sh diff --git a/scripts/import-order-cleanup.py b/.travis/import-order-cleanup.py similarity index 100% rename from scripts/import-order-cleanup.py rename to .travis/import-order-cleanup.py diff --git a/.travis/import-order-cleanup.sh b/.travis/import-order-cleanup.sh new file mode 100755 index 000000000..2ff502790 --- /dev/null +++ b/.travis/import-order-cleanup.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +set -e + +python .travis/import-order-cleanup.py -o $1 -t $(git ls-files "*\.go" | grep -v -e vendor) diff --git a/Makefile b/Makefile index 09edad0fb..8728ac933 100644 --- a/Makefile +++ b/Makefile @@ -29,7 +29,7 @@ ensure-generate-is-noop: generate .PHONY: format format: @echo Formatting code... - @./scripts/import-order-cleanup.sh inplace + @.travis/import-order-cleanup.sh inplace @go fmt $(PACKAGES) .PHONY: lint diff --git a/scripts/import-order-cleanup.sh b/scripts/import-order-cleanup.sh deleted file mode 100755 index 033a70f50..000000000 --- a/scripts/import-order-cleanup.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/bin/bash - -set -e - -python scripts/import-order-cleanup.py -o $1 -t $(git ls-files "*\.go" | grep -v -e vendor) From 5e83c531eadaea98645fb0d6a40f7ebc589e2e75 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 4 Dec 2018 11:44:07 +0100 Subject: [PATCH 3/3] Fail check if import order is incorrect Signed-off-by: Pavol Loffay --- .gitignore | 2 ++ Makefile | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 9c19bbccf..8154bfb3b 100644 --- a/.gitignore +++ b/.gitignore @@ -76,3 +76,5 @@ tags .vscode/* .history # End of https://www.gitignore.io/api/go,vim,emacs,visualstudiocode +fmt.log +import.log diff --git a/Makefile b/Makefile index 8728ac933..3f7f5c89f 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,8 @@ GO_FLAGS ?= GOOS=linux GOARCH=amd64 CGO_ENABLED=0 KUBERNETES_CONFIG ?= "$(HOME)/.kube/config" WATCH_NAMESPACE ?= default BIN_DIR ?= "build/_output/bin" +IMPORT_LOG=import.log +FMT_LOG=fmt.log OPERATOR_NAME ?= jaeger-operator NAMESPACE ?= "$(USER)" @@ -20,7 +22,9 @@ PACKAGES := $(shell go list ./cmd/... ./pkg/...) .PHONY: check check: @echo Checking... - @$(foreach file, $(shell go fmt $(PACKAGES) 2>&1), echo "Some files need formatting. Failing." || exit 1) + @go fmt $(PACKAGES) > $(FMT_LOG) + @.travis/import-order-cleanup.sh stdout > $(IMPORT_LOG) + @[ ! -s "$(FMT_LOG)" -a ! -s "$(IMPORT_LOG)" ] || (echo "Go fmt, license check, or import ordering failures, run 'make format'" | cat - $(FMT_LOG) $(IMPORT_LOG) && false) .PHONY: ensure-generate-is-noop ensure-generate-is-noop: generate