[libvirt] [jenkins-ci PATCH 0/2] fix libosinfo rpm build

Pavel Hrdina (2): jobs: rename check_env into job_env jobs: use job_env in all job templates jobs/autotools.yaml | 5 ++++- jobs/defaults.yaml | 2 +- jobs/perl-makemaker.yaml | 3 +++ projects/libosinfo.yaml | 2 +- projects/libvirt.yaml | 2 +- 5 files changed, 10 insertions(+), 4 deletions(-) -- 2.13.6

The check_env was used only for "make check" but there might be some environment variables that needs to be configured for more than one job of the same project. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- jobs/autotools.yaml | 2 +- jobs/defaults.yaml | 2 +- projects/libosinfo.yaml | 2 +- projects/libvirt.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml index 19f62be..9ed5efe 100644 --- a/jobs/autotools.yaml +++ b/jobs/autotools.yaml @@ -120,7 +120,7 @@ builders: - shell: | {make_env} - {check_env} + {job_env} cd build if ! $MAKE -j{smp} check then diff --git a/jobs/defaults.yaml b/jobs/defaults.yaml index 0e628bb..43face2 100644 --- a/jobs/defaults.yaml +++ b/jobs/defaults.yaml @@ -9,6 +9,6 @@ if [ "$unamestr" = 'FreeBSD' ]; then MAKE='gmake' fi - check_env: | + job_env: | smp: 3 spam: ymankad@redhat.com libvirt-ci@redhat.com diff --git a/projects/libosinfo.yaml b/projects/libosinfo.yaml index 9ab2281..a02246f 100644 --- a/projects/libosinfo.yaml +++ b/projects/libosinfo.yaml @@ -11,7 +11,7 @@ - libvirt-freebsd-10 - libvirt-freebsd-11 title: libosinfo - check_env: | + job_env: | export OSINFO_SYSTEM_DIR=$VIRT_PREFIX/share/osinfo jobs: - autotools-build-job: diff --git a/projects/libvirt.yaml b/projects/libvirt.yaml index 5661066..459aa17 100644 --- a/projects/libvirt.yaml +++ b/projects/libvirt.yaml @@ -28,7 +28,7 @@ parent_jobs: 'libvirt-master-build' - autotools-check-job: parent_jobs: 'libvirt-master-syntax-check' - check_env: | + job_env: | export VIR_TEST_EXPENSIVE=1 export VIR_TEST_DEBUG=2 - autotools-rpm-job: -- 2.13.6

This fixes libosinfo RPM build, we need to set "OSINFO_SYSTEM_DIR" for RPM build as well since the spec file runs tests and they need valid osinfo-db. Broken by <359b069f7e4700bee337588eab0a9d83cac94010>. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- jobs/autotools.yaml | 3 +++ jobs/perl-makemaker.yaml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml index 9ed5efe..e41d5ab 100644 --- a/jobs/autotools.yaml +++ b/jobs/autotools.yaml @@ -41,6 +41,7 @@ builders: - shell: | {make_env} + {job_env} mkdir build cd build ../autogen.sh --prefix=$VIRT_PREFIX {autogen_args} @@ -82,6 +83,7 @@ builders: - shell: | {make_env} + {job_env} cd build $MAKE -j{smp} syntax-check publishers: @@ -164,6 +166,7 @@ builders: - shell: | {make_env} + {job_env} cd build sed -i -e 's/BuildRequires: osinfo-db.*//' {name}.spec sed -i -e 's/BuildRequires: libvirt.*devel.*//' {name}.spec diff --git a/jobs/perl-makemaker.yaml b/jobs/perl-makemaker.yaml index 0a3227a..3e53a29 100644 --- a/jobs/perl-makemaker.yaml +++ b/jobs/perl-makemaker.yaml @@ -42,6 +42,7 @@ - shell: | perl Makefile.PL PREFIX="$VIRT_PREFIX" {make_env} + {job_env} $MAKE $MAKE -j{smp} install $MAKE -j{smp} manifest @@ -83,6 +84,7 @@ builders: - shell: | {make_env} + {job_env} $MAKE -j{smp} test {test_args} publishers: - email: @@ -121,6 +123,7 @@ builders: - shell: | {make_env} + {job_env} sed -i -e 's/BuildRequires: *perl-Sys-Virt.*//' *.spec sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec rm -f *.tar.{archive_format} -- 2.13.6

