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: set git user configuration if it is not set #17782

Merged
merged 2 commits into from
Apr 17, 2020
Merged

ci: set git user configuration if it is not set #17782

merged 2 commits into from
Apr 17, 2020

Conversation

kuisathaverat
Copy link
Contributor

What does this PR do?

It checks for git configuration and setup it if there is no configuration.

Why is it important?

It seems needed for some command in the build system.

[2020-04-17T08:47:40.738Z] Metricset test created.
[2020-04-17T08:47:40.738Z] vendor/github.com/elastic/beats/v7/metricbeat/Makefile:84: warning: overriding commands for target `integration-tests'
[2020-04-17T08:47:40.738Z] vendor/github.com/elastic/beats/v7/libbeat/scripts/Makefile:216: warning: ignoring old commands for target `integration-tests'
[2020-04-17T08:48:17.399Z] Generated fields.yml for testmetricbeat to /private/var/lib/jenkins/workspace/Beats_beats-beats-mbp_master/src/beatpath/testmetricbeat/fields.yml
[2020-04-17T08:48:21.385Z] Generated fields.yml for testmetricbeat to /private/var/lib/jenkins/workspace/Beats_beats-beats-mbp_master/src/beatpath/testmetricbeat/build/fields/fields.all.yml
[2020-04-17T08:48:46.899Z] WARNING: You are using pip version 19.2.3, however version 20.0.2 is available.
[2020-04-17T08:48:46.899Z] You should consider upgrading via the 'pip install --upgrade pip' command.
[2020-04-17T08:48:49.915Z] cp: module/*/_meta/kibana/*: No such file or directory
[2020-04-17T08:48:49.915Z] make[1]: [kibana] Error 1 (ignored)
[2020-04-17T08:49:04.967Z] exec: go list -m
[2020-04-17T08:49:35.189Z] 
[2020-04-17T08:49:35.189Z] *** Please tell me who you are.
[2020-04-17T08:49:35.189Z] 
[2020-04-17T08:49:35.189Z] Run
[2020-04-17T08:49:35.189Z] 
[2020-04-17T08:49:35.189Z]   git config --global user.email "you@example.com"
[2020-04-17T08:49:35.189Z]   git config --global user.name "Your Name"
[2020-04-17T08:49:35.189Z] 
[2020-04-17T08:49:35.189Z] to set your account's default identity.
[2020-04-17T08:49:35.189Z] Omit --global to set the identity only in this repository.
[2020-04-17T08:49:35.189Z] 
[2020-04-17T08:49:35.189Z] fatal: unable to auto-detect email address (got 'jenkins@worker-c07ll940dwyl.(none)')
[2020-04-17T08:49:35.189Z] Error: running "git commit -q -m Initial commit, Add generated files" failed with exit code 128
[2020-04-17T08:49:35.189Z] make: *** [prepare-test] Error 128

@kuisathaverat kuisathaverat requested review from jsoriano, urso, andrewkroh and a team April 17, 2020 09:53
@kuisathaverat kuisathaverat self-assigned this Apr 17, 2020
@andrewkroh
Copy link
Member

This appears to be a bug in the generator test. The generator test creates a new git repo. It does run these commands, but too late.

beats/generator/common/Makefile

Lines 15 to 16 in a846029

git config user.email "beats-jenkins@test.com" || exit 1 ; \
git config user.name "beats-jenkins" || exit 1 ; \

These values need to be set after the repo is git init'ed and before git add, but it really can't as written since it's hard-coded to assume that the global git config is setup.

beats/generator/common/beatgen/beatgen.go

Lines 130 to 141 in bbf9d66

mg.Deps(setup.GitInit)
if cfg["type"] == "metricbeat" {
//This is runV because it'll ask for user input, so we need stdout.
err = sh.RunV("make", "create-metricset")
if err != nil {
return errors.Wrap(err, "error running create-metricset")
}
}
mg.Deps(setup.Update)
mg.Deps(setup.GitAdd)

andrewkroh
andrewkroh approved these changes Apr 17, 2020
View reviewed changes
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

I'm good with the workaround. Can you please add something like this so we know to come back and remove it when the Beat Generator is fixed.

// TODO (2020-04-07): This is a work-around to fix the Beat generator tests.
// See https://github.com/elastic/beats/issues/17787.

@andrewkroh andrewkroh mentioned this pull request Apr 17, 2020
Closed
v1v
v1v reviewed Apr 17, 2020
View reviewed changes
Jenkinsfile
@@ -673,6 +673,7 @@ def withBeatsEnv(boolean archive, Closure body) {
sh(label: "Install Go ${GO_VERSION}", script: ".ci/scripts/install-go.sh")
sh(label: "Install docker-compose ${DOCKER_COMPOSE_VERSION}", script: ".ci/scripts/install-docker-compose.sh")
sh(label: "Install Mage", script: "make mage")
setGitConfig()
Copy link
Member

Choose a reason for hiding this comment

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

NIT: do we want to add those details to the shared library? We did add a similar one for the apm, see the step setupAPMGitEmail

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, the intention is to make the workaround and extend setupAPGitEmail to make a new one more for Beats

v1v
v1v approved these changes Apr 17, 2020
View reviewed changes
Copy link
Member

@v1v v1v left a comment

Choose a reason for hiding this comment

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

/LGTM

v1v
v1v reviewed Apr 17, 2020
View reviewed changes
Jenkinsfile Outdated
def setGitConfig(){
sh(label: 'check git config', script: '''
if [ -z "$(git config --get user.email)" ]; then
git config user.email "58790750+beatsmachine@users.noreply.github.com"
Copy link
Member

Choose a reason for hiding this comment

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

The apm*** user got the same prefix, is 58790750+ correct? I don't know if it's a manual configuration done in GitHub or a coincidence that the email is the same one

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 made a C&P, the email is not important, it is a no-reply email :)

kuisathaverat
kuisathaverat commented Apr 17, 2020
View reviewed changes
Jenkinsfile Outdated Show resolved Hide resolved
@kuisathaverat kuisathaverat merged commit b3576ca into elastic:master Apr 17, 2020
@kuisathaverat kuisathaverat deleted the fix-git-error branch April 17, 2020 12:16
@andrewkroh andrewkroh mentioned this pull request Apr 21, 2020
Merged
3 tasks
jsoriano pushed a commit to jsoriano/beats that referenced this pull request Apr 28, 2020
@kuisathaverat @jsoriano
ci: set git user configuration if it is not set (elastic#17782)
ba63fb3 
* ci: set git user configuration if it is not set

* Apply suggestions from code review
@jsoriano jsoriano mentioned this pull request Apr 28, 2020
Merged
jsoriano added a commit that referenced this pull request Apr 28, 2020
@jsoriano @kuisathaverat
@v1v @andrewkroh
Backport some features in Jenkinsfile to 7.x (#18040)
d6ce568 
Backport some features added to Jenkinsfile to 7.x branch:
* Dry run option.
* Docker login.
* Git config for generator tests.
* Filter changes using go list.

These are the cherry-picked changes:
* fix: login into the docker registry (#17620)
* feat: filter changes using go list output (#17397)
* fix: disable workaround on macos (#17750)
* ci: set git user configuration if it is not set (#17782)
* fix: mount Docker credentials (#17798)
* Review dependency patterns collection in Jenkins (#18004)

Co-authored-by: Ivan Fernandez Calvo <kuisathaverat@users.noreply.github.com>
Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com>
Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@v1v v1v v1v approved these changes

@andrewkroh andrewkroh andrewkroh approved these changes

@cachedout cachedout cachedout approved these changes

@jsoriano jsoriano Awaiting requested review from jsoriano

@urso urso Awaiting requested review from urso

Assignees

@kuisathaverat kuisathaverat

Labels
automation bug ci
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kuisathaverat @andrewkroh @cachedout @v1v