Road to the 3.0 (beta) Release

[Rudolph Bott]

Hey everyone,

we (Apollon, Sascha and myself) have uncovered several issues on our road to the 3.0.0beta1 Release. Following you will find a quick writeup of our findings - most need to be addressed. Several things have changed between 2.16 and 3.0 which break the devel/release script and/or "make distcheck" in one way or another.

AM_MAINTAINER_MODE Patch breaks the out-of-tree-build

This commit[1] has introduced AM_MAINTAINER_MODE but now "make distcheck" (from the devel/release script) breaks in an out-of-tree build because the file vcs-version is missing. Apollon has already taken a look at it and provided a PR [2] and it needs to be reviewed. Thank you for quick help.

Overly long lines in QA and unit tests

The 2to3 migration created some lines which are too long for the Python standard of 80 chars, examples:

test/py/ganeti.rapi.client_unittest.py:165

test/py/ganeti.rpc_unittest.py:402

test/py/ganeti.utils.version_unittest.py:59

test/py/ganeti.utils.version_unittest.py:62

qa/qa_job_utils.py:286

qa/qa_rapi.py:406

Instead of using the distcheck to find these we should check the whole repository at once to find all "line too long" problems and fix them in a single PR. We should also ensure these problems are found by "make pylint" and include that in the Github Actions workflow to find these problems earlier. Sidenote: neither pylint nor pep8 were installed on my testsystem, "make distcheck" seems to use some separate magic to detect overly long lines but I have not yet looked into that.

Wrong link in the 3.0 NEWS file

We cherry-picked the NEWS file from this [3] branch but it contains a typo in the Github Issue link in the first paragraph (the rst-target link does not work, the numbers of the issue ID are flipped). Easy to fix, just here for completeness

The release script needs LC_ALL=C

The script assumes the locale to be set to "C". It will fail (at least through the check-news script) when the locale is set to anything else. This situation could be improved (probably in more than one way).

Python 3 error in check-news

This slipped through the 2to3 migration and has already fixed in the current master branch with this commit [4] (but it is mentioned here for completeness).

I guess thats it for now. Feel free to open PRs for the above mentioned issues or help reviewing them, as they pour in :-)

Cheers,

Rudi

[1] https://github.com/ganeti/ganeti/commit/edc49e6a420b8b01517f0900452c6919375e5ca0

[2] https://github.com/ganeti/ganeti/pull/1465

[3] https://github.com/ganeti/ganeti/tree/prepare-3.0

[4] https://github.com/ganeti/ganeti/commit/f662d3553503ffab934184c56c26ebd1960a96a9

--

 Rudolph Bott - bo...@sipgate.de

 Telefon: +49 (0)211-63 55 56-41

 Telefax: +49 (0)211-63 55 55-22

 sipgate GmbH - Gladbacher Str. 74 - 40219 Düsseldorf

 HRB Düsseldorf 39841 - Geschäftsführer: Thilo Salmon, Tim Mois

 Steuernummer: 106/5724/7147, Umsatzsteuer-ID: DE219349391

 www.sipgate.de - www.sipgate.co.uk

[Sascha Lucas]

Hi Rudi,

On Mon, 25 May 2020, Rudolph Bott wrote:

> Following you will find a quick writeup of our
> findings

Thanks a lot for writing up.

> *Overly long lines in QA and unit tests*
...

> earlier. Sidenote: neither pylint nor pep8 were installed on my testsystem,
> "make distcheck" seems to use some separate magic to detect overly long
> lines but I have not yet looked into that.

This script checking for long lines is autotools/check-python-code. It is
called from the "LC_ALL=C make check-local" target. Which already needs
setting the locale.

After hacking away two errors, where .github/ and .github/workflows/ is
not in Makefile.am, it unveils the long lines:

lib/hypervisor/hv_kvm/__init__.py:176: raise(RuntimeError("QMP
decorator could not find a valid ganeti instance object"))
lib/hypervisor/hv_kvm/__init__.py:2557: max_bandwidth_in_bytes =
instance.hvparams[constants.HV_MIGRATION_BANDWIDTH] * 1024 * 1024
lib/hypervisor/hv_kvm/__init__.py:2558:
self.qmp.SetMigrationParameters(max_bandwidth_in_bytes,
instance.hvparams[constants.HV_MIGRATION_DOWNTIME])
lib/hypervisor/hv_kvm/__init__.py:2562:
self.qmp.SetMigrationCapabilities(migration_caps.split(_MIGRATION_CAPS_DELIM))
lib/hypervisor/hv_kvm/__init__.py:2611:
migration_status.transferred_ram = query_migrate["ram"]["transferred"]
Longest line in lib/hypervisor/hv_kvm/__init__.py is longer than 80
characters

qa/qa_job_utils.py:286: raise
self._exc_info[0](self._exc_info[1]).with_traceback(self._exc_info[2])
Longest line in qa/qa_job_utils.py is longer than 80 characters

qa/qa_rapi.py:406: missing_params = [x for x in params_of_interest if x
not in info and x not in exceptions]
Longest line in qa/qa_rapi.py is longer than 80 characters

test/py/ganeti.rapi.client_unittest.py:165: for name in [s for s in
dir(client) if (s.startswith("ECODE_") and s != "ECODE_ALL")]:
Longest line in test/py/ganeti.rapi.client_unittest.py is longer than 80
characters

test/py/ganeti.rpc_unittest.py:402: self.assertEqual(res(nodes,
NotImplemented), list(zip(nodes, addresses, nodes)))
Longest line in test/py/ganeti.rpc_unittest.py is longer than 80
characters

Found 5 problem(s) while checking code.

The known one introduced and fixed in stable-2.16 are excluded.

Thanks, Sascha.

[Sascha Lucas]

Hi,

Apollon has made the master branch "release-able" (#1469). Thank you very
much!

On Mon, 25 May 2020, Rudolph Bott wrote:

> *Wrong link in the 3.0 NEWS file*

fixed in prepare-3.0 branch.

> *The release script needs LC_ALL=C*

needs to be documented or can be scripted in the release-script? i.e.

LC_ALL=C make -j$PARALLEL distcheck-release
LC_ALL=C fakeroot make -j$PARALLEL dist-release

So only NEWS content is missing.

Thanks so far, Sascha.

[Sascha Lucas]

Hi,

I tried to run devel/release with latest master. Seems like if there are
still open issues:

$ LC_ALL=C URL=file:///${HOME}/ganeti devel/release master

Some issues are just missing version info in various doc files:

Incorrect version in doc/iallocator.rst, expected 3.0
Incorrect version in doc/hooks.rst, expected 3.0
Incorrect version in doc/virtual-cluster.rst, expected 3.0
Incorrect version in doc/security.rst, expected 3.0
File ../../doc/design-3.0.rst not found
doc/design-draft.rst was not updated for version 3.0

doc/design-3.0.rst must also be added in doc/index.rst, Makefile.am
(docinput) and mentioned in doc/design-draft.rst.

But more serious is, that I'm hitting a Haskell test failure (output
shortend by using visual diff with my eyes):

Config_serialisation: [Failed]
*** Failed! Falsifiable (after 227 tests):
Expected equality, but got mismatch

expected: ConfigData { ... clusterOsparams = GenericContainer
{fromContainer = fromList [("\DEL\241\179\169\185UZ",GenericContainer
{fromContainer = fromList [...,("t\225\133\130}G\239\191\189\RSN"...

but got: ConfigData {... clusterOsparams = GenericContainer
{fromContainer = fromList [("\DEL\241\179\169\185UZ",GenericContainer
{fromContainer = fromList [...,("t\225\133\130}G\239\191\191\RSN"...

========================================
1 of 108 tests failed
Please report to gan...@googlegroups.com
========================================
make[2]: *** [Makefile:3732: check-TESTS] Error 1
make[2]: Leaving directory
'/tmp/gntrelease.JOF6HxI5fR/dist/ganeti-3.0.0~beta1/_build/sub'
make[1]: *** [Makefile:3996: check-am] Error 2
make[1]: Leaving directory
'/tmp/gntrelease.JOF6HxI5fR/dist/ganeti-3.0.0~beta1/_build/sub'
make: *** [Makefile:3917: distcheck] Error 1

Does anyone else see this error?

Thanks, Sascha.

[Sascha Lucas]

Hi,

just self answering ...

On Fri, 29 May 2020, Sascha Lucas wrote:

> $ LC_ALL=C URL=file:///${HOME}/ganeti devel/release master

> Config_serialisation: [Failed]
> *** Failed! Falsifiable (after 227 tests):
> Expected equality, but got mismatch

It seems that parallel build breaks the release script. Using:

$ PARALLEL=1 LC_ALL=C URL=file:///${HOME}/ganeti devel/release master

produces the tar ball. Auto detected PARALLEL would be 4 in my case.

Thanks, Sascha.

[Apollon Oikonomopoulos]

On 18:56 Fri 29 May , Sascha Lucas wrote:
> Hi,
>
> just self answering ...
>
> On Fri, 29 May 2020, Sascha Lucas wrote:
>
> > $ LC_ALL=C URL=file:///${HOME}/ganeti devel/release master
>
> > Config_serialisation: [Failed]
> > *** Failed! Falsifiable (after 227 tests):
> > Expected equality, but got mismatch
>
> It seems that parallel build breaks the release script. Using:
>
> $ PARALLEL=1 LC_ALL=C URL=file:///${HOME}/ganeti devel/release master

That has nothing to do with parallel builds, unfortunately some of the
Haskell serialization tests using Quickcheck are flaky and once in a
while Quickcheck manages to generate input that breaks serialization.
Ideally we'd like to find and fix these bugs, but they don't seem to
happen with any kind of "sane" input.

That said, I think we can move forward with the release. Any objections?

Cheers,
Apollon

>
> produces the tar ball. Auto detected PARALLEL would be 4 in my case.
>
> Thanks, Sascha.
>

> --
> You received this message because you are subscribed to the Google Groups "ganeti-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to ganeti-devel...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/ganeti-devel/alpine.DEB.2.20.2005291852120.9694%40ivy.loc.

[Sascha Lucas]

Hi,

On Tue, 2 Jun 2020, Apollon Oikonomopoulos wrote:

> On 18:56 Fri 29 May, Sascha Lucas wrote:

>> On Fri, 29 May 2020, Sascha Lucas wrote:
>>
>>> $ LC_ALL=C URL=file:///${HOME}/ganeti devel/release master
>>
>>> Config_serialisation: [Failed]
>>> *** Failed! Falsifiable (after 227 tests):
>>> Expected equality, but got mismatch
>>
>> It seems that parallel build breaks the release script. Using:
>>
>> $ PARALLEL=1 LC_ALL=C URL=file:///${HOME}/ganeti devel/release master
>
> That has nothing to do with parallel builds, unfortunately some of the
> Haskell serialization tests using Quickcheck are flaky and once in a
> while Quickcheck manages to generate input that breaks serialization.

Ah, It happened earlier to me, that hs-test (or hs-check) fails pseudo
randomly. I just run make again and thought of some race condition. Good
to know that there is an other source for the flakiness.

> That said, I think we can move forward with the release. Any objections?

No objections. I've prepared #1475 for the latest changes in NEWS +
release date.

Maybe you can take a look on #1474, but this should not block the
3.0-beta1 release.

Thanks, Sascha.