On Thu, Nov 02, 2017 at 01:45:51PM +0100, Pavel Hrdina wrote:
This fixes libosinfo RPM build, we need to set "OSINFO_SYSTEM_DIR" for RPM build as well since the spec file runs tests and they need valid osinfo-db. Broken by <359b069f7e4700bee337588eab0a9d83cac94010>.
I'm not suggesting you fix this right now, but our current RPM build process is very broken by design, because we are completely ignoring dependancies between packages. We've hacked around this by culling the BuildRequires lines from the spec file before running rpmbuild, and relying on a bit of luck to have the rpmbuild find the stuff we just built into $VIRT_PREFIX. As this test suite problem shows though, this is very fragile. One way to fix this is to create a yum repo populated with all RPMs we build during CI. Then setup a mock vroot to do the builds in, which includes this local yum repo. That way we can honour the RPM deps correctly getting a more accurate test of RPM build process. My main concern with this is that mock is kind of slow to bootstrap the initial vroot, so this would slow our CI somewhat.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- jobs/autotools.yaml | 3 +++ jobs/perl-makemaker.yaml | 3 +++ 2 files changed, 6 insertions(+)
diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml index 9ed5efe..e41d5ab 100644 --- a/jobs/autotools.yaml +++ b/jobs/autotools.yaml @@ -41,6 +41,7 @@ builders: - shell: | {make_env} + {job_env} mkdir build cd build ../autogen.sh --prefix=$VIRT_PREFIX {autogen_args} @@ -82,6 +83,7 @@ builders: - shell: | {make_env} + {job_env} cd build $MAKE -j{smp} syntax-check publishers: @@ -164,6 +166,7 @@ builders: - shell: | {make_env} + {job_env} cd build sed -i -e 's/BuildRequires: osinfo-db.*//' {name}.spec sed -i -e 's/BuildRequires: libvirt.*devel.*//' {name}.spec diff --git a/jobs/perl-makemaker.yaml b/jobs/perl-makemaker.yaml index 0a3227a..3e53a29 100644 --- a/jobs/perl-makemaker.yaml +++ b/jobs/perl-makemaker.yaml @@ -42,6 +42,7 @@ - shell: | perl Makefile.PL PREFIX="$VIRT_PREFIX" {make_env} + {job_env} $MAKE $MAKE -j{smp} install $MAKE -j{smp} manifest @@ -83,6 +84,7 @@ builders: - shell: | {make_env} + {job_env} $MAKE -j{smp} test {test_args} publishers: - email: @@ -121,6 +123,7 @@ builders: - shell: | {make_env} + {job_env} sed -i -e 's/BuildRequires: *perl-Sys-Virt.*//' *.spec sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec rm -f *.tar.{archive_format} -- 2.13.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Thu, Nov 02, 2017 at 03:42:56PM +0000, Daniel P. Berrange wrote:
On Thu, Nov 02, 2017 at 01:45:51PM +0100, Pavel Hrdina wrote:
This fixes libosinfo RPM build, we need to set "OSINFO_SYSTEM_DIR" for RPM build as well since the spec file runs tests and they need valid osinfo-db. Broken by <359b069f7e4700bee337588eab0a9d83cac94010>.
I'm not suggesting you fix this right now, but our current RPM build process is very broken by design, because we are completely ignoring dependancies between packages. We've hacked around this by culling the BuildRequires lines from the spec file before running rpmbuild, and relying on a bit of luck to have the rpmbuild find the stuff we just built into $VIRT_PREFIX. As this test suite problem shows though, this is very fragile.
I completely agree with it, it's fragile and it uses all the environment variables to make sure that all the dependencies can be resolved.
One way to fix this is to create a yum repo populated with all RPMs we build during CI. Then setup a mock vroot to do the builds in, which includes this local yum repo. That way we can honour the RPM deps correctly getting a more accurate test of RPM build process.
My main concern with this is that mock is kind of slow to bootstrap the initial vroot, so this would slow our CI somewhat.
We need to look into it how much it will slow down our CI since we have very limited resources. Maybe it's time to ask whether we can have more resources. Pavel

