[PATCH 1/3] Hook up jenkins support into makebumpver.

[Chris Lumens]

First, add a line to your ~/.rhbzauth file that points to the json API on your
jenkins server:

JENKINS=http://jenkins.wherever.com/api/json

If this is not set, then the jenkins check will be skipped.

Then, make sure the name being passed to makebumpver is the same as the
beginning of some jenkins job you want to check, and that self.git_branch
is the end of the same jenkins job.  Naming is important.

For now this is only in an advisory role - jenkins will be checked and if the
tests do not pass, a warning will be logged.  But it will not stop the build.
---
 scripts/makebumpver | 54 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/scripts/makebumpver b/scripts/makebumpver
index 798e7a7..4300d84 100755
--- a/scripts/makebumpver
+++ b/scripts/makebumpver
@@ -3,7 +3,7 @@
 # makebumpver - Increment version number and add in RPM spec file changelog
 #               block.  Ensures rhel*-branch commits reference RHEL bugs.
 #
-# Copyright (C) 2009-2014  Red Hat, Inc.
+# Copyright (C) 2009-2015  Red Hat, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU Lesser General Public License as published
@@ -26,14 +26,17 @@ log = logging.getLogger("bugzilla")
 log.setLevel(logging.INFO)

import bugzilla
+from contextlib import closing
 import datetime
 import getopt
 import getpass
+import json
 import os
 import re
 import subprocess
 import sys
 import textwrap
+import urllib
 from ConfigParser import ConfigParser

class MakeBumpVer:
@@ -41,6 +44,7 @@ class MakeBumpVer:
         log.debug("%s", kwargs)
         self.bzserver = 'bugzilla.redhat.com'
         self.bzurl = "https://%s/xmlrpc.cgi" % self.bzserver
+        self.jenkins = None
         self.username = None
         self.password = None
         self.bz = None
