Change in vdsm[master]: Added standard build skeleton

David Caro has uploaded a new change for review.

Change subject: Added standard build skeleton
......................................................................

Added standard build skeleton

This will replace all/any other vdsm jobs being run now.

Change-Id: I24b446d3ae38c944b5a28d56ed0965d0f2edc206
Signed-off-by: David Caro 
---
A automation/build-artifacts.sh
A automation/check-merged.sh
A automation/check-patch.sh
3 files changed, 9 insertions(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/41928/1

diff --git a/automation/build-artifacts.sh b/automation/build-artifacts.sh
new file mode 100644
index 0000000..231f390
--- /dev/null
+++ b/automation/build-artifacts.sh
@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+echo "TODO"
diff --git a/automation/check-merged.sh b/automation/check-merged.sh
new file mode 100644
index 0000000..231f390
--- /dev/null
+++ b/automation/check-merged.sh
@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+echo "TODO"
diff --git a/automation/check-patch.sh b/automation/check-patch.sh
new file mode 100644
index 0000000..231f390
--- /dev/null
+++ b/automation/check-patch.sh
@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+echo "TODO"

[e-mail] has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 1:

Please add the required make calls so this keep the current behavior.

Then we can refine the behavior by modifying the scripts.

Can you add a comment in each file, what is the purpuse of the script, and when it is called? This comment should be useful for any project using these scripts.

David Caro has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 1:

it requires quite a lot more work to keep the current behavior, but that is on your hands to run whatever you want to run as tests, I'll add a small description to each script

[e-mail] has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 2:

(3 comments)

https://gerrit.ovirt.org/#/c/41928/2/automation/build-artifacts.sh
File automation/build-artifacts.sh:

Line 1: #!/usr/bin/env bash
Line 2: #
Line 3: # Build the artifacts that are going to be archived, they will go under
Line 4: # $repo_root/exported-artifacts directory
What is repo_root? is this an environment variable?
Line 5: #
Line 6:

https://gerrit.ovirt.org/#/c/41928/2/automation/check-merged.sh
File automation/check-merged.sh:

Line 2: #
Line 3: # Executed on each merge, should be considered as the gate for the change to
Line 4: # get merged, if not working no changes will get merged
Line 5: 
Line 6: echo "TODO"
We need at least the implementation that keeps the current behavior.

https://gerrit.ovirt.org/#/c/41928/2/automation/check-patch.sh
File automation/check-patch.sh:

Line 2: #
Line 3: # Run on each patch to gerrit, should be faster than check-meged and require
Line 4: # less resources but thorough enough to provide relevant feedback
Line 5: 
Line 6: echo "TODO"
We need at least the implementation that keeps the current behavior.

David Caro has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 2:

(3 comments)

I think you misunderstood my part here, I'm not going to tell you what to test and how to test it, I'm not going to write the tests, I can give you support and a platform to do it, but it's up to you (project devels) to test whatever you want however you want to test it and maintain those tests.

You decide if you want to use pytest, make, autotools, doctest, pep8, pylint, bats or anything else, and you decide what is required to pass to get a patch merged, mainly because you are the ones that know how the product works and have the knowledge to decide which tool/methodology is more appropriate for the project.

https://gerrit.ovirt.org/#/c/41928/2/automation/build-artifacts.sh
File automation/build-artifacts.sh:

Line 1: #!/usr/bin/env bash
Line 2: #
Line 3: # Build the artifacts that are going to be archived, they will go under
Line 4: # $repo_root/exported-artifacts directory
> What is repo_root? is this an environment variable?

no, it's the git repository root directory, when this script runs it will be $PWD
Line 5: #
Line 6:

https://gerrit.ovirt.org/#/c/41928/2/automation/check-merged.sh
File automation/check-merged.sh:

Line 2: #
Line 3: # Executed on each merge, should be considered as the gate for the change to
Line 4: # get merged, if not working no changes will get merged
Line 5: 
Line 6: echo "TODO"
> We need at least the implementation that keeps the current behavior.

which test/tests you want to keep it's behavior from? there are quite a few with totally different configuration and requirements

https://gerrit.ovirt.org/#/c/41928/2/automation/check-patch.sh
File automation/check-patch.sh:

Line 2: #
Line 3: # Run on each patch to gerrit, should be faster than check-meged and require
Line 4: # less resources but thorough enough to provide relevant feedback
Line 5: 
Line 6: echo "TODO"
> We need at least the implementation that keeps the current behavior.

idem as the previous shell script

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 2:

David, can you point me to the current code running vdsm tests for patch and for merge?

Do you want this to replace the multiple jobs such as pep8, unittests, etc.?

The simplest implementation would be:

check-patch:
make check

check-merge:
make check

This will run pep8, python3, pyflakes, and the unittests which are not marked as slowtests or stresstest.

For build-artifacts, I don't think we want to implement this, since this build requirements are already in the spec. We cannot maintain duplicate of this info in another file.

What I would like do is to run yum-builddep ./vdsm.spec before the build instead.

What do you think?

David Caro has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 2:

You can search in jenkins for:

vdsm_master-libgfapi_create-rpms
  vdsm_master_install-rpm-sanity
  vdsm_master_pep8
  vdsm_master_unit-tests
  vdsm_master_pep8

The ones that end with _created or _gerrit run on each patch, the ones with _merged run on merged patches only. The ones that don't say, are out of the system so you should check directly the config

David Caro has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 2:

replace the last pep8 for vdsm_master_create-rpms

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 2:

Davic, can you setup a job running these scripts?

I want to upload the next version with initial implementation of the scripts.

[e-mail] has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 3:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/41928/3/automation/check-patch.req
File automation/check-patch.req:

Line 1: yum
You can replace this with one call to yum builddep in check-patch.sh

Who wil maintain this list, and update it when the spec changes? Why should we pay for maintaining the same info twice?
Line 2: sudo
Line 3: dhclient
Line 4: autoreconf
Line 5: gettext-devel

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 3:

(5 comments)

Nice!

https://gerrit.ovirt.org/#/c/41928/3/automation/check-patch.sh
File automation/check-patch.sh:

Line 1: #!/bin/bash -e
Line 2: #
Line 3: # Run on each patch to gerrit, should be faster than check-meged and require
Line 4: # less resources but thorough enough to provide relevant feedback
David: we need more specific contract with the ci

- how do you signal success?
- how do you signal failure?
- how do you signla error?

I think the simplest way would be to use the process return value:

- 0 - success: +1 patch
- 1 - some tests failed: -1 patch
- 2 - error: no change in patch
Line 5: 
Line 6: echo "Hello - ifra functional tests"
Line 7: 
Line 8: ./autogen.sh --system; make rpm NOSE_EXCLUDE=.*

Line 4: # less resources but thorough enough to provide relevant feedback
Line 5: 
Line 6: echo "Hello - ifra functional tests"
Line 7: 
Line 8: ./autogen.sh --system; make rpm NOSE_EXCLUDE=.*
You do everything here except running the tests, which is the only thing we should do here :-)
Line 9: yum install ~/rpmbuild/RPMS/noarch/vdsm*
Line 10: 
Line 11: 
Line 12: vdsm-tool configure --force

