Merge ~lucaskanashiro/ubuntu/+source/heartbeat:fix-autopkgtest-regression into ubuntu/+source/heartbeat:ubuntu/devel

Proposed by Lucas Kanashiro
Status: Rejected
Rejected by: Lucas Kanashiro
Proposed branch: ~lucaskanashiro/ubuntu/+source/heartbeat:fix-autopkgtest-regression
Merge into: ubuntu/+source/heartbeat:ubuntu/devel
Diff against target: 37 lines (+9/-2)
3 files modified
debian/changelog (+6/-0)
debian/control (+2/-1)
debian/tests/control (+1/-1)
Reviewer Review Type Date Requested Status
Sergio Durigan Junior (community) Approve
Canonical Server Pending
Review via email: mp+402281@code.launchpad.net

Description of the change

Fix autopkgtest regression due to new resource-agents. The fix is adding resource-agents-supported to the test dependencies, the test uses the IPaddr2 agent which is now in the -supported package. Since heartbeat is not a package that we want to support in the HA stack, Rafael and I decided to introduce a minimum delta instead of changing runtime dependency relationships.

PPA:

https://launchpad.net/~lucaskanashiro/+archive/ubuntu/ha-stack/+packages

autopkgtest summary:

autopkgtest [15:48:35]: @@@@@@@@@@@@@@@@@@@@ summary
heartbeat PASS

To post a comment you must log in.
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thanks for the MP, Lucas.

I'm wondering here, doesn't it make sense to actually update heartbeat's Depends/Recommends and add resource-agents-supported given that (from what I was able to see) using IPaddr2 should work out-of-the-box? I think I understand the decision to just update the autopkgtest deps, but I'm worried that a user that has heartbeat installed and working using IPaddr2 will see a breakage happen when she updates to the latest version of the package. WDYT?

review: Needs Information
Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

Thanks for the extra thought on this Sergio. However, from the hearbeat description you can see that the goal is to be a messaging layer which does not mean it should provide any agent. Agents are configured via pacemaker and hearbeat can be used by pacemaker. This issue was raised because of the way the test was written.

Nevertheless, I understand your point, this might became an issue when users relying on it upgrade their system. But to convince you that this is fine, in the following MP changes are added to pacemaker to recommend resource-agents-supported:

https://code.launchpad.net/~lucaskanashiro/ubuntu/+source/pacemaker/+git/pacemaker/+merge/402284

With that, when users upgrade their system if they have pacemaker installed the resource-agents-supported will be pulled in (by default apt install recommended packages), and the IPaddr2 will become available.

As I mentioned we are not planning to support heartbeat, so the idea is to minimize the delta and make the maintenance more straightforward for us. FWIW I discussed this with Rafael and we agreed on this solution.

Is this enough to convince you? Or do you still have any concern?

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thanks for the clarification.

I don't have a deep knowledge about these packages and how they interact with one another, so I'm definitely not going to block this MP if you and Rafael think this is the best way forward. I was mostly concerned about a possible use case that might break with this change; I understand that heartbeat might be used together with pacemaker, but for some reason it also Depends on resource-agents, so I thought that it could be possible to use heartbeat directly (without pacemaker) *and* still want resource-agents* to be available in the system.

In any case, the change itself is simple enough and LGTM, so if you and Rafael agree with it, I'm also +1. Thanks!

review: Approve
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Actually @sergio is right. Thinking about the upgrade path for hirsute -> impish, one having resource-agents installed (or fence-agents) would need to upgrade to resource-agents AND resource-agents-supported (same with fence-agents).

Considering guidelines at: https://wiki.debian.org/PackageTransition I think we have:

#7 Split: only A existed; move some files from A to B; new A does not require new B

Not sure, we could create a virtual package resource-agents containing both resource-agents-supported and resource-agents-unsupported. This way upgrades would still depend on resource-agents and not be brake.

Or we could document this change in release changelog... but knowing Ubuntu dev team, I think they would prefer a smooth upgrade.

On the next release, we could simply remove resource-agents virtual package and let resource-agents-supported and resource-agents-unsupported installed. This way ones that have installed only resource-agents-supported during impish would be ok, and ones having installed resource-agents package in groovy would also be ok.

What do you think ?

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

I agree with your proposal Rafael. It will require changes in src:resource-agents and src:fence-agents but nothing in this MP, am I right? But with the following proposal I believe no changes will be needed in src:heartbeat:

