diff --git a/Makefile b/Makefile index a2351c7fb..f0c616417 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ docker-manifest: @echo skip manifest creation .PHONY: docs -docs: build +docs: ./scripts/genflagdocs.sh .PHONY: docs-check diff --git a/funcbench/Makefile b/funcbench/Makefile index 4ed4b8729..dc0cff9fc 100644 --- a/funcbench/Makefile +++ b/funcbench/Makefile @@ -8,12 +8,12 @@ clean: resource_delete cluster_delete cluster_create: $(INFRA_CMD) gke cluster create -a ${AUTH_FILE} \ - -v PROJECT_ID:${PROJECT_ID} -v ZONE:${ZONE} -v CLUSTER_NAME:funcbench-${PR_NUMBER} \ + -v PROJECT_ID:${PROJECT_ID} -v ZONE:${ZONE} -v CLUSTER_NAME:funcbench-${PR_NUMBER} -v PR_NUMBER:${PR_NUMBER} \ -f manifests/cluster.yaml cluster_delete: $(INFRA_CMD) gke cluster delete -a ${AUTH_FILE} \ - -v PROJECT_ID:${PROJECT_ID} -v ZONE:${ZONE} -v CLUSTER_NAME:funcbench-${PR_NUMBER} \ + -v PROJECT_ID:${PROJECT_ID} -v ZONE:${ZONE} -v CLUSTER_NAME:funcbench-${PR_NUMBER} -v PR_NUMBER:${PR_NUMBER} \ -f manifests/cluster.yaml resource_apply: diff --git a/funcbench/README.md b/funcbench/README.md index 8da866b24..07c4a75d8 100644 --- a/funcbench/README.md +++ b/funcbench/README.md @@ -12,8 +12,7 @@ funcbench currently supports two modes, Local and GitHub. Running it in the Gith > Clean git state is required. -[embedmd]: # "funcbench-flags.txt" - +[embedmd]:# (funcbench-flags.txt) ```txt usage: funcbench [] [] @@ -47,11 +46,12 @@ Flags: longer than duration d, panic. Args: - Can be one of '.', branch name or commit SHA of the - branch to compare against. If set to '.', branch/commit - is the same as the current one; funcbench will run once - and try to compare between 2 sub-benchmarks. Errors out - if there are no sub-benchmarks. + Can be one of '.', tag name, branch name or commit SHA + of the branch to compare against. If set to '.', + branch/commit is the same as the current one; funcbench + will run once and try to compare between 2 + sub-benchmarks. Errors out if there are no + sub-benchmarks. [] Function regex to use for benchmark.Supports RE2 regexp and is fully anchored, by default will run all benchmarks. diff --git a/funcbench/env.go b/funcbench/env.go index a7eb2a536..b711dfdb0 100644 --- a/funcbench/env.go +++ b/funcbench/env.go @@ -84,7 +84,6 @@ func newGitHubEnv(ctx context.Context, e environment, gc *gitHubClient, workspac r, err := git.PlainCloneContext(ctx, fmt.Sprintf("%s/%s", workspace, gc.repo), false, &git.CloneOptions{ URL: fmt.Sprintf("https://github.com/%s/%s.git", gc.owner, gc.repo), Progress: os.Stdout, - Depth: 1, }) if err != nil { return nil, errors.Wrap(err, "clone git repository") diff --git a/funcbench/main.go b/funcbench/main.go index d6ea53cb3..97f969717 100644 --- a/funcbench/main.go +++ b/funcbench/main.go @@ -22,7 +22,6 @@ import ( "os/exec" "os/signal" "path/filepath" - "strings" "syscall" "time" @@ -103,7 +102,7 @@ func main() { "disabled if set to 0. If a test binary runs longer than duration d, panic."). Short('d').Default("2h").DurationVar(&cfg.benchTimeout) - app.Arg("target", "Can be one of '.', branch name or commit SHA of the branch "+ + app.Arg("target", "Can be one of '.', tag name, branch name or commit SHA of the branch "+ "to compare against. If set to '.', branch/commit is the same as the current one; "+ "funcbench will run once and try to compare between 2 sub-benchmarks. "+ "Errors out if there are no sub-benchmarks."). @@ -215,14 +214,7 @@ func startBenchmark( return nil, errors.Wrap(err, "not clean worktree") } - // Get info about target. - targetCommit, compareWithItself, err := getTargetInfo(ctx, env.Repo(), env.CompareTarget()) - if err != nil { - return nil, errors.Wrap(err, "getTargetInfo") - } - bench.logger.Println("Target:", targetCommit.String(), "Current Ref:", ref.Hash().String()) - - if compareWithItself { + if env.CompareTarget() == "." { bench.logger.Println("Assuming sub-benchmarks comparison.") subResult, err := bench.exec(ctx, wt.Filesystem.Root(), ref.Hash()) if err != nil { @@ -236,6 +228,18 @@ func startBenchmark( return cmps, nil } + // Get info about target. + targetCommit := getTargetInfo(env.Repo(), env.CompareTarget()) + if targetCommit == plumbing.ZeroHash { + return nil, fmt.Errorf("cannot find target %s", env.CompareTarget()) + } + + bench.logger.Println("Target:", targetCommit.String(), "Current Ref:", ref.Hash().String()) + + if targetCommit == ref.Hash() { + return nil, fmt.Errorf("target: %s is the same as current ref %s (or is on the same commit); No changes would be expected; Aborting", targetCommit, ref.String()) + } + bench.logger.Println("Assuming comparing with target (clean workdir will be checked.)") // Execute benchmark A. @@ -285,33 +289,15 @@ func interrupt(logger Logger, cancel <-chan struct{}) error { } } -// getTargetInfo returns the hash of the target, -// if target is the same as the current ref, set compareWithItself to true. -func getTargetInfo(ctx context.Context, repo *git.Repository, target string) (ref plumbing.Hash, compareWithItself bool, _ error) { - if target == "." { - return plumbing.Hash{}, true, nil - } - - currRef, err := repo.Head() +// getTargetInfo returns the hash of the target if found, +// otherwise returns plumbing.ZeroHash. +// NOTE: if both a branch and a tag have the same name, it always chooses the branch name. +func getTargetInfo(repo *git.Repository, target string) plumbing.Hash { + hash, err := repo.ResolveRevision(plumbing.Revision(target)) if err != nil { - return plumbing.ZeroHash, false, err + return plumbing.ZeroHash } - - if target == strings.TrimPrefix(currRef.Name().String(), "refs/heads/") || target == currRef.Hash().String() { - return currRef.Hash(), true, errors.Errorf("target: %s is the same as current ref %s (or is on the same commit); No changes would be expected; Aborting", target, currRef.String()) - } - - commitHash := plumbing.NewHash(target) - if !commitHash.IsZero() { - return commitHash, false, nil - } - - targetRef, err := repo.Reference(plumbing.NewBranchReferenceName(target), false) - if err != nil { - return plumbing.ZeroHash, false, err - } - - return targetRef.Hash(), false, nil + return *hash } type commander struct { diff --git a/funcbench/main_test.go b/funcbench/main_test.go index e86c4fd66..438e141ec 100644 --- a/funcbench/main_test.go +++ b/funcbench/main_test.go @@ -15,6 +15,12 @@ package main import ( "testing" + + fixtures "gopkg.in/src-d/go-git-fixtures.v3" + "gopkg.in/src-d/go-git.v4" + "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" + "gopkg.in/src-d/go-git.v4/storage/filesystem" ) func TestMarkdownFormatting(t *testing.T) { @@ -49,3 +55,29 @@ BenchmarkBufferedSeriesIterator-8 0 0 +0.00%` t.Errorf("Output did not match.\ngot:\n%#v\nwant:\n%#v", formattedTable, expectedTable) } } + +func TestGetTargetInfo(t *testing.T) { + _ = fixtures.Init() + f := fixtures.Basic().One() + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) + r, err := git.Open(sto, f.DotGit()) + if err != nil { + t.Errorf("error when open repository: %s", err) + } + + testCases := map[string]string{ + "notFound": plumbing.ZeroHash.String(), + "HEAD": "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", + "master": "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", + "branch": "e8d3ffab552895c19b9fcf7aa264d277cde33881", + "v1.0.0": "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", + "918c48b83bd081e863dbe1b80f8998f058cd8294": "918c48b83bd081e863dbe1b80f8998f058cd8294", + } + + for target, hash := range testCases { + commit := getTargetInfo(r, target) + if commit.String() != hash { + t.Errorf("error when get target %s, expect %s, got %s", target, hash, commit) + } + } +} diff --git a/funcbench/manifests/benchmark/2_job.yaml b/funcbench/manifests/benchmark/2_job.yaml index b2f3064a6..b543eb2c6 100644 --- a/funcbench/manifests/benchmark/2_job.yaml +++ b/funcbench/manifests/benchmark/2_job.yaml @@ -27,4 +27,4 @@ spec: - name: GITHUB_TOKEN value: "{{ .GITHUB_TOKEN }}" nodeSelector: - cloud.google.com/gke-nodepool: funcbench-{{ .PR_NUMBER }} + node-name: funcbench-{{ .PR_NUMBER }} \ No newline at end of file diff --git a/funcbench/manifests/cluster.yaml b/funcbench/manifests/cluster.yaml index fb5311adb..c21d7e89f 100644 --- a/funcbench/manifests/cluster.yaml +++ b/funcbench/manifests/cluster.yaml @@ -2,7 +2,7 @@ projectid: {{ .PROJECT_ID }} zone: {{ .ZONE }} cluster: name: {{ .CLUSTER_NAME }} - initialclusterversion: 1.15 + initialclusterversion: 1.16 nodepools: - name: {{ .CLUSTER_NAME }} initialnodecount: 1 @@ -10,3 +10,5 @@ cluster: machinetype: n1-highmem-4 imagetype: COS disksizegb: 100 + labels: + node-name: funcbench-{{ .PR_NUMBER }} \ No newline at end of file diff --git a/go.mod b/go.mod index 984330595..34a2b8f3f 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( google.golang.org/genproto v0.0.0-20200331122359-1ee6d9798940 google.golang.org/grpc v1.28.0 gopkg.in/alecthomas/kingpin.v2 v2.2.6 + gopkg.in/src-d/go-git-fixtures.v3 v3.5.0 gopkg.in/src-d/go-git.v4 v4.13.1 gopkg.in/yaml.v2 v2.3.0 k8s.io/api v0.18.4 diff --git a/infra/README.md b/infra/README.md index 8c1c47f64..c9014f11d 100644 --- a/infra/README.md +++ b/infra/README.md @@ -10,8 +10,7 @@ Eg. `somefile.yaml` will be parsed, whereas `somefile_noparse.yaml` will not be ## Usage and examples: -[embedmd]: # "./infra-flags.txt" - +[embedmd]:# (infra-flags.txt) ```txt usage: infra [] [ ...] diff --git a/prombench/manifests/cluster-infra/2_ingress-nginx-controller.yaml b/prombench/manifests/cluster-infra/2_ingress-nginx-controller.yaml index a5ed7f8b9..f7dbe4b6a 100644 --- a/prombench/manifests/cluster-infra/2_ingress-nginx-controller.yaml +++ b/prombench/manifests/cluster-infra/2_ingress-nginx-controller.yaml @@ -262,7 +262,7 @@ spec: successThreshold: 1 timeoutSeconds: 10 nodeSelector: - cloud.google.com/gke-nodepool: main-node + node-name: main-node --- kind: Service diff --git a/prombench/manifests/cluster-infra/3b_prometheus-meta.yaml b/prombench/manifests/cluster-infra/3b_prometheus-meta.yaml index 8cad3bfec..39b98dcb7 100644 --- a/prombench/manifests/cluster-infra/3b_prometheus-meta.yaml +++ b/prombench/manifests/cluster-infra/3b_prometheus-meta.yaml @@ -232,7 +232,7 @@ spec: claimName: prometheus-meta terminationGracePeriodSeconds: 300 nodeSelector: - cloud.google.com/gke-nodepool: main-node + node-name: main-node --- apiVersion: v1 kind: Service diff --git a/prombench/manifests/cluster-infra/5a_alertmanager_deployment.yaml b/prombench/manifests/cluster-infra/5a_alertmanager_deployment.yaml index 1cdcabbaf..b9d3bfc92 100644 --- a/prombench/manifests/cluster-infra/5a_alertmanager_deployment.yaml +++ b/prombench/manifests/cluster-infra/5a_alertmanager_deployment.yaml @@ -63,7 +63,7 @@ spec: claimName: prometheus-meta terminationGracePeriodSeconds: 300 nodeSelector: - cloud.google.com/gke-nodepool: main-node + node-name: main-node --- apiVersion: v1 kind: Service diff --git a/prombench/manifests/cluster-infra/5b_amGithubNotifier_deployment.yaml b/prombench/manifests/cluster-infra/5b_amGithubNotifier_deployment.yaml index 070ece492..65ef7c8e0 100644 --- a/prombench/manifests/cluster-infra/5b_amGithubNotifier_deployment.yaml +++ b/prombench/manifests/cluster-infra/5b_amGithubNotifier_deployment.yaml @@ -33,7 +33,7 @@ spec: secretName: oauth-token terminationGracePeriodSeconds: 300 nodeSelector: - cloud.google.com/gke-nodepool: main-node + node-name: main-node --- apiVersion: v1 kind: Service diff --git a/prombench/manifests/cluster-infra/6b_loki_stateful_set.yaml b/prombench/manifests/cluster-infra/6b_loki_stateful_set.yaml index c6aeaeb50..07dc998bb 100644 --- a/prombench/manifests/cluster-infra/6b_loki_stateful_set.yaml +++ b/prombench/manifests/cluster-infra/6b_loki_stateful_set.yaml @@ -113,7 +113,7 @@ spec: securityContext: readOnlyRootFilesystem: true nodeSelector: - cloud.google.com/gke-nodepool: main-node + node-name: main-node terminationGracePeriodSeconds: 30 volumes: - name: config diff --git a/prombench/manifests/cluster-infra/6d_promtail_daemon_set.yaml b/prombench/manifests/cluster-infra/6d_promtail_daemon_set.yaml index e1e9d3394..9dc4016e5 100644 --- a/prombench/manifests/cluster-infra/6d_promtail_daemon_set.yaml +++ b/prombench/manifests/cluster-infra/6d_promtail_daemon_set.yaml @@ -121,7 +121,7 @@ spec: successThreshold: 1 timeoutSeconds: 1 nodeSelector: - cloud.google.com/gke-nodepool: main-node + node-name: main-node tolerations: - key: node-role.kubernetes.io/master effect: NoSchedule diff --git a/prombench/manifests/cluster-infra/7a_commentmonitor_configmap_noparse.yaml b/prombench/manifests/cluster-infra/7a_commentmonitor_configmap_noparse.yaml index 035f2a133..df5b59adf 100644 --- a/prombench/manifests/cluster-infra/7a_commentmonitor_configmap_noparse.yaml +++ b/prombench/manifests/cluster-infra/7a_commentmonitor_configmap_noparse.yaml @@ -6,6 +6,7 @@ data: eventmap.yml: | - event_type: prombench_start regex_string: (?mi)^/prombench\s*(?Pmaster|v[0-9]+\.[0-9]+\.[0-9]+\S*)\s*$ + label: prombench comment_template: | ⏱️ Welcome to Prometheus Benchmarking Tool. ⏱️ @@ -51,6 +52,7 @@ data: - event_type: funcbench_start regex_string: (?m)^/funcbench[[:blank:]]+(?P[\w\-\/\.]+)[[:blank:]]*(?P(?:Benchmark\w*)?(?:\.\*)?)?[[:blank:]]*$ + label: funcbench comment_template: | ⏱️ Welcome to Funcbench Tool. ⏱️ diff --git a/prombench/manifests/cluster-infra/7b_commentmonitor_deployment.yaml b/prombench/manifests/cluster-infra/7b_commentmonitor_deployment.yaml index 4f7de10f3..377bfd2f9 100644 --- a/prombench/manifests/cluster-infra/7b_commentmonitor_deployment.yaml +++ b/prombench/manifests/cluster-infra/7b_commentmonitor_deployment.yaml @@ -20,12 +20,12 @@ spec: args: - "--eventmap=/etc/cm/eventmap.yml" - "--webhooksecretfile=/etc/github/whsecret" + - "--command-prefix=/prombench" + - "--command-prefix=/funcbench" name: comment-monitor env: - name: DOMAIN_NAME value: {{ .DOMAIN_NAME }} - - name: LABEL_NAME - value: prombench - name: GITHUB_TOKEN valueFrom: secretKeyRef: @@ -49,7 +49,7 @@ spec: name: comment-monitor-eventmap terminationGracePeriodSeconds: 300 nodeSelector: - cloud.google.com/gke-nodepool: main-node + node-name: main-node --- apiVersion: v1 kind: Service diff --git a/prombench/manifests/cluster-infra/grafana_deployment.yaml b/prombench/manifests/cluster-infra/grafana_deployment.yaml index dec2a58af..749bf0023 100644 --- a/prombench/manifests/cluster-infra/grafana_deployment.yaml +++ b/prombench/manifests/cluster-infra/grafana_deployment.yaml @@ -70,7 +70,7 @@ spec: configMap: name: grafana-dashboards nodeSelector: - cloud.google.com/gke-nodepool: main-node + node-name: main-node --- apiVersion: v1 kind: Service diff --git a/prombench/manifests/cluster-infra/node-exporter.yaml b/prombench/manifests/cluster-infra/node-exporter.yaml index 09aa9467b..6a6581499 100644 --- a/prombench/manifests/cluster-infra/node-exporter.yaml +++ b/prombench/manifests/cluster-infra/node-exporter.yaml @@ -24,7 +24,7 @@ spec: hostNetwork: true hostPID: true nodeSelector: - cloud.google.com/gke-nodepool: main-node + node-name: main-node containers: - image: quay.io/prometheus/node-exporter:v0.16.0 args: diff --git a/prombench/manifests/cluster.yaml b/prombench/manifests/cluster.yaml index 5217642b7..48a840ad2 100644 --- a/prombench/manifests/cluster.yaml +++ b/prombench/manifests/cluster.yaml @@ -4,8 +4,6 @@ cluster: name: {{ .CLUSTER_NAME }} initialclusterversion: 1.14 nodepools: - # GKE creates a label on every node in a pool - # cloud.google.com/gke-nodepool: # This node-pool will be used for running monitoring components - name: main-node initialnodecount: 1 @@ -13,3 +11,5 @@ cluster: machinetype: n1-standard-4 imagetype: COS disksizegb: 300 + labels: + node-name: main-node diff --git a/prombench/manifests/prombench/benchmark/2_fake-webserver.yaml b/prombench/manifests/prombench/benchmark/2_fake-webserver.yaml index 6a29ffcc0..aefb00d71 100644 --- a/prombench/manifests/prombench/benchmark/2_fake-webserver.yaml +++ b/prombench/manifests/prombench/benchmark/2_fake-webserver.yaml @@ -36,7 +36,7 @@ data: - name: metrics5 containerPort: 8084 nodeSelector: - cloud.google.com/gke-nodepool: nodes-{{ .PR_NUMBER }} + node-name: nodes-{{ .PR_NUMBER }} isolation: none --- apiVersion: apps/v1 @@ -70,7 +70,7 @@ spec: - name: metrics5 containerPort: 8084 nodeSelector: - cloud.google.com/gke-nodepool: nodes-{{ .PR_NUMBER }} + node-name: nodes-{{ .PR_NUMBER }} isolation: none --- apiVersion: v1 diff --git a/prombench/manifests/prombench/benchmark/3b_prometheus-test_deployment.yaml b/prombench/manifests/prombench/benchmark/3b_prometheus-test_deployment.yaml index 86c13797a..6e4880e68 100644 --- a/prombench/manifests/prombench/benchmark/3b_prometheus-test_deployment.yaml +++ b/prombench/manifests/prombench/benchmark/3b_prometheus-test_deployment.yaml @@ -91,7 +91,7 @@ spec: emptyDir: {} terminationGracePeriodSeconds: 300 nodeSelector: - cloud.google.com/gke-nodepool: prometheus-{{ .PR_NUMBER }} + node-name: prometheus-{{ .PR_NUMBER }} isolation: prometheus --- apiVersion: v1 @@ -178,7 +178,7 @@ spec: path: /mnt/disks/ssd0 terminationGracePeriodSeconds: 300 nodeSelector: - cloud.google.com/gke-nodepool: prometheus-{{ .PR_NUMBER }} + node-name: prometheus-{{ .PR_NUMBER }} isolation: prometheus --- apiVersion: v1 diff --git a/prombench/manifests/prombench/benchmark/3c_promtail-bench_deployment.yaml b/prombench/manifests/prombench/benchmark/3c_promtail-bench_deployment.yaml index 1ef67dec8..413522ff2 100644 --- a/prombench/manifests/prombench/benchmark/3c_promtail-bench_deployment.yaml +++ b/prombench/manifests/prombench/benchmark/3c_promtail-bench_deployment.yaml @@ -136,7 +136,7 @@ spec: successThreshold: 1 timeoutSeconds: 1 nodeSelector: - cloud.google.com/gke-nodepool: prometheus-{{ .PR_NUMBER }} + node-name: prometheus-{{ .PR_NUMBER }} isolation: prometheus tolerations: - key: node-role.kubernetes.io/master @@ -229,7 +229,7 @@ spec: successThreshold: 1 timeoutSeconds: 1 nodeSelector: - cloud.google.com/gke-nodepool: prometheus-{{ .PR_NUMBER }} + node-name: prometheus-{{ .PR_NUMBER }} isolation: prometheus tolerations: - key: node-role.kubernetes.io/master diff --git a/prombench/manifests/prombench/benchmark/4_node-exporter.yaml b/prombench/manifests/prombench/benchmark/4_node-exporter.yaml index 7fe7dc1af..49da47b40 100644 --- a/prombench/manifests/prombench/benchmark/4_node-exporter.yaml +++ b/prombench/manifests/prombench/benchmark/4_node-exporter.yaml @@ -34,14 +34,14 @@ spec: operator: In values: - test-{{ normalise .RELEASE }} - topologyKey: cloud.google.com/gke-nodepool + topologyKey: node-name securityContext: runAsNonRoot: true runAsUser: 65534 hostNetwork: true hostPID: true nodeSelector: - cloud.google.com/gke-nodepool: prometheus-{{ .PR_NUMBER }} + node-name: prometheus-{{ .PR_NUMBER }} containers: - image: quay.io/prometheus/node-exporter:v0.16.0 args: @@ -101,14 +101,14 @@ spec: operator: In values: - test-pr-{{ .PR_NUMBER }} - topologyKey: cloud.google.com/gke-nodepool + topologyKey: node-name securityContext: runAsNonRoot: true runAsUser: 65534 hostNetwork: true hostPID: true nodeSelector: - cloud.google.com/gke-nodepool: prometheus-{{ .PR_NUMBER }} + node-name: prometheus-{{ .PR_NUMBER }} containers: - image: quay.io/prometheus/node-exporter:v0.16.0 args: @@ -165,7 +165,7 @@ spec: hostNetwork: true hostPID: true nodeSelector: - cloud.google.com/gke-nodepool: nodes-{{ .PR_NUMBER }} + node-name: nodes-{{ .PR_NUMBER }} containers: - image: quay.io/prometheus/node-exporter:v0.16.0 args: diff --git a/prombench/manifests/prombench/benchmark/6_loadgen.yaml b/prombench/manifests/prombench/benchmark/6_loadgen.yaml index 1398e63a8..d7048334c 100644 --- a/prombench/manifests/prombench/benchmark/6_loadgen.yaml +++ b/prombench/manifests/prombench/benchmark/6_loadgen.yaml @@ -87,7 +87,7 @@ spec: configMap: name: fake-webserver-config-for-scaler nodeSelector: - cloud.google.com/gke-nodepool: nodes-{{ .PR_NUMBER }} + node-name: nodes-{{ .PR_NUMBER }} isolation: none --- apiVersion: apps/v1 @@ -127,7 +127,7 @@ spec: configMap: name: prometheus-loadgen nodeSelector: - cloud.google.com/gke-nodepool: nodes-{{ .PR_NUMBER }} + node-name: nodes-{{ .PR_NUMBER }} isolation: none --- apiVersion: v1 diff --git a/prombench/manifests/prombench/nodepools.yaml b/prombench/manifests/prombench/nodepools.yaml index 72cf51fc7..ec84f9b19 100644 --- a/prombench/manifests/prombench/nodepools.yaml +++ b/prombench/manifests/prombench/nodepools.yaml @@ -13,6 +13,7 @@ cluster: localssdcount: 1 #SSD is used to give fast-lookup to Prometheus servers being benchmarked labels: isolation: prometheus + node-name: prometheus-{{ .PR_NUMBER }} - name: nodes-{{ .PR_NUMBER }} initialnodecount: 1 config: @@ -21,4 +22,5 @@ cluster: disksizegb: 100 localssdcount: 0 #use standard HDD. SSD not needed for fake-webservers. labels: - isolation: none \ No newline at end of file + isolation: none + node-name: nodes-{{ .PR_NUMBER }} \ No newline at end of file diff --git a/scripts/genflagdocs.sh b/scripts/genflagdocs.sh index 5b2de4c34..915f4749e 100755 --- a/scripts/genflagdocs.sh +++ b/scripts/genflagdocs.sh @@ -24,7 +24,7 @@ if [[ "${CHECK}" == "check" ]]; then RESULT=$? if [[ "$RESULT" != "0" ]]; then cat << EOF -Docs have discrepancies, do 'make docs' and commit changes: +Docs have discrepancies, do 'make build docs' and commit changes: ${DIFF} EOF diff --git a/tools/commentMonitor/README.md b/tools/commentMonitor/README.md index e9b4850f9..5b5ee36ce 100644 --- a/tools/commentMonitor/README.md +++ b/tools/commentMonitor/README.md @@ -4,7 +4,6 @@ Simple webhook server to parse GitHub comments and take actions based on the com Currently it only works with [`issue_comment` event](https://developer.github.com/v3/activity/events/types/#issuecommentevent) coming from PRs. ### Environment Variables: -- `LABEL_NAME`: If set, will add the label to the PR. - `GITHUB_TOKEN` : GitHub oauth token used for posting comments and settings the label. - Any other environment variable used in any of the comment templates in `eventmap.yml`. @@ -14,11 +13,12 @@ Running commentMonitor requires the eventmap file which is specified by the `--e The `regex_string`, `event_type` and `comment_template` can be specified in the `eventmap.yml` file. Example content of the `eventmap.yml` file: -``` +```yaml - event_type: prombench_stop regex_string: (?mi)^/prombench\s+cancel\s*$ comment_template: | Benchmark cancel is in progress. + label: prombench ``` If a GitHub comment matches with `regex_string`, then commentMonitor will trigger a [`repository_dispatch`](https://developer.github.com/v3/repos/#create-a-repository-dispatch-event) with the event type `event_type` and then post a comment to the issue with `comment_template`. The extracted out arguments will be passed to the [`client_payload`](https://developer.github.com/v3/repos/#example-5) of the `repository_dispatch` event. @@ -54,6 +54,8 @@ Flags: --eventmap="./eventmap.yml" Filepath to eventmap file. --port="8080" port number to run webhook in. + --command-prefix=COMMAND-PREFIX ... + Specify allowed command prefix. Eg."/prombench" ``` ### Building Docker Image diff --git a/tools/commentMonitor/client.go b/tools/commentMonitor/client.go index be4a32b24..947abb995 100644 --- a/tools/commentMonitor/client.go +++ b/tools/commentMonitor/client.go @@ -15,7 +15,6 @@ package main import ( "bytes" - "context" "fmt" "log" "os" @@ -32,16 +31,18 @@ type commentMonitorClient struct { eventMap webhookEventMaps eventType string commentTemplate string + label string } // Set eventType and commentTemplate if -// regexString is validated against provided commentBody. -func (c *commentMonitorClient) validateRegex() bool { +// regexString is validated against provided command. +func (c *commentMonitorClient) validateRegex(command string) bool { for _, e := range c.eventMap { c.regex = regexp.MustCompile(e.RegexString) - if c.regex.MatchString(c.ghClient.commentBody) { + if c.regex.MatchString(command) { c.commentTemplate = e.CommentTemplate c.eventType = e.EventType + c.label = e.Label log.Println("comment validation successful") return true } @@ -50,7 +51,7 @@ func (c *commentMonitorClient) validateRegex() bool { } // Verify if user is allowed to perform activity. -func (c commentMonitorClient) verifyUser(ctx context.Context, verifyUserDisabled bool) error { +func (c commentMonitorClient) verifyUser(verifyUserDisabled bool) error { if !verifyUserDisabled { var allowed bool allowedAssociations := []string{"COLLABORATOR", "MEMBER", "OWNER"} @@ -61,7 +62,7 @@ func (c commentMonitorClient) verifyUser(ctx context.Context, verifyUserDisabled } if !allowed { b := fmt.Sprintf("@%s is not a org member nor a collaborator and cannot execute benchmarks.", c.ghClient.author) - if err := c.ghClient.postComment(ctx, b); err != nil { + if err := c.ghClient.postComment(b); err != nil { return fmt.Errorf("%v : couldn't post comment", err) } return fmt.Errorf("author is not a member or collaborator") @@ -72,27 +73,28 @@ func (c commentMonitorClient) verifyUser(ctx context.Context, verifyUserDisabled } // Extract args if regexString provided. -func (c *commentMonitorClient) extractArgs(ctx context.Context) error { +func (c *commentMonitorClient) extractArgs(command string) error { var err error if c.regex != nil { - // Add comment arguments. - commentArgs := c.regex.FindStringSubmatch(c.ghClient.commentBody)[1:] - commentArgsNames := c.regex.SubexpNames()[1:] - for i, argName := range commentArgsNames { + // Add command arguments. + commandArgs := c.regex.FindStringSubmatch(command)[1:] + commandArgsNames := c.regex.SubexpNames()[1:] + for i, argName := range commandArgsNames { if argName == "" { return fmt.Errorf("using named groups is mandatory") } - c.allArgs[argName] = commentArgs[i] + c.allArgs[argName] = commandArgs[i] } // Add non-comment arguments if any. c.allArgs["PR_NUMBER"] = strconv.Itoa(c.ghClient.pr) - c.allArgs["LAST_COMMIT_SHA"], err = c.ghClient.getLastCommitSHA(ctx) + c.allArgs["LAST_COMMIT_SHA"], err = c.ghClient.getLastCommitSHA() if err != nil { return fmt.Errorf("%v: could not fetch SHA", err) } - err = c.ghClient.createRepositoryDispatch(ctx, c.eventType, c.allArgs) + // TODO (geekodour) : We could run this in a seperate method. + err = c.ghClient.createRepositoryDispatch(c.eventType, c.allArgs) if err != nil { return fmt.Errorf("%v: could not create repository_dispatch event", err) } @@ -100,10 +102,9 @@ func (c *commentMonitorClient) extractArgs(ctx context.Context) error { return nil } -// Set label to Github pr if LABEL_NAME is set. -func (c commentMonitorClient) postLabel(ctx context.Context) error { - if os.Getenv("LABEL_NAME") != "" { - if err := c.ghClient.createLabel(ctx, os.Getenv("LABEL_NAME")); err != nil { +func (c commentMonitorClient) postLabel() error { + if c.label != "" { + if err := c.ghClient.createLabel(c.label); err != nil { return fmt.Errorf("%v : couldn't set label", err) } log.Println("label successfully set") @@ -111,7 +112,7 @@ func (c commentMonitorClient) postLabel(ctx context.Context) error { return nil } -func (c commentMonitorClient) generateAndPostComment(ctx context.Context) error { +func (c commentMonitorClient) generateAndPostComment() error { if c.commentTemplate != "" { // Add all env vars to allArgs. for _, e := range os.Environ() { @@ -125,7 +126,7 @@ func (c commentMonitorClient) generateAndPostComment(ctx context.Context) error return err } // Post the comment. - if err := c.ghClient.postComment(ctx, buf.String()); err != nil { + if err := c.ghClient.postComment(buf.String()); err != nil { return fmt.Errorf("%v : couldn't post generated comment", err) } log.Println("comment successfully posted") diff --git a/tools/commentMonitor/ghclient.go b/tools/commentMonitor/ghclient.go index f3ace62c5..762b1cef4 100644 --- a/tools/commentMonitor/ghclient.go +++ b/tools/commentMonitor/ghclient.go @@ -18,8 +18,10 @@ import ( "encoding/json" "fmt" "log" + "os" "github.com/google/go-github/v29/github" + "golang.org/x/oauth2" ) type githubClient struct { @@ -30,31 +32,51 @@ type githubClient struct { author string commentBody string authorAssociation string + ctx context.Context } -func (c githubClient) postComment(ctx context.Context, commentBody string) error { +func newGithubClient(ctx context.Context, e *github.IssueCommentEvent) (*githubClient, error) { + ghToken := os.Getenv("GITHUB_TOKEN") + if ghToken == "" { + return nil, fmt.Errorf("env var missing") + } + ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: ghToken}) + tc := oauth2.NewClient(ctx, ts) + return &githubClient{ + clt: github.NewClient(tc), + owner: *e.GetRepo().Owner.Login, + repo: *e.GetRepo().Name, + pr: *e.GetIssue().Number, + author: *e.Sender.Login, + authorAssociation: *e.GetComment().AuthorAssociation, + commentBody: *e.GetComment().Body, + ctx: ctx, + }, nil +} + +func (c githubClient) postComment(commentBody string) error { issueComment := &github.IssueComment{Body: github.String(commentBody)} - _, _, err := c.clt.Issues.CreateComment(ctx, c.owner, c.repo, c.pr, issueComment) + _, _, err := c.clt.Issues.CreateComment(c.ctx, c.owner, c.repo, c.pr, issueComment) return err } -func (c githubClient) createLabel(ctx context.Context, labelName string) error { +func (c githubClient) createLabel(labelName string) error { benchmarkLabel := []string{labelName} - _, _, err := c.clt.Issues.AddLabelsToIssue(ctx, c.owner, c.repo, c.pr, benchmarkLabel) + _, _, err := c.clt.Issues.AddLabelsToIssue(c.ctx, c.owner, c.repo, c.pr, benchmarkLabel) return err } -func (c githubClient) getLastCommitSHA(ctx context.Context) (string, error) { +func (c githubClient) getLastCommitSHA() (string, error) { // https://developer.github.com/v3/pulls/#list-commits-on-a-pull-request listops := &github.ListOptions{Page: 1, PerPage: 250} - l, _, err := c.clt.PullRequests.ListCommits(ctx, c.owner, c.repo, c.pr, listops) + l, _, err := c.clt.PullRequests.ListCommits(c.ctx, c.owner, c.repo, c.pr, listops) if len(l) == 0 { return "", fmt.Errorf("pr does not have a commit") } return l[len(l)-1].GetSHA(), err } -func (c githubClient) createRepositoryDispatch(ctx context.Context, eventType string, clientPayload map[string]string) error { +func (c githubClient) createRepositoryDispatch(eventType string, clientPayload map[string]string) error { allArgs, err := json.Marshal(clientPayload) if err != nil { return fmt.Errorf("%v: could not encode client payload", err) @@ -67,6 +89,6 @@ func (c githubClient) createRepositoryDispatch(ctx context.Context, eventType st } log.Printf("creating repository_dispatch with payload: %v", string(allArgs)) - _, _, err = c.clt.Repositories.Dispatch(ctx, c.owner, c.repo, rd) + _, _, err = c.clt.Repositories.Dispatch(c.ctx, c.owner, c.repo, rd) return err } diff --git a/tools/commentMonitor/main.go b/tools/commentMonitor/main.go index bcd77e5fb..5cc4927b2 100644 --- a/tools/commentMonitor/main.go +++ b/tools/commentMonitor/main.go @@ -21,9 +21,9 @@ import ( "net/http" "os" "path/filepath" + "strings" "github.com/google/go-github/v29/github" - "golang.org/x/oauth2" "gopkg.in/alecthomas/kingpin.v2" "gopkg.in/yaml.v2" ) @@ -32,6 +32,7 @@ type commentMonitorConfig struct { verifyUserDisabled bool eventMapFilePath string whSecretFilePath string + commandPrefixes []string whSecret []byte eventMap webhookEventMaps port string @@ -42,6 +43,7 @@ type webhookEventMap struct { EventType string `yaml:"event_type"` CommentTemplate string `yaml:"comment_template"` RegexString string `yaml:"regex_string"` + Label string `yaml:"label"` } type webhookEventMaps []webhookEventMap @@ -63,6 +65,8 @@ func main() { app.Flag("port", "port number to run webhook in."). Default("8080"). StringVar(&cmConfig.port) + app.Flag("command-prefix", `Specify allowed command prefix. Eg."/prombench" `). + StringsVar(&cmConfig.commandPrefixes) kingpin.MustParse(app.Parse(os.Args[1:])) mux := http.NewServeMux() @@ -71,24 +75,6 @@ func main() { log.Fatal(http.ListenAndServe(fmt.Sprintf(":%v", cmConfig.port), mux)) } -func newGithubClient(ctx context.Context, e *github.IssueCommentEvent) (*githubClient, error) { - ghToken := os.Getenv("GITHUB_TOKEN") - if ghToken == "" { - return nil, fmt.Errorf("env var missing") - } - ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: ghToken}) - tc := oauth2.NewClient(ctx, ts) - return &githubClient{ - clt: github.NewClient(tc), - owner: *e.GetRepo().Owner.Login, - repo: *e.GetRepo().Name, - pr: *e.GetIssue().Number, - author: *e.Sender.Login, - authorAssociation: *e.GetComment().AuthorAssociation, - commentBody: *e.GetComment().Body, - }, nil -} - func (c *commentMonitorConfig) loadConfig() error { // Get eventmap file. data, err := ioutil.ReadFile(c.eventMapFilePath) @@ -110,6 +96,24 @@ func (c *commentMonitorConfig) loadConfig() error { return nil } +func extractCommand(s string) string { + s = strings.TrimLeft(s, "\r\n\t ") + if i := strings.Index(s, "\n"); i != -1 { + s = s[:i] + } + s = strings.TrimRight(s, "\r\n\t ") + return s +} + +func checkCommandPrefix(command string, prefixes []string) bool { + for _, p := range prefixes { + if strings.HasPrefix(command, p) { + return true + } + } + return false +} + func (c *commentMonitorConfig) webhookExtract(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() @@ -155,15 +159,27 @@ func (c *commentMonitorConfig) webhookExtract(w http.ResponseWriter, r *http.Req return } - // Validate regex. - if !cmClient.validateRegex() { - //log.Println(err) // Don't log on failure. + // Strip whitespace. + command := extractCommand(cmClient.ghClient.commentBody) + + // test-infra command check. + if !checkCommandPrefix(command, c.commandPrefixes) { http.Error(w, "comment validation failed", http.StatusOK) return } + // Validate regex. + if !cmClient.validateRegex(command) { + log.Println("invalid command syntax: ", command) + if err := cmClient.ghClient.postComment("command syntax invalid"); err != nil { + log.Printf("%v : couldn't post comment", err) + } + http.Error(w, "command syntax invalid", http.StatusBadRequest) + return + } + // Verify user. - err = cmClient.verifyUser(ctx, c.verifyUserDisabled) + err = cmClient.verifyUser(c.verifyUserDisabled) if err != nil { log.Println(err) http.Error(w, "user not allowed to run command", http.StatusForbidden) @@ -171,7 +187,7 @@ func (c *commentMonitorConfig) webhookExtract(w http.ResponseWriter, r *http.Req } // Extract args. - err = cmClient.extractArgs(ctx) + err = cmClient.extractArgs(command) if err != nil { log.Println(err) http.Error(w, "could not extract arguments", http.StatusBadRequest) @@ -179,7 +195,7 @@ func (c *commentMonitorConfig) webhookExtract(w http.ResponseWriter, r *http.Req } // Post generated comment to GitHub pr. - err = cmClient.generateAndPostComment(ctx) + err = cmClient.generateAndPostComment() if err != nil { log.Println(err) http.Error(w, "could not post comment to GitHub", http.StatusBadRequest) @@ -187,7 +203,7 @@ func (c *commentMonitorConfig) webhookExtract(w http.ResponseWriter, r *http.Req } // Set label to GitHub pr. - err = cmClient.postLabel(ctx) + err = cmClient.postLabel() if err != nil { log.Println(err) http.Error(w, "could not set label to GitHub", http.StatusBadRequest) diff --git a/tools/commentMonitor/main_test.go b/tools/commentMonitor/main_test.go new file mode 100644 index 000000000..936b8bcfb --- /dev/null +++ b/tools/commentMonitor/main_test.go @@ -0,0 +1,55 @@ +// Copyright 2020 The Prometheus 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 main + +import "testing" + +func TestExtractCommand(t *testing.T) { + testCases := []struct { + commentBody string + command string + }{ + {"\r\n/funcbench master\t", "/funcbench master"}, + {"\r\n/funcbench master\n", "/funcbench master"}, + {"\r\n/funcbench master\r\n", "/funcbench master"}, + {"/prombench master\r\n", "/prombench master"}, + {"/funcbench master .*\t\r\nSomething", "/funcbench master .*"}, + {"command without forwardslash", "command without forwardslash"}, + } + for _, tc := range testCases { + command := extractCommand(tc.commentBody) + if command != tc.command { + t.Errorf("want %s, got %s", tc.command, command) + } + } +} +func TestCheckCommandPrefix(t *testing.T) { + prefixes := []string{"/funcbench", "/prombench", "/somebench"} + testCases := []struct { + command string + valid bool + }{ + {"/funcbench master", true}, + {"/somebench master", true}, + {"/querybench master", false}, + {"prombench master", false}, + } + for _, tc := range testCases { + t.Run(tc.command, func(t *testing.T) { + if checkCommandPrefix(tc.command, prefixes) != tc.valid { + t.Errorf("want %v, got %v", tc.valid, !tc.valid) + } + }) + } +}