@@ -57,6 +61,8 @@ class MakeBumpVer:
                     self.username = line[10:].strip('"'')
                 elif line.startswith('RHBZ_PASSWORD='):
                     self.password = line[14:].strip('"'')
+                elif line.startswith('JENKINS='):
+                    self.jenkins = line[8:].strip('"'')

self.gituser = self._gitConfig('user.name')
         self.gitemail = self._gitConfig('user.email')
@@ -82,9 +88,11 @@ class MakeBumpVer:
         self.skip_all = kwargs.get('skip_all', False)
         self.tx_config = kwargs.get('tx_config')
         self.skip_tx = kwargs.get("skip_tx", False)
+        self.skip_jenkins = kwargs.get("skip_jenkins", False)

if self.skip_all:
             self.skip_acks = True
+            self.skip_jenkins = True
             self.skip_tx = True

self.git_branch = None
@@ -441,6 +449,36 @@ class MakeBumpVer:
         f.writelines(bottom)
         f.close()

+    def check_jenkins(self):
+        if not self.git_branch:
+            log.error("No git branch, cannot check jenkins")
+            return False
+
+        if not self.jenkins:
+            log.warning("No jenkins defined, skipping")
+            return True
+
+        ret = True
+
+        with closing(urllib.urlopen(self.jenkins)) as f:
+            contents = f.readlines()
+
+            j = json.loads(contents[0])
+
+            # Get the name of the job we should be looking for in jenkins.
+            # This name is composed from the name of the project, plus the
+            # branch of the project without any "-branch" stuff at the end.
+            targetJob = self.name + "-" + self.git_branch
+            if targetJob.endswith("-branch"):
+                targetJob = targetJob[:-7]
+
+            for job in j["jobs"]:
+                if job["name"] == targetJob:
+                    ret = job["color"] == "blue"
+                    break
+
+        return ret
+
     def check_transifex(self):
         """
         Make sure that the transifex branch matches the current git branch
@@ -475,6 +513,10 @@ class MakeBumpVer:
         if not self.skip_tx and not self.check_transifex():
             sys.exit(1)

+        # For now, jenkins is in an advisory role so do not require it to pass.
+        if not self.skip_jenkins and not self.check_jenkins():
+            log.warning("jenkins test results do not pass; ignoring for now")
+
         newVersion = self._incrementVersion()
         fixedIn = "%s-%s-%s" % (self.name, newVersion, self.release)
         rpmlog = self._rpmLog(fixedIn)
@@ -495,6 +537,7 @@ def usage(cmd):
     sys.stdout.write("    -S, --skip-all   Skip all checks\n")
     sys.stdout.write("    -d, --debug      Turn on debug logging to stdout\n")
     sys.stdout.write("    --skip-tx        Skip checking Transifex config for branch name\n")
+    sys.stdout.write("    --skip-jenkins   Skip checking Jenkins for test results\n")
     sys.stdout.write("\nThe -i switch is intended for use with utility commits that we do not need to\n")
     sys.stdout.write("reference in the spec file changelog.  The -m switch is used to map a Fedora\n")
     sys.stdout.write("BZ number to a RHEL BZ number for the spec file changelog.  Use -m if you have\n")
@@ -509,14 +552,14 @@ def main(argv):
     tx_config = os.path.realpath(cwd + '/.tx/config')
     name, version, release, bugreport = None, None, None, None
     ignore, bugmap = None, None
-    show_help, unknown, skip_acks, skip_all, skip_tx = False, False, False, False, False
+    show_help, unknown, skip_acks, skip_all, skip_tx, skip_jenkins = False, False, False, False, False, False
     opts = []

try:
         opts, _args = getopt.getopt(sys.argv[1:], 'n:v:r:b:i:m:sSd?',
                                    ['name=', 'version=', 'release=',
                                     'bugreport=', 'ignore=', 'map=',
-                                    'debug', 'help', 'skip-tx'])
+                                    'debug', 'help', 'skip-tx', 'skip-jenkins'])
     except getopt.GetoptError:
         show_help = True

@@ -541,6 +584,8 @@ def main(argv):
             log.setLevel(logging.DEBUG)
         elif o in ('--skip-tx'):
             skip_tx = True
+        elif o in ('--skip-jenkins'):
+            skip_jenkins = True
         elif o in ('-?', '--help'):
             show_help = True
         else:
@@ -577,7 +622,8 @@ def main(argv):
     mbv = MakeBumpVer(name=name, version=version, release=release,
                       bugreport=bugreport, ignore=ignore, bugmap=bugmap,
                       configure=configure, spec=spec, skip_acks=skip_acks,
-                      skip_all=skip_all, tx_config=tx_config, skip_tx=skip_tx)
+                      skip_all=skip_all, tx_config=tx_config, skip_tx=skip_tx,
+                      skip_jenkins=skip_jenkins)
     mbv.run()

if __name__ == "__main__":

[PATCH 2/3] Require the livecd target to be larger now.

[Chris Lumens]

This appears to be something required by the move to DNF.
---
 tests/gui/anaconda-autogui-testing.ks | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/gui/anaconda-autogui-testing.ks b/tests/gui/anaconda-autogui-testing.ks
index 56d0062..21c9348 100644
--- a/tests/gui/anaconda-autogui-testing.ks
+++ b/tests/gui/anaconda-autogui-testing.ks
@@ -10,7 +10,7 @@ rootpw qweqwe
 bootloader --location=mbr
 zerombr
 clearpart --all
-part / --fstype="ext4" --size=5000
+part / --fstype="ext4" --size=6000

%post
 cat >> /etc/rc.d/init.d/livesys << EOF

[PATCH 3/3] Fix an issue in the previous pre-existing repo kickstart patch.

[Chris Lumens]

The previous patch was intended to allow you to do this:

repo --name=updates

And then anaconda would read the pre-existing updates repo out of /etc
and enable the repo.  This worked, but it had one fatal flaw:  because we
read all the repos off disk, you couldn't then add your own repo with the
same name as one of those pre-existing ones.  This isn't normally a problem
except for livecd kickstarts, and they do this all over the place.

In yumpayload.py we solve this by letting a new repo delete the definition
it would conflict with, so we need to do that in dnfpayload.py too.
---
 pyanaconda/packaging/dnfpayload.py | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py
index 21c68b8..214988e 100644
--- a/pyanaconda/packaging/dnfpayload.py
+++ b/pyanaconda/packaging/dnfpayload.py
@@ -227,8 +227,23 @@ class DNFPayload(packaging.PackagePayload):
         if mirrorlist:
             repo.mirrorlist = mirrorlist
         repo.sslverify = not (ksrepo.noverifyssl or flags.noverifyssl)
-        repo.enable()
-        self._base.repos.add(repo)
+
+        # If this repo is already known, it's one of two things:
+        # (1) The user is trying to do "repo --name=updates" in a kickstart file
+        #     and we should just know to enable the already existing on-disk
+        #     repo config.
+        # (2) It's a duplicate, and we need to delete the existing definition
+        #     and use this new one.  The highest profile user of this is livecd
+        #     kickstarts.
+        if repo.id in self._base.repos:
+            if not url and not mirrorlist:
+                self._base.repos[repo.id].enable()
+            else:
+                self._base.repos.pop(repo.id)
+        # If the repo's not already known, we've got to add it.
+        else:
+            self._base.repos.add(repo)
+            repo.enable()

# Load the metadata to verify that the repo is valid
         try:
@@ -755,13 +770,7 @@ class DNFPayload(packaging.PackagePayload):
                     repo.enable()

for ksrepo in self.data.repo.dataList():
-            # This is a repo we already have a config file in /etc/anaconda.repos.d,
-            # so we just need to enable it here.  See the kickstart docs for the repo
-            # command.
-            if not ksrepo.baseurl and not ksrepo.mirrorlist:
-                self._base.repos[ksrepo.name].enable()
-            else:
-                self._add_repo(ksrepo)
+            self._add_repo(ksrepo)

ksnames = [r.name for r in self.data.repo.dataList()]
         ksnames.append(constants.BASE_REPO_NAME)

[PATCH 3/3] Fix an issue in the previous pre-existing repo kickstart patch.

[Vratislav Podzimek]

On Fri, 2015-01-16 at 12:24 -0500, Chris Lumens wrote:
> The previous patch was intended to allow you to do this:
> repo --name=updates
>
> And then anaconda would read the pre-existing updates repo out of /etc
> and enable the repo.  This worked, but it had one fatal flaw:  because we
> read all the repos off disk, you couldn't then add your own repo with the
> same name as one of those pre-existing ones.  This isn't normally a problem
> except for livecd kickstarts, and they do this all over the place.
>
> In yumpayload.py we solve this by letting a new repo delete the definition
> it would conflict with, so we need to do that in dnfpayload.py too.
>
> pyanaconda/packaging/dnfpayload.py | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py
> index 21c68b8..214988e 100644
> --- a/pyanaconda/packaging/dnfpayload.py
> +++ b/pyanaconda/packaging/dnfpayload.py
> @@ -227,8 +227,23 @@ class DNFPayload(packaging.PackagePayload):
>          if mirrorlist:
>              repo.mirrorlist = mirrorlist
>          repo.sslverify = not (ksrepo.noverifyssl or flags.noverifyssl)
>    repo.enable()
>    self._base.repos.add(repo)
>    # If this repo is already known, it's one of two things:
>    # (1) The user is trying to do "repo --name=updates" in a kickstart file
>    #     and we should just know to enable the already existing on-disk
>    #     repo config.
>    # (2) It's a duplicate, and we need to delete the existing definition
>    #     and use this new one.  The highest profile user of this is livecd
>    #     kickstarts.
>    if repo.id in self._base.repos:
>        if not url and not mirrorlist:
>            self._base.repos[repo.id].enable()
>        else:
>            self._base.repos.pop(repo.id)

I must admit I don't understand this line. How does the new repo become
enabled? It's not really obvious and I think it deserves a comment in
the code.

[PATCH 3/3] Fix an issue in the previous pre-existing repo kickstart patch.

[Chris Lumens]

> > diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py
> > index 21c68b8..214988e 100644
> > --- a/pyanaconda/packaging/dnfpayload.py
> > +++ b/pyanaconda/packaging/dnfpayload.py
> > @@ -227,8 +227,23 @@ class DNFPayload(packaging.PackagePayload):
> >          if mirrorlist:
> >              repo.mirrorlist = mirrorlist
> >          repo.sslverify = not (ksrepo.noverifyssl or flags.noverifyssl)
> >    repo.enable()
> >    self._base.repos.add(repo)
> >    # If this repo is already known, it's one of two things:
> >    # (1) The user is trying to do "repo --name=updates" in a kickstart file
> >    #     and we should just know to enable the already existing on-disk
> >    #     repo config.
> >    # (2) It's a duplicate, and we need to delete the existing definition
> >    #     and use this new one.  The highest profile user of this is livecd
> >    #     kickstarts.
> >    if repo.id in self._base.repos:
> >        if not url and not mirrorlist:
> >            self._base.repos[repo.id].enable()
> >        else:
> >            self._base.repos.pop(repo.id)
>
> I must admit I don't understand this line. How does the new repo become
> enabled? It's not really obvious and I think it deserves a comment in
> the code.

Ugh, yeah, I think I need to duplicate the two lines in the else just
below this point, here.  I'm sick of this code.

- Chris

[Brian C. Lane]

On Fri, Jan 16, 2015 at 12:24:16PM -0500, Chris Lumens wrote:
>    with closing(urllib.urlopen(self.jenkins)) as f:
>        contents = f.readlines()
>        j = json.loads(contents[0])

This should be wrapped with except (TypeError, ValueError) to catch
problems with the data.

[Chris Lumens]

> This should be wrapped with except (TypeError, ValueError) to catch
> problems with the data.

Done, as discussed in IRC.

- Chris