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.

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

[collectd 6] RFC: Automatic release process #4222

Open
octo opened this issue Jan 3, 2024 · 9 comments
Open

[collectd 6] RFC: Automatic release process #4222

octo opened this issue Jan 3, 2024 · 9 comments
Projects
Milestone

Comments

@octo
Copy link
Member

octo commented Jan 3, 2024

I think for collectd 6 we should move away from large and manual releases and towards frequent automatic releases, ideally full CI/CD.

Proposed workflow:

  • Trigger the release pipeline on a regular basis (daily? weekly?).
  • Check whether there have been any changes since the last run and exit if not.
  • Extract the new pull requests and from them the ChangeLog: lines.
    • Future improvement: Add changes to the ChangeLog file and commit.
  • Add a release tag (this is required for creating a "release" next)
  • Create a new draft "release" on GitHub with the extracted ChangeLog lines.
  • Set PACKAGE_VERSION to something sane, such as 6.2024.01.03
  • Run make dist to create a .tar.bz2 file.
  • Attach the tar archive to the release and mark the release as published.
  • Trigger a workflow in the collectd.github.io repo to reference the new release from the website.

What do you think about frequent, automatic releases? What are your thoughts on dropping the differentiation between feature releases and patch releases?

@octo octo added this to the 6.0 milestone Jan 3, 2024
@octo octo added this to To do in collectd 6 via automation Jan 3, 2024
@eero-t
Copy link
Contributor

eero-t commented Jan 3, 2024

What do you think about frequent, automatic releases?

Actual releases normally have a manually curated list of major changes, and public pre-releases for extra testing.

Release testing

Because collectd has so many plugins, of which many require specific setups to test & generate data, and some expertise to validate that their data still makes sense, IMHO pre-releases do not work so well for collectd (now that many plugins are lacking active maintainers). Getting info on issues with rarely used plugins could take years, as users may try them only after upgrading their enterprise distro...

From that point, doing more frequent releases that explicitly state what is actually tested, and what is not tested, sounds OK.

=> Listing tests coverage information for the automated releases would be great, but at least which plugins do not have tests, which have tests, and which of those tests were ran/passing should be listed. This information needs to be per-distro, as tested plugins differ between them.

I think this info would look good in table format, with plugins being listed in row headings, distros in column headings and MISSING / SKIP / PASS info in the table cells.

(Hopefully that will also encourage people to submit PRs for improving test coverage of the plugins they use or maintain.)

Release change notes

Typically people do not update their SW to completely new versions daily or even weekly. Not even rolling distros, I think.

While there can be separate automated tagging which is done more frequently (to support testing & discussing issues), and which do not need change notes, I think it makes sense to have automated releases more rarely, e.g. monthly, to "encourage" distros and individuals to converge on testing same releases. That way it's easier for them to compare their results.

Releases do need change information, but that should be easy to automate. In addition to PRs' ChangeLog: lines, link to commit list (between current & previous release tag) [1] and git diff --compact-summary statistics could be fine, in addition to the above mentioned testing info.

Users will also want more curated changes overview, but I think that could be maintained manually in separate Changelog document. Somebody could e.g. twice a year checks merged PRs & commits between last tag mentioned in that file, and the latest one, and document the main changes between them.

[1] E.g: collectd-5.11.0...collectd-5.12.0

Feature vs. patch releases

What are your thoughts on dropping the differentiation between feature releases and patch releases?

Patch releases are something that fix issues found with testing happening after main release, they do not introduce new features, nor change dependencies, like feature releases could do.

If such differentiation is dropped from regular release generation, I think some part of collectd version number should still be incremented whenever a major dependency change is done.

(And if there's a serious bug that was not fixed before such increment, then a doing manual patch release could be still be relevant, but I think it would be OK for doing that only after somebody specifically requests it.)

@octo
Copy link
Member Author

octo commented Jan 3, 2024

Actual releases normally have a manually curated list of major changes, and public pre-releases for extra testing.

I can agree to that definition. The problems I see are:

  1. Manually curating a ChangeLog takes a lot of time and I'm not convinced it brings enough value to justify the effort.
  2. Pre-releases did not surface critical bugs in the past. It just adds to the manual effort required.

[…] IMHO pre-releases do not work so well for collectd (now that many plugins are lacking active maintainers). Getting info on issues with rarely used plugins could take years, as users may try them only after upgrading their enterprise distro...

Agreed.

=> Listing tests coverage information for the automated releases would be great, but at least which plugins do not have tests, which have tests, and which of those tests were ran/passing should be listed.

Agreed that some test coverage info would be nice to have from a project health point of view. I don't think this is required for a release pipeline.

This information needs to be per-distro, as tested plugins differ between them.

I don't quite see how this is a distro specific thing. When we have a test for, say, the Write Prometheus plugin, doesn't this apply to all distros equally? Different architectures on the other hand could eventually be a different story. But by and large we don't have tests for architecture dependent plugins, such as CPU, memory, etc …

(Hopefully that will also encourage people to submit PRs for improving test coverage of the plugins they use or maintain.)

That'd be great, but I'm not going to hold my breath. I think the contributions from Intel have been exemplary in this regard ;)