On Thu, Nov 02, 2017 at 04:54:38PM +0100, Pavel Hrdina wrote:
On Thu, Nov 02, 2017 at 03:42:56PM +0000, Daniel P. Berrange wrote:
On Thu, Nov 02, 2017 at 01:45:51PM +0100, Pavel Hrdina wrote:
This fixes libosinfo RPM build, we need to set "OSINFO_SYSTEM_DIR" for RPM build as well since the spec file runs tests and they need valid osinfo-db. Broken by <359b069f7e4700bee337588eab0a9d83cac94010>.
I'm not suggesting you fix this right now, but our current RPM build process is very broken by design, because we are completely ignoring dependancies between packages. We've hacked around this by culling the BuildRequires lines from the spec file before running rpmbuild, and relying on a bit of luck to have the rpmbuild find the stuff we just built into $VIRT_PREFIX. As this test suite problem shows though, this is very fragile.
I completely agree with it, it's fragile and it uses all the environment variables to make sure that all the dependencies can be resolved.
One way to fix this is to create a yum repo populated with all RPMs we build during CI. Then setup a mock vroot to do the builds in, which includes this local yum repo. That way we can honour the RPM deps correctly getting a more accurate test of RPM build process.
My main concern with this is that mock is kind of slow to bootstrap the initial vroot, so this would slow our CI somewhat.
We need to look into it how much it will slow down our CI since we have very limited resources. Maybe it's time to ask whether we can have more resources.
I think it is well overdue for us to acquire more hardware for Jenkins slaves. If CentOS have a 2nd machine they can spare for us then great, otherwise we should try to push RH to fund some... Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Thu, 2017-11-02 at 13:45 +0100, Pavel Hrdina wrote:
Pavel Hrdina (2): jobs: rename check_env into job_env jobs: use job_env in all job templates
jobs/autotools.yaml | 5 ++++- jobs/defaults.yaml | 2 +- jobs/perl-makemaker.yaml | 3 +++ projects/libosinfo.yaml | 2 +- projects/libvirt.yaml | 2 +- 5 files changed, 10 insertions(+), 4 deletions(-)
I'm not sure this approach is the best one. I like the idea of having a place where we can stick common environment variables - for us that mostly means paths, and we should definitely move guest configuration out of its definition in Jenkins and into this repository, as we prototyped yesterday. But here you're still keeping that info local. Moreover, you're moving the VIR_TEST_* variables from check_env to job_env, which means they end up being defined even during regular 'make'. I seem to recall Dan speaking up against that earlier. I think we should have: global_env - as the name implies, global; for $VIRT_PREFIX and all other paths that affect one or more build jobs, like $OSINFO_SYSTEM_DIR and $MAKE build_env - local; for variables that affect the build step of a single job (can't think of any right now) test_env - local; for variables that affect the test suite of a single job, like $VIR_TEST_DEBUG For projects that inherit from anything but generic-*-job, global_env will be included automatically; projects that don't use any of the known build system will have to take care of that themselves, but then again they already have to do that for make_env. Moreover, we should be able to convert libvirt-cim to use autotools-*-job by tweaking its autogen.sh script, which would leave osinfo-db as the only user of generic-*-job. -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Nov 02, 2017 at 04:32:12PM +0100, Andrea Bolognani wrote:
On Thu, 2017-11-02 at 13:45 +0100, Pavel Hrdina wrote:
Pavel Hrdina (2): jobs: rename check_env into job_env jobs: use job_env in all job templates
jobs/autotools.yaml | 5 ++++- jobs/defaults.yaml | 2 +- jobs/perl-makemaker.yaml | 3 +++ projects/libosinfo.yaml | 2 +- projects/libvirt.yaml | 2 +- 5 files changed, 10 insertions(+), 4 deletions(-)
I'm not sure this approach is the best one.
I like the idea of having a place where we can stick common environment variables - for us that mostly means paths, and we should definitely move guest configuration out of its definition in Jenkins and into this repository, as we prototyped yesterday.
This is a fix of the commit that broke it, I'm not trying to move all environment variables out of Jenkins.
But here you're still keeping that info local. Moreover, you're moving the VIR_TEST_* variables from check_env to job_env, which means they end up being defined even during regular 'make'. I seem to recall Dan speaking up against that earlier.
This is not true, the VIR_TEST_* variables are defined only for "autotools-check-job", see <projects/libvirt.yaml>.
I think we should have:
global_env - as the name implies, global; for $VIRT_PREFIX and all other paths that affect one or more build jobs, like $OSINFO_SYSTEM_DIR and $MAKE
I'm not sure whether there is some different way how to do it but I would rather use different approach than using these defines since you can easily overwrite them and forget to include it.
build_env - local; for variables that affect the build step of a single job (can't think of any right now) test_env - local; for variables that affect the test suite of a single job, like $VIR_TEST_DEBUG
I thought about splitting it into {build,test} but we don't need to have it that way and it feels like an overkill.
For projects that inherit from anything but generic-*-job, global_env will be included automatically; projects that don't use any of the known build system will have to take care of that themselves, but then again they already have to do that for make_env.
And this is exactly why I don't want to do it this way, the "global_env" needs to be present always, like it is now with all the environment variables configured in Jenkins.
Moreover, we should be able to convert libvirt-cim to use autotools-*-job by tweaking its autogen.sh script, which would leave osinfo-db as the only user of generic-*-job.
+1 Pavel

On Thu, Nov 02, 2017 at 04:49:38PM +0100, Pavel Hrdina wrote:
On Thu, Nov 02, 2017 at 04:32:12PM +0100, Andrea Bolognani wrote:
Moreover, we should be able to convert libvirt-cim to use autotools-*-job by tweaking its autogen.sh script, which would leave osinfo-db as the only user of generic-*-job.
+1
The only reason libvirt-cim didn't use the autotools job was that its setup script was called autoconfiscate insteadof autogen.sh. I see that Andrea has fixed that crazyness now so we can use the autotools job easily enough i expect :-) Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Thu, 2017-11-02 at 15:56 +0000, Daniel P. Berrange wrote:
On Thu, Nov 02, 2017 at 04:49:38PM +0100, Pavel Hrdina wrote:
On Thu, Nov 02, 2017 at 04:32:12PM +0100, Andrea Bolognani wrote:
Moreover, we should be able to convert libvirt-cim to use autotools-*-job by tweaking its autogen.sh script, which would leave osinfo-db as the only user of generic-*-job.
+1
The only reason libvirt-cim didn't use the autotools job was that its setup script was called autoconfiscate insteadof autogen.sh. I see that Andrea has fixed that crazyness now so we can use the autotools job easily enough i expect :-)
There's one more difference still: libvirt-cim's autogen.sh doesn't automatically call configure, which autotools-*-job expect. Easily fixed, though. -- Andrea Bolognani / Red Hat / Virtualization

On Thu, 2017-11-02 at 16:49 +0100, Pavel Hrdina wrote:
On Thu, Nov 02, 2017 at 04:32:12PM +0100, Andrea Bolognani wrote:
I'm not sure this approach is the best one.
I like the idea of having a place where we can stick common environment variables - for us that mostly means paths, and we should definitely move guest configuration out of its definition in Jenkins and into this repository, as we prototyped yesterday.
This is a fix of the commit that broke it, I'm not trying to move all environment variables out of Jenkins.
I understand that, and I didn't expect you to of course, but I was considering your changes with that frame of reference and it looked to me like you were changing some things that would have to be changed again for that to happen, so I thought it would be better to go in that direction right away.
But here you're still keeping that info local. Moreover, you're moving the VIR_TEST_* variables from check_env to job_env, which means they end up being defined even during regular 'make'. I seem to recall Dan speaking up against that earlier.
This is not true, the VIR_TEST_* variables are defined only for "autotools-check-job", see <projects/libvirt.yaml>.
You're right, sorry for the noise.
I think we should have:
global_env - as the name implies, global; for $VIRT_PREFIX and all other paths that affect one or more build jobs, like $OSINFO_SYSTEM_DIR and $MAKE
I'm not sure whether there is some different way how to do it but I would rather use different approach than using these defines since you can easily overwrite them and forget to include it.
See below.
build_env - local; for variables that affect the build step of a single job (can't think of any right now) test_env - local; for variables that affect the test suite of a single job, like $VIR_TEST_DEBUG
I thought about splitting it into {build,test} but we don't need to have it that way and it feels like an overkill.
After looking into it better, I agree that splitting them doesn't make much sense at all. How about local_env instead of job_env, to complement global_env defined above? Assuming we don't discard my proposal entirely, that is :)
For projects that inherit from anything but generic-*-job, global_env will be included automatically; projects that don't use any of the known build system will have to take care of that themselves, but then again they already have to do that for make_env.
And this is exactly why I don't want to do it this way, the "global_env" needs to be present always, like it is now with all the environment variables configured in Jenkins.
Is there any way we can have that guarantee without asking users of generic-*-job (which shouldn't be many at all, ideally just the one after we've fixed libvirt-cim) to include it themselves? I'm really not an expert in jenkins-job-builder but since we're setting the environment using export in the 'command' section of the job definition, I don't see much of a way around it. So while I agree with your concern about forgetting to include global_env when defining new jobs, I think that it would still be a better approach than keeping the environment variables in the per-worker Jenkins configuration, where they are not tracked and basically entirely invisible to anyone but the CI administrators. -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
Daniel P. Berrange
-
Pavel Hrdina