Line 5: 
Line 6: echo "Hello - ifra functional tests"
Line 7: 
Line 8: ./autogen.sh --system; make rpm NOSE_EXCLUDE=.*
Line 9: yum install ~/rpmbuild/RPMS/noarch/vdsm*
Errors are ignored - we should check and handle error for each command.
Line 10: 
Line 11: 
Line 12: vdsm-tool configure --force
Line 13: service vdsmd start

Line 9: yum install ~/rpmbuild/RPMS/noarch/vdsm*
Line 10: 
Line 11: 
Line 12: vdsm-tool configure --force
Line 13: service vdsmd start
Why do we need to buid, create rpms, install, configure and start vdsm for each version in each patch?

This is waste of resources. I think we should only run the tests (make check) for each patch, do the rest during merge.
Line 14: 
Line 15: 
Line 16: # copy logs
Line 17: mkdir exported-artifacts

Line 14: 
Line 15: 
Line 16: # copy logs
Line 17: mkdir exported-artifacts
Line 18: cp /var/log/vdsm/* exported-artifacts
Failure here must not fail the patch in gerrit

Yaniv Bronhaim has posted comments on this change.

Change subject: Added standard build skeleton
......................................................................

Patch Set 3:

(3 comments)

https://gerrit.ovirt.org/#/c/41928/3/automation/check-patch.req
File automation/check-patch.req:

Line 1: yum
> You can replace this with one call to yum builddep in check-patch.sh

because this is how ci scripts currently work and I don't plan to parse the spec but align to theirs instructions
Line 2: sudo
Line 3: dhclient
Line 4: autoreconf
Line 5: gettext-devel

https://gerrit.ovirt.org/#/c/41928/3/automation/check-patch.sh
File automation/check-patch.sh:

Line 1: #!/bin/bash -e
Line 2: #
Line 3: # Run on each patch to gerrit, should be faster than check-meged and require
Line 4: # less resources but thorough enough to provide relevant feedback
> David: we need more specific contract with the ci

not related to the patch
Line 5: 
Line 6: echo "Hello - ifra functional tests"
Line 7: 
Line 8: ./autogen.sh --system; make rpm NOSE_EXCLUDE=.*

Line 4: # less resources but thorough enough to provide relevant feedback
Line 5: 
Line 6: echo "Hello - ifra functional tests"
Line 7: 
Line 8: ./autogen.sh --system; make rpm NOSE_EXCLUDE=.*
> You do everything here except running the tests, which is the only thing we

wip , I know what I do and I need to update the commit message. the reason for this patch is to introduce functional tests job. I plan to run vdsmd and trigger tests/run_tests.sh functional/* -> those tests will depend on specific environment state and this job should create the needed state
Line 9: yum install ~/rpmbuild/RPMS/noarch/vdsm*
Line 10: 
Line 11: 
Line 12: vdsm-tool configure --force

Change in vdsm[master]: [WIP] Automation scripts for running vdsm functional tests job

[e-mail] has posted comments on this change.

Change subject: [WIP] Automation scripts for running vdsm functional tests job
......................................................................

Patch Set 4:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Change in vdsm[master]: [WIP] Automation scripts for running vdsm functional tests job

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: [WIP] Automation scripts for running vdsm functional tests job
......................................................................

Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/41928/3/automation/check-patch.req
File automation/check-patch.req:

Line 1: yum
> because this is how ci scripts currently work and I don't plan to parse the

Who will maintain the duplicate requirements?

Each time we add something the spec we must add it also here.
Line 2: sudo
Line 3: dhclient
Line 4: autoreconf
Line 5: gettext-devel

Change in vdsm[master]: [WIP] Automation scripts for running vdsm functional tests job

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: [WIP] Automation scripts for running vdsm functional tests job
......................................................................

Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/41928/3/automation/check-patch.sh
File automation/check-patch.sh:

Line 4: # less resources but thorough enough to provide relevant feedback
Line 5: 
Line 6: echo "Hello - ifra functional tests"
Line 7: 
Line 8: ./autogen.sh --system; make rpm NOSE_EXCLUDE=.*
> wip , I know what I do and I need to update the commit message. the reason

Makes sense, but not for each patch - this belongs to check-merge.sh.

Check done for each patch should be fast and reliable. Functional tests are too slow and fragile to run automatically for each test.
Line 9: yum install ~/rpmbuild/RPMS/noarch/vdsm*
Line 10: 
Line 11: 
Line 12: vdsm-tool configure --force

Change in vdsm[master]: [WIP] Automation scripts for running vdsm functional tests job

David Caro has posted comments on this change.

Change subject: [WIP] Automation scripts for running vdsm functional tests job
......................................................................

Patch Set 3:

(3 comments)

https://gerrit.ovirt.org/#/c/41928/3/automation/check-patch.req
File automation/check-patch.req:

Line 1: yum
> Who will maintain the duplicate requirements?

Yep, if it's properly defined in the spec, you can build the rpm and install it, or similar to get the deps, but as long as it's not handled you will have to add it here too.

Until now we did it in the jenkins jobs directly, and that made patches break each time there was a new dep
Line 2: sudo
Line 3: dhclient
Line 4: autoreconf
Line 5: gettext-devel

https://gerrit.ovirt.org/#/c/41928/3/automation/check-patch.sh
File automation/check-patch.sh:

Line 1: #!/bin/bash -e
Line 2: #
Line 3: # Run on each patch to gerrit, should be faster than check-meged and require
Line 4: # less resources but thorough enough to provide relevant feedback
> David: we need more specific contract with the ci

Nop, there's only 0 or 1, if you don't want to fail the pass, return 0, if you do, return != 0. No need for a third state
Line 5: 
Line 6: echo "Hello - ifra functional tests"
Line 7: 
Line 8: ./autogen.sh --system; make rpm NOSE_EXCLUDE=.*

Line 5: 
Line 6: echo "Hello - ifra functional tests"
Line 7: 
Line 8: ./autogen.sh --system; make rpm NOSE_EXCLUDE=.*
Line 9: yum install ~/rpmbuild/RPMS/noarch/vdsm*
> Errors are ignored - we should check and handle error for each command.

set -e breaks execution if there are any failed commands
Line 10: 
Line 11: 
Line 12: vdsm-tool configure --force
Line 13: service vdsmd start

Change in vdsm[master]: [WIP] Automation scripts for running vdsm functional tests job

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: [WIP] Automation scripts for running vdsm functional tests job
......................................................................

Patch Set 4: Code-Review-1

I believe that this has been fixed in https://gerrit.ovirt.org/46135 and other patches.

Please abandon.

Change in vdsm[master]: [WIP] Automation scripts for running vdsm functional tests job

Eyal Edri has abandoned this change.

Change subject: [WIP] Automation scripts for running vdsm functional tests job
......................................................................

Abandoned

already implemented. see danken comment.

Change in vdsm[master]: [WIP] Automation scripts for running vdsm functional tests job

[e-mail] has posted comments on this change.

Change subject: [WIP] Automation scripts for running vdsm functional tests job
......................................................................

Patch Set 4:

* Update tracker::IGNORE, no Bug-Url found