- In src:resource-agents create a transitional binary package called resource-agents which will depend on resource-agents-unsupported and resource-agents-supported.
  + It will require a rename of current resource-agents (unsupported agents) to resource-agents-unsupported.

- In src:fence-agents create a new transitional binary package called fence-agents which will depend on resource-agents-unsupported and resource-agents-supported.
  + It will require a rename of current fence-agents (unsupported agents) to fence-agents-unsupported.

Is that what you thought Rafael? Does this sound good to you Sergio?

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

However, this is kind of sorted out with the latest upload of src:resource-agents:

https://launchpad.net/ubuntu/+source/resource-agents/1:4.7.0-1ubuntu3

The resource-agents binary packages (unsupported agents) recommends resource-agents-supported. If an user is upgrading their system without disabling Recommends installation both package will become available.

Checking the heartbeat autopkgtest failure I can see that resource-agents-supported is listed as a recommended package but it is not installed, which probably means that it does not install recommended packages by default:

https://autopkgtest.ubuntu.com/results/autopkgtest-impish/impish/amd64/h/heartbeat/20210503_164832_cfb3b@/log.gz

Do you think we should change the current approach (resource-agents recommending resource-agents-supported) to the transitional package one?

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

I think we should do the transitional package because there might be many stuff depending on resource-agents - like HA JuJu Charms - that might not have recommends installed automatically. By having the transitional package we would be making sure that we don't depend on end user setup for the upgrade to occur smoothly. There is also the benefit of allowing us to remove the virtual package in the next release, and still keep what end-user installed (if -supported only, as users of hirsute/groovy would have both -supported and -unsupported installed by the transitional package).

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Id like to hear @sergio on this (and to review this idea).

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

+1 on Rafael's argument and also +1 for waiting Sergio's feedback :)

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thanks for the further discussion, guys.

I have reviewed the proposal, and here is what I understood is going to happen:

- Make "resource-agents" a transitional package which will depend on:
  + resource-agents-supported (to be renamed to resource-agents-core or some such) and
  + resource-agents-unsupported (to be renamed to resource-agents-extra)

- Do the same thing for fence-agents.

I understand that this will require a bit more work, pinging AA and waiting for them to accept the new names, but I also think that it is the best course of action for this problem: this way we make sure that the upgrade is smooth and won't break users' setups.

So a +1 from me. Thanks.

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

Unmerged commits

e32f6dc... by Lucas Kanashiro

update-maintainer

acea794... by Lucas Kanashiro

Update changelog

e452a1c... by Lucas Kanashiro

d/t/control: add resource-agents-supported as a test dependency

The test uses IPaddr2 agent and it is not provided by resource-agents
but resource-agents-supported.

The decision to add resource-agents-supported as a test dependency is to
avoid adding a bigger and more complex delta in debian/control to change
package dependencies.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 656b875..7c89583 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,9 @@
6+heartbeat (1:3.0.6-11ubuntu1) impish; urgency=medium
7+
8+ * d/t/control: add resource-agents-supported as a test dependency.
9+
10+ -- Lucas Kanashiro <kanashiro@ubuntu.com> Wed, 05 May 2021 15:43:50 -0300
11+
12 heartbeat (1:3.0.6-11) unstable; urgency=medium
13
14 [ Debian Janitor ]
15diff --git a/debian/control b/debian/control
16index dcc1cb8..4f25298 100644
17--- a/debian/control
18+++ b/debian/control
19@@ -1,7 +1,8 @@
20 Source: heartbeat
21 Section: admin
22 Priority: optional
23-Maintainer: Debian HA Maintainers <debian-ha-maintainers@lists.alioth.debian.org>
24+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
25+XSBC-Original-Maintainer: Debian HA Maintainers <debian-ha-maintainers@lists.alioth.debian.org>
26 Uploaders: Valentin Vidic <vvidic@debian.org>
27 Build-Depends:
28 autoconf,
29diff --git a/debian/tests/control b/debian/tests/control
30index e2c85e3..6560138 100644
31--- a/debian/tests/control
32+++ b/debian/tests/control
33@@ -1,3 +1,3 @@
34-Depends: heartbeat
35+Depends: heartbeat, resource-agents-supported
36 Restrictions: needs-root, isolation-container
37 Tests: heartbeat

Subscribers

People subscribed via source and target branches