Typically people do not update their SW to completely new versions daily or even weekly. Not even rolling distros, I think.

I'm convinced that fast is better than slow. If we make the collectd 6 changes available as a tarball now, we'll get an order of magnitude more feedback and bug reports. If we create a new tarball frequently, we keep the cycle short, get fixes and new features into the hands of users quicker and receive feedback sooner.

We gain nothing by releasing less frequently. While spending time crafting a release manually may feel like it's conducive to stability, there is not actually a correlation between care/manual work and stability. Automated testing is the only thing that counts in that regard. (gets off his soap box)

[…] it makes sense to have automated releases more rarely, e.g. monthly, to "encourage" distros and individuals to converge on testing same releases. That way it's easier for them to compare their results.

There is a problem with people reporting bugs in older versions that have been fixed in the mean time. We've had this problem with three releases per year and I think we'll continue to have this problem. However, if we have a newer release available, chances are that some users will try the latest version before opening a bug report, adressing a sub-section of these issues.

Releases do need change information, but that should be easy to automate.

+1 to that

Users will also want more curated changes overview […]

I'm not so sure about that, and I'd be willing to let them contribute those summaries if they are important to them.

If [feature vs. patch release] differentiation is dropped from regular release generation, I think some part of collectd version number should still be incremented whenever a major dependency change is done.

We could enforce that each PR contains one of two labels: "fix" or "feature". We create the release number as "6..", e.g. "6.42.20240103". Whenever we create a release that contains a "feature" PR, we increase <minor>, otherwise only the date changes. This would give us semantic versions (assuming we don't accidentally break backwards compatibility) with very little overhead.

(And if there's a serious bug that was not fixed before such increment, then a doing manual patch release could be still be relevant, but I think it would be OK for doing that only after somebody specifically requests it.)

I think with the above approach, we'd have the ability to do this. I don't think we should use this often though.

@eero-t
Copy link
Contributor

eero-t commented Jan 3, 2024

=> Listing tests coverage information for the automated releases would be great, but at least which plugins do not have tests, which have tests, and which of those tests were ran/passing should be listed.

Agreed that some test coverage info would be nice to have from a project health point of view. I don't think this is required for a release pipeline.

Expectation management is IMHO really important part of project user communication.

Release is implied promise to users, of things being released having also been tested to work. However, I think there are quite a few plugins where that's not the case.

Therefore I would really prefer (automatically generated) release providing info on what was tested i.e. what is supposed to work (and what's not).

If you really do not think that reasonable, I'd suggest at least adding "--enabled-untested-plugins" option, as an alternative for making it clearer to users that there are no promises about certain plugins working. That's much harder for users to check than table in the release notes though.

This information needs to be per-distro, as tested plugins differ between them.

I don't quite see how this is a distro specific thing. When we have a test for, say, the Write Prometheus plugin, doesn't this apply to all distros equally?

Plugin enabling in CI, is distro specific: https://github.com/collectd/collectd/tree/collectd-6.0/.expected-plugins

Therefore running of their tests is also distro specific, isn't it?

There are also plugins which have tests, but which are not enabled in CI for any distro, and therefore not tested by CI (for example my Sysman GPU plugin [1]). In my proposed "test results" table "SKIP" would be indicating that.

[1] Dev package with Level-Zero headers was not in distros when it was merged. Now it is in few of them:

Typically people do not update their SW to completely new versions daily or even weekly. Not even rolling distros, I think.

I'm convinced that fast is better than slow.

That seems to be about end user (search) performance experience, not releases...

If we make the collectd 6 changes available as a tarball now, we'll get an order of magnitude more feedback and bug reports. If we create a new tarball frequently, we keep the cycle short, get fixes and new features into the hands of users quicker and receive feedback sooner.

...But at least in development/test phase fast release cycle is indeed great!

Testing such process first for v6 branch sounds also a good idea.

We gain nothing by releasing less frequently. While spending time crafting a release manually may feel like it's conducive to stability, there is not actually a correlation between care/manual work and stability. Automated testing is the only thing that counts in that regard. (gets off his soap box)

If the intention is just to provide test binaries for every commit, I really hope GitHub has some better way to provide those than calling them releases! :-)

(If every commit is a "release", i.e. picking release equals to picking a random commit, I don't think it to add value to users and they may stop trusting the project producing them...)

Users will also want more curated changes overview […]

Especially users updating the project more rarely. I've been following couple of projects for a long time, and I personally really appreciate such overviews, as it's really hard to get sense of major changes in a project just from commit messages titles (as they are rather cryptic in those projects).

If release notes can collect changeLog: entries from merged PRs, as you suggested, and everybody makes sure they are descriptive, that could be enough though.

@octo
Copy link
Member Author

octo commented Jan 4, 2024

Agreed that some test coverage info would be nice to have from a project health point of view. I don't think this is required for a release pipeline.
[…]
Therefore I would really prefer (automatically generated) release providing info on what was tested i.e. what is supposed to work (and what's not).

I see "what was tested" and "what is supposed to work" as two different metrics. And we need to differentiate a third distinct concept: does the plugin build.

For example, I want to get the CPU plugin into the "it's supposed to work" category, even though it is entirely untested. The Ceph plugin is the inverse example: it has unit tests but has not been migrated to the collectd 6 data structures. The network plugin is an example of a plugin that doesn't build in the collectd-6.0 branch.

I fully agree that we need to communicate clearly which plugins we consider "done", and which plugins are still to be migrated and will likely change in a backwards incompatible manner in the process.

If you really do not think that reasonable, I'd suggest at least adding "--enabled-untested-plugins" option, as an alternative for making it clearer to users that there are no promises about certain plugins working. That's much harder for users to check than table in the release notes though.

That was my thinking, too. Disable all plugins still using value_list_t and provide an --enable-legacy-plugins option. I'm open to better word suggestions than "legacy", but think "untested" causes confusion and is not the right word for what I want to get across.

Plugin enabling in CI, is distro specific: https://github.com/collectd/collectd/tree/collectd-6.0/.expected-plugins
Therefore running of their tests is also distro specific, isn't it?

That primarily influences if we test whether a plugin builds on a given distribution. That can break and frequently does.
(Unit) testing is different: if a plugin is (unit) tested on Debian but not RedHat, I'd still consider the plugin "tested" – I'm not aware of distro specific code.

Building the plugin on a different distribution or with a different dependency version is a different matter.
Using the plugin on a different architecture (e.g. using the cpu plugin on Linux vs. BSD) will cause different code to be compiles, which can influence (unit) test coverage (but I'm not aware of an instance of this; I think all architecture dependent plugins are untested at the moment).

There are also plugins which have tests, but which are not enabled in CI for any distro, and therefore not tested by CI (for example my Sysman GPU plugin [1]). In my proposed "test results" table "SKIP" would be indicating that.

[1] Dev package with Level-Zero headers was not in distros when it was merged. Now it is in few of them:

I have added these packages to Debian 12 and Fedora 39.

Maybe we should just tell users what we know, i.e. use status messages such as "compiles", "test successful", and "disabled".

I'm convinced that fast is better than slow.

That seems to be about end user (search) performance experience, not releases...
[…]
...But at least in development/test phase fast release cycle is indeed great!

Yeah, it's supposed to be a guiding principle, illustrated using a web service.

If the intention is just to provide test binaries for every commit, I really hope GitHub has some better way to provide those than calling them releases! :-)

No, the proposal is to actually buy-in to Continuous Delivery and create release tarballs frequenly, ideally daily (if there have been any changes).

(If every commit is a "release", i.e. picking release equals to picking a random commit, I don't think it to add value to users and they may stop trusting the project producing them...)

A release is more than a random commit, because we only push the release if the code builds and all tests pass. Our test coverage is poor, so this is not a strong guarantee, but this is not fundamentally different from what we have done in the past. In the past I experimented with creating a release branch and letting it sit for a few weeks before creating the official release. In my experience this has not surfaced additional issues before the official release. So in a sense, the picked commit was similarly "random".

If release notes can collect changeLog: entries from merged PRs, as you suggested, and everybody makes sure they are descriptive, that could be enough though.

Yeah, that's the proposal. The quality of ChangeLog entries will vary, and we may need to improve our tooling to help users provide good data. If that fails, maintainers can quite easily modify the PR description to touch things up.

@eero-t
Copy link
Contributor

eero-t commented Jan 4, 2024

That was my thinking, too. Disable all plugins still using value_list_t and provide an --enable-legacy-plugins option. I'm open to better word suggestions than "legacy", but think "untested" causes confusion and is not the right word for what I want to get across.

IMHO legacy is more about something being deprecated, rather than being untested, or not yet being ported to new APIs. Maybe that could be named as "--enable-old-api-plugins"?

Some (separate) indication/list of which plugins neither have tests, nor have been properly tested for years (although they would still build), would still be nice. Only unit tests being there can be checked automatically though, so such thing may need to be manually updated... :-/

That primarily influences if we test whether a plugin builds on a given distribution. That can break and frequently does. (Unit) testing is different: if a plugin is (unit) tested on Debian but not RedHat, I'd still consider the plugin "tested" – I'm not aware of distro specific code.

Building the plugin on a different distribution or with a different dependency version is a different matter. Using the plugin on a different architecture (e.g. using the cpu plugin on Linux vs. BSD) will cause different code to be compiles, which can influence (unit) test coverage (but I'm not aware of an instance of this; I think all architecture dependent plugins are untested at the moment).

That's what I had in mind. There can be issues due to distros providing different dependency versions, and different plugin code paths being enabled due to that, with different results. See the open Rieman plugin bug discussion as example.

I have added these packages to Debian 12 and Fedora 39.

Great, thanks!

Maybe we should just tell users what we know, i.e. use status messages such as "compiles", "test successful", and "disabled".

Sounds good to me!

If the intention is just to provide test binaries for every commit, I really hope GitHub has some better way to provide those than calling them releases! :-)

No, the proposal is to actually buy-in to Continuous Delivery and create release tarballs frequenly, ideally daily (if there have been any changes).

That sounds more like "snapshot" than "release".

A release is more than a random commit, because we only push the release if the code builds and all tests pass.

As CI checks for PRs should already be doing those checks, in principle all commits should build & pass tests (that were enabled when PR was merged), right?

Or did you mean that releases will run some additional tests?

@octo
Copy link
Member Author

octo commented Jan 15, 2024

The --enable-compatibility-mode configure flag is added in #4236

Maybe we should just tell users what we know

I put some more details into the collectd 6 spreadsheet: https://docs.google.com/spreadsheets/d/1ss4NJeJ00CwAmGgIRGQ2EJM9LFifPPrJcq_c0tGN_vY/edit?usp=sharing

Or did you mean that releases will run some additional tests?

No, not for now anyways. We could add more costly tests that we only run when creating a release, e.g. run collectd with a few plugins and ensure that the expected metrics show up. For the time being I propose we do basically the same things we check for every PR:

  • Whether expected plugins get built
  • make check + memory leaks with Valgrind
  • make distcheck (out-of-tree builds and uninstall)

@eero-t
Copy link
Contributor

eero-t commented Jan 15, 2024

I put some more details into the collectd 6 spreadsheet: https://docs.google.com/spreadsheets/d/1ss4NJeJ00CwAmGgIRGQ2EJM9LFifPPrJcq_c0tGN_vY/edit?usp=sharing

Where that list of plugins comes from? It's missing e.g. gpu_sysman, write_open_telemetry, write_prometheus...

For the time being I propose we do basically the same things we check for every PR:

Sounds fine to me, but I would really prefer them being named as "snapshots", rather than "releases", at least until some additional testing [1] is done on them, compared to normal PR/CI.

[1] Regardless of whether it's automated, manual, or just waiting few weeks for bugs to be reported before naming one of the snapshots as a new time based release.

@octo
Copy link
Member Author

octo commented Jan 15, 2024

That list of plugins was created when we started with collectd 6. Write Prometheus is on there (though I did have a filter view enabled earlier today), the other two have been added after collectd-6.0 had diverged from main.

@eero-t
Copy link
Contributor

eero-t commented Jan 24, 2024

Sounds fine to me, but I would really prefer them being named as "snapshots", rather than "releases", at least until some additional testing [1] is done on them, compared to normal PR/CI.

Date based tag would indicate release being snapshots. New release tags should be greater than old ones (package maintainers of course can work around upgrade versioning issues, but they do not appreciate it...): https://www.debian.org/doc/debian-policy/ch-controlfields.html#version

To get people moving from old releases to these snapshots, I think there needs to be clear documentation of the new release process (schedule, tests that need to be passed etc), and release notes linking that doc + stating that no other kind of releases will be forthcoming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
collectd 6
  
To do
Development

No branches or pull requests

2 participants