Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ci: improve linting speed #22103

Merged
merged 11 commits into from
Oct 28, 2020
Merged

ci: improve linting speed #22103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
be932d7
ci: improve linting speed
kuisathaverat Oct 22, 2020
362b2e5
fix: use multiple agents for the matrix
kuisathaverat Oct 22, 2020
59d0018
fix: add python-env
kuisathaverat Oct 22, 2020
c682f3b
fix: set make folder parameter
kuisathaverat Oct 22, 2020
284add9
feat: move k8s check and dev-tools to their own stages
kuisathaverat Oct 22, 2020
36d600a
fix: lint in a single stage is faster
kuisathaverat Oct 22, 2020
4faca89
fix: suggested changes
kuisathaverat Oct 26, 2020
b28b754
chore:change triggers
kuisathaverat Oct 26, 2020
0b9e387
Merge branch 'master' of github.com:elastic/beats into speed_up_lint
kuisathaverat Oct 27, 2020
8e33fce
Merge branch 'master' of github.com:elastic/beats into speed_up_lint
kuisathaverat Oct 27, 2020
81aebd2
chore: update for packetbeat
kuisathaverat Oct 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,12 @@ pipeline {
GOFLAGS = '-mod=readonly'
}
steps {
withGithubNotify(context: 'Lint') {
withBeatsEnv(archive: false, id: 'lint') {
withGithubNotify(context: "Lint") {
withBeatsEnv(archive: false, id: "lint") {
dumpVariables()
cmd(label: 'make check', script: 'make check')
cmd(label: "make check-python", script: "make check-python")
cmd(label: "make check-go", script: "make check-go")
cmd(label: "Check for changes", script: "make check-no-changes")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will we decide what goes in this linting and what goes in the project-specific linting? For example this linting is not checking the license headers now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved the checks that are in a foreach in the main Makefile, so I think that every check that it is implemented on the beats should run only on beans. Indeed we should try to add check only to the beans to allow parallelize the task.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried make -j <n>?

Copy link
Contributor Author

@kuisathaverat kuisathaverat Oct 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the VMs are not really powerful machines that do not have too many cores and are not dedicated hardware, so this could decrease a little the time(maybe 5-10min or less) of the task but far away from this approach that uses a VM for each stage (40min).

}
}
}
Expand Down Expand Up @@ -490,7 +492,7 @@ def terraformApply(String directory) {
}

/**
* Tear down the terraform environments, by looking for all terraform states in directory
* Tear down the terraform environments, by looking for all terraform states in directory
* then it runs terraform destroy for each one.
* It uses terraform states previously stashed by startCloudTestEnv.
*/
Expand Down
3 changes: 2 additions & 1 deletion Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ projects:
- "x-pack/libbeat"
- "x-pack/metricbeat"
- "x-pack/winlogbeat"
- "dev-tools"
##- "x-pack/heartbeat" It's not yet in the 1.0 pipeline.
##- "x-pack/journalbeat" It's not yet in the 1.0 pipeline.
##- "x-pack/packetbeat" It's not yet in the 1.0 pipeline.
Expand Down Expand Up @@ -45,4 +46,4 @@ disabled:
labels: ## Skip the GitHub Pull Request builds if any of the given GitHub labels match with the assigned labels in the PR.
- skip-ci
## TODO: This will allow to configure what to do based on the PR configuration
draft: true ## Skip the GitHub Pull Request builds with Draft PRs.
draft: true ## Skip the GitHub Pull Request builds with Draft PRs.
25 changes: 20 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,20 +94,35 @@ clean: mage

## check : TBD.
.PHONY: check
check: python-env
check:
@$(foreach var,$(PROJECTS) dev-tools $(PROJECTS_XPACK_MAGE),$(MAKE) -C $(var) check || exit 1;)
@$(FIND) -name *.py -name *.py -not -path "*/build/*" -exec $(PYTHON_ENV)/bin/autopep8 -d --max-line-length 120 {} \; | (! grep . -q) || (echo "Code differs from autopep8's style" && false)
@$(FIND) -name *.py -not -path "*/build/*" | xargs $(PYTHON_ENV)/bin/pylint --py3k -E || (echo "Code is not compatible with Python 3" && false)
$(MAKE) check-python
# check if vendor folder does not exists
[ ! -d vendor ]
@# Validate that all updates were committed
# Validate that all updates were committed
@$(MAKE) update
@$(MAKE) check-headers
go mod tidy
@$(MAKE) check-go
@$(MAKE) check-no-changes

## ccheck-go : Check there is no changes in Go modules.
.PHONY: check-go
check-go:
@go mod tidy

## ccheck-no-changes : Check there is no local changes.
.PHONY: check-no-changes
check-no-changes:
@git diff | cat
@git update-index --refresh
@git diff-index --exit-code HEAD --

## check-python : Python Linting.
.PHONY: check-python
check-python: python-env
@$(FIND) -name *.py -name *.py -not -path "*/build/*" -exec $(PYTHON_ENV)/bin/autopep8 -d --max-line-length 120 {} \; | (! grep . -q) || (echo "Code differs from autopep8's style" && false)
@$(FIND) -name *.py -not -path "*/build/*" | xargs $(PYTHON_ENV)/bin/pylint --py3k -E || (echo "Code is not compatible with Python 3" && false)

## check-headers : Check the license headers.
.PHONY: check-headers
check-headers: mage
Expand Down
5 changes: 5 additions & 0 deletions auditbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C auditbeat check;
make -C auditbeat update;
make check-no-changes;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, check calls check-no-changes but it's called explicitly since the scope it's within the whole workspace rather than the auditbeat folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the check target on beats does not call check-no-changes target, the check and update targets can make changes on files so we have to run check-no-changes to ensure everything is OK.

arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
3 changes: 3 additions & 0 deletions deploy/kubernetes/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ when:
branches: true ## for all the branches
changeset: ## when PR contains any of those entries in the changeset
- "^deploy/kubernetes/.*"
- "^libbeat/docs/version.asciidoc"
comments: ## when PR comment contains any of those entries
- "/test deploy/kubernetes"
labels: ## when PR labels matches any of those entries
Expand All @@ -11,5 +12,7 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
lint:
make: "make -C deploy/kubernetes all"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should be executed when libbeat/docs/version.asciidoc changes.

(Long-term we should probably stop commiting these files and move them to docs build)

And I guess we need to check for changes so it detects anything.

Suggested change
make: "make -C deploy/kubernetes all"
make: |
make -C deploy/kubernetes all;
make check-no-changes;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuisathaverat I see you have added the trigger on changes in the version file, thanks!

But the other point in this comment about adding a check is still pending to be addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is https://github.com/elastic/beats/blob/master/deploy/kubernetes/Jenkinsfile.yml#L5

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the other point in this comment about adding a check is still pending to be addressed.

make -C deploy/kubernetes all is not checking anything, I think we also need to run make check-no-changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, we need to runmake check-no-changes also

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add it?

k8sTest:
k8sTest: "v1.18.2,v1.17.2,v1.16.4,v1.15.7,v1.14.10"
16 changes: 16 additions & 0 deletions dev-tools/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
when:
branches: true ## for all the branches
changeset: ## when PR contains any of those entries in the changeset
- "^dev-tools/.*"
kuisathaverat marked this conversation as resolved.
Show resolved Hide resolved
- "^libbeat/scripts/Makefile"
comments: ## when PR comment contains any of those entries
- "/test dev-tools"
labels: ## when PR labels matches any of those entries
- "dev-tools"
parameters: ## when parameter was selected in the UI.
- "dev-tools"
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
lint:
make: "make -C dev-tools check"
5 changes: 5 additions & 0 deletions filebeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. This stage is upercased in some files, and lowercased in others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep I saw it on the GitHub checks

make: |
make -C filebeat check;
make -C filebeat update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
6 changes: 5 additions & 1 deletion heartbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C heartbeat check;
make -C heartbeat update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down Expand Up @@ -78,4 +83,3 @@ stages:
- "windows-10"
branches: true ## for all the branches
tags: true ## for all the tags

5 changes: 5 additions & 0 deletions journalbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C journalbeat check;
make -C journalbeat update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
5 changes: 5 additions & 0 deletions libbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C libbeat check;
make -C libbeat update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
5 changes: 5 additions & 0 deletions metricbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C metricbeat check;
make -C metricbeat update;
make check-no-changes;
unitTest:
mage: "mage build unitTest"
goIntegTest:
Expand Down
5 changes: 5 additions & 0 deletions packetbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C packetbeat check;
make -C packetbeat update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
5 changes: 5 additions & 0 deletions winlogbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C winlogbeat check;
make -C winlogbeat update;
make check-no-changes;
crosscompile:
make: "make -C winlogbeat crosscompile"
windows:
Expand Down
5 changes: 5 additions & 0 deletions x-pack/auditbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C x-pack/auditbeat check;
make -C x-pack/auditbeat update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
5 changes: 5 additions & 0 deletions x-pack/dockerlogbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C x-pack/dockerlogbeat check;
make -C x-pack/dockerlogbeat update;
make check-no-changes;
build:
mage: "mage build test"
withModule: true ## run the ITs only if the changeset affects a specific module.
5 changes: 5 additions & 0 deletions x-pack/elastic-agent/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C x-pack/elastic-agent check;
make -C x-pack/elastic-agent update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
5 changes: 5 additions & 0 deletions x-pack/filebeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C x-pack/filebeat check;
make -C x-pack/filebeat update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
5 changes: 5 additions & 0 deletions x-pack/functionbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C x-pack/functionbeat check;
make -C x-pack/functionbeat update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
5 changes: 5 additions & 0 deletions x-pack/libbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C x-pack/libbeat check;
make -C x-pack/libbeat update;
make check-no-changes;
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
5 changes: 5 additions & 0 deletions x-pack/metricbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
make: |
make -C x-pack/metricbeat check;
make -C x-pack/metricbeat update;
make check-no-changes;
build:
cloud: "mage build test"
withModule: true ## run the ITs only if the changeset affects a specific module.
Expand Down
5 changes: 5 additions & 0 deletions x-pack/packetbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ when:
tags: true ## for all the tags
platform: "linux && ubuntu-18" ## default label for all the stages
stages:
Lint:
mage: |
mage check;
mage update;
make: "make check-no-changes"
arm:
mage: "mage build unitTest"
platforms: ## override default label in this specific stage.
Expand Down
7 changes: 7 additions & 0 deletions x-pack/winlogbeat/Jenkinsfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ when:
tags: true ## for all the tags
platform: "windows-2019" ## default label for all the stages
stages:
Lint:
make: |
make -C x-pack/winlogbeat check;
make -C x-pack/winlogbeat update;
make check-no-changes;
platforms: ## override default labels in this specific stage.
- "linux && ubuntu-18"
build:
mage: "mage build unitTest"
withModule: true
Expand Down