Dev: corosync: add subcommands 'crm corosync link ...' for managing multi-links in knet (jsc#PED-8083) #1471

nicholasyang2022 · 2024-06-24T08:55:08Z

Knet transport in corosync provides flexible configurations of multi-link connectivity. This pull request adds some subcommands to manage these configurations, including:

crm corosync link show to list configured links
crm corosync link update <linknumber> [<nodename>=<new_address> ... ] [options <option>=[value] ...] to modify an existing link
crm corosync link add <nodename>=<new_address> ... [options <option>=[value] ...] to add a new link.
crm corosync link remove <linknumber> to remove an existing link.

These subcommands operate on local configuration file corosync.conf. To make the changes to take effects, the configuration file needs to be synchronized across the cluster with crm corosync push and corosync service needs to be restarted.

codecov · 2024-06-24T09:18:21Z

Codecov Report

Attention: Patch coverage is 93.42723% with 28 lines in your changes missing coverage. Please review.

Project coverage is 69.94%. Comparing base (2abacf2) to head (2c07507).

Files	Patch %	Lines
crmsh/ui_corosync.py	88.70%	20 Missing ⚠️
crmsh/corosync.py	96.80%	7 Missing ⚠️
crmsh/bootstrap.py	66.66%	1 Missing ⚠️

Additional details and impacted files

Flag	Coverage Δ
integration	`54.80% <88.02%> (+0.54%)`	⬆️
unit	`52.71% <68.30%> (+0.24%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
crmsh/corosync_config_format.py	`100.00% <100.00%> (ø)`
crmsh/iproute2.py	`100.00% <100.00%> (ø)`
crmsh/bootstrap.py	`88.65% <66.66%> (-0.01%)`	⬇️
crmsh/corosync.py	`87.64% <96.80%> (+7.03%)`	⬆️
crmsh/ui_corosync.py	`74.18% <88.70%> (+24.18%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

test/features/corosync_ui.feature

crmsh/ui_corosync.py

liangxin1300 · 2024-06-28T07:56:05Z

crm(live/alp-1)corosync link# show
Link 0:
  Options:

  Node addresses:
    Node 1: alp-1	192.168.122.189
    Node 2: alp-2	192.168.122.145

Link 1:
  Options:

  Node addresses:
    Node 1: alp-1	10.10.10.61
    Node 2: alp-2	10.10.10.62

crm(live/alp-1)corosync link# add alp-1=20.20.20.61 alp-2=10.10.10.62

10.10.10.62 is already used, it should raise an error here

liangxin1300 · 2024-06-28T08:25:24Z

crm(live/alp-1)corosync link# add alp-1=10.10.10.61 alp-2=10.10.10.61
INFO: Use "crm corosync diff" to show the difference
INFO: Use "crm corosync push" to sync

Should complaint when assigning the same IP

liangxin1300 · 2024-06-28T08:28:40Z

It should raise an error when assigning not existing IPaddress when adding a link
No complaint for the invalid IP when adding a link

crm(live/alp-1)corosync link# add alp-1=20.20.20 alp-2=30.30.30
INFO: Use "crm corosync diff" to show the difference
INFO: Use "crm corosync push" to sync

liangxin1300 · 2024-07-01T00:07:39Z

crm(live/alp-1)corosync link# show
Link 0:
  Options:

  Node addresses:
    Node 1: alp-1       192.168.122.189
    Node 2: alp-2       192.168.122.145

Link 1:
  Options:
    knet_transport:     sctp

  Node addresses:
    Node 1: alp-1       10.10.10.61
    Node 2: alp-2       10.10.10.62

I think the showing order should be Node addresses first, then Options, since:

all of your usages are adding addr first, then options
people might care addresses most, not the options, which mostly use the default ones

liangxin1300 · 2024-07-01T00:31:44Z

After adding link twice:

nodelist {
        node {
                ring0_addr: 192.168.122.189
                name: alp-1
                nodeid: 1
                ring1_addr: 10.10.10.61
                ring2_addr: 20.20.20.61
        }

        node {
                ring0_addr: 192.168.122.145
                name: alp-2
                nodeid: 2
                ring1_addr: 10.10.10.62
                ring2_addr: 20.20.20.62
        }

}

Please put the ring 0-2 together

liangxin1300 · 2024-07-01T00:34:19Z

crm(live/alp-1)corosync link# update 2 
INFO: Use "crm corosync diff" to show the difference
INFO: Use "crm corosync push" to sync

This case should raise an error and show the usage?

nicholasyang2022 · 2024-07-02T05:02:09Z

crm(live/alp-1)corosync link# update 2 
INFO: Use "crm corosync diff" to show the difference
INFO: Use "crm corosync push" to sync

This case should raise an error and show the usage?

This is syntactically correct according to the usage. IMO it is better to exit successfully with a warning message.

liangxin1300 · 2024-07-08T06:06:45Z

crm(live/alp-1)corosync link# show

  Node addresses:
    Node 1: alp-1       192.168.122.68
    Node 2: alp-2       192.168.122.238
Link 0:
  Options:


  Node addresses:
    Node 1: alp-1       10.10.10.61
    Node 2: alp-2       10.10.10.62
Link 1:
  Options:

crm(live/alp-1)corosync link#

Link <index> should be the first line

liangxin1300 · 2024-07-08T06:14:38Z

crm(live/alp-1)corosync link# show

  Node addresses:
    Node 1: alp-1       192.168.122.68
    Node 2: alp-2       192.168.122.238
Link 0:
  Options:


  Node addresses:
    Node 1: alp-1       10.10.10.61
    Node 2: alp-2       10.10.10.62
Link 1:
  Options:

crm(live/alp-1)corosync link# remove 0
ERROR: corosync.link.remove: Cannot remove the last link.

When there are two or more links, remove the first link(index is 0) should be allowed
I've tested that corosync works well when there is no ring0_addr

…8083)

…links

…dating links

…y arguement

…(jsc#PED-8083)

Not fully work. As the help system does not support subcommands at 3rd-level. Apply suggestions from code review for docs Co-authored-by: Roger Zhou <zzhou@suse.com>

…w links

… when an not updatable option is specified

…odule

…code

…D-8083) Both updating link options and adding/removing links are hot reloadable. Updating the address of existing nodes is not hot reloadable. Update crmsh/ui_corosync.py Co-authored-by: Roger Zhou <zzhou@suse.com> do not reload or restart if corosync.service is not running Co-authored-by: Roger Zhou <zzhou@suse.com>

… nodes (jsc#PED-8083)

…luster (jsc#PED-8083)

nicholasyang2022 · 2024-08-12T06:22:41Z

We have some problem in the functional tests with corosync-cmapctl -m stats.

     Given Run "crm corosync link remove 1" OK on "hanode1"                                                                 # opt/crmsh/test/features/steps/step_implementation.py:101
ERROR:Step:Run "crm corosync link remove 1" OK on "hanode1":
ERROR: corosync.link.remove: Cannot remove link 1. blablabla

Here the whole stats.knet.node2.link0 section is missing from the stats:

hanode2:/ # corosync-cmapctl -m stats | grep connected
stats.knet.node1.link0.connected (u8) = 1
stats.knet.node1.link1.connected (u8) = 1
stats.knet.node2.link0.connected (u8) = 1

While corosync.log shows that link1 is working:

hanode2:/ # tail /var/log/cluster/corosync.log 
Aug 12 06:12:39 [1819] hanode2 corosync info    [TOTEM ] Configured link number 1: local addr: 172.18.0.3, port=5406
Aug 12 06:12:39 [1819] hanode2 corosync info    [KNET  ] host: host: 1 (passive) best link: 0 (pri: 10)
Aug 12 06:12:39 [1819] hanode2 corosync info    [KNET  ] host: host: 1 (passive) best link: 0 (pri: 10)
Aug 12 06:12:39 [1819] hanode2 corosync info    [KNET  ] host: host: 1 (passive) best link: 0 (pri: 10)
Aug 12 06:12:39 [1819] hanode2 corosync info    [KNET  ] pmtud: MTU manually set to: 0
Aug 12 06:12:42 [1819] hanode2 corosync info    [KNET  ] rx: host: 1 link: 1 is up
Aug 12 06:12:42 [1819] hanode2 corosync info    [KNET  ] link: Resetting MTU for link 1 because host 1 joined
Aug 12 06:12:42 [1819] hanode2 corosync info    [KNET  ] host: host: 1 (passive) best link: 1 (pri: 11)
Aug 12 06:12:42 [1819] hanode2 corosync info    [KNET  ] pmtud: PMTUD link change for host: 1 link: 1 from 485 to 1397
Aug 12 06:12:42 [1819] hanode2 corosync info    [KNET  ] pmtud: Global data MTU changed to: 1397

zzhou1 · 2024-08-14T07:27:37Z

We have some problem in the functional tests with corosync-cmapctl -m stats.

A guess work, maybe corosync link state is still transitioning, maybe need wait a little time for the stable state?

nicholasyang2022 changed the title ~~Dev: corosync: add subcommands 'crm corosync link ...' for managing multi-links in knet~~ Dev: corosync: add subcommands 'crm corosync link ...' for managing multi-links in knet (jsc#PED-8083) Jun 24, 2024

nicholasyang2022 force-pushed the ped-8083 branch 5 times, most recently from 8fb8600 to 255f49c Compare June 27, 2024 05:27

nicholasyang2022 requested a review from liangxin1300 June 27, 2024 06:26

nicholasyang2022 marked this pull request as ready for review June 27, 2024 06:26

liangxin1300 reviewed Jun 28, 2024

View reviewed changes

test/features/corosync_ui.feature Outdated Show resolved Hide resolved

liangxin1300 reviewed Jun 28, 2024

View reviewed changes

crmsh/ui_corosync.py Show resolved Hide resolved

nicholasyang2022 mentioned this pull request Jun 28, 2024

help module does not support 3 levels of subcommands #1478

Open

nicholasyang2022 force-pushed the ped-8083 branch 2 times, most recently from ba0a473 to 24777f7 Compare June 28, 2024 10:19

nicholasyang2022 force-pushed the ped-8083 branch from 24777f7 to 11d1d42 Compare July 2, 2024 04:54

nicholasyang2022 force-pushed the ped-8083 branch 4 times, most recently from a0db49d to 53fcbc3 Compare July 4, 2024 05:35

nicholasyang2022 requested a review from liangxin1300 July 4, 2024 06:06

nicholasyang2022 force-pushed the ped-8083 branch from 53fcbc3 to 05a1dcc Compare July 8, 2024 06:48

nicholasyang2022 and others added 26 commits August 2, 2024 11:24

Dev: corosync: refactor COROSYNC_KNOWN_SEC_NAMES_WITH_LIST

b8b072b

Dev: corosync: implement collecting link info from config (jsc#PED-8083)

bb712ed

Dev: ui_corosync: add subcommand 'crm corosync link show' (jsc#PED-8083)

26ef2c5

minor code cleanup

f8d7dff

Dev: corosync: implement updating link options (jsc#PED-8083)

bb43cc8

minor code cleanup

e5897a7

Dev: ui_corosync: add subcommand 'crm corosync link update' (jsc#PED-…

ca81599

…8083)

unused code removal

75c2ff9

Dev: corosync: implement updating node addresses (jsc#ped-8083)

2d13d82

Dev: corosync: implement removing links (jsc#PED-8083)

e8abf7d

Dev: corosync: implement adding links (jsc#PED-8083)

fd8df46

Dev: ui_corosync: add subcommand 'crm corosync link add' (jsc#PED-8083)

f024898

Fix: corosync: show detect duplicated addresses when adding/updating …

da69883

…links

Fix: ui_corosync: should detect unconfigured addresses when adding/up…

83ef615

…dating links

Dev: ui_corosync: print a message when running link update without an…

39c1459

…y arguement

Dev: corosync_config_format: sort keys to put ringX_addrs together …

08e8f69

…(jsc#PED-8083)

Fix: corosync: does not detect last link correctly

134b191

Dev: behave: add funtional tests for 'crm corosync link ...'

1dd074e

Dev: docs: add documents for crm corosync link

2a0e6f0

Not fully work. As the help system does not support subcommands at 3rd-level. Apply suggestions from code review for docs Co-authored-by: Roger Zhou <zzhou@suse.com>

Fix: ui_corosync: should not allow empty option values when adding ne…

4639b67

…w links

Dev: corosync: add the list of updatable options in the error message…

9c6e2a7

… when an not updatable option is specified

Dev: bootstrap: refactor to reuse KNET_LINK_NUM_LIMIT from corosync m…

c5e37df

…odule

Dev: ui_corosync: refactor to reuse configuration loading and saving …

fb4ba18

…code

Dev: ui_corosync: refine the error messages for missing or duplicated…

3280f3f

… nodes (jsc#PED-8083)

Dev: corosync: allow linknumber not to start with 0

2c07507

nicholasyang2022 force-pushed the ped-8083 branch from 6b19adc to 2c07507 Compare August 2, 2024 03:26

Dev: ui_corosync: reject to remove a link if removing it breaks the c…

a1bc475

…luster (jsc#PED-8083)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev: corosync: add subcommands 'crm corosync link ...' for managing multi-links in knet (jsc#PED-8083) #1471

Dev: corosync: add subcommands 'crm corosync link ...' for managing multi-links in knet (jsc#PED-8083) #1471

nicholasyang2022 commented Jun 24, 2024 •

edited by zzhou1

Loading

codecov bot commented Jun 24, 2024 •

edited

Loading

liangxin1300 commented Jun 28, 2024

liangxin1300 commented Jun 28, 2024

liangxin1300 commented Jun 28, 2024 •

edited

Loading

liangxin1300 commented Jul 1, 2024

liangxin1300 commented Jul 1, 2024

liangxin1300 commented Jul 1, 2024

nicholasyang2022 commented Jul 2, 2024

liangxin1300 commented Jul 8, 2024 •

edited

Loading

liangxin1300 commented Jul 8, 2024

nicholasyang2022 commented Aug 12, 2024

zzhou1 commented Aug 14, 2024

Dev: corosync: add subcommands 'crm corosync link ...' for managing multi-links in knet (jsc#PED-8083) #1471

Are you sure you want to change the base?

Dev: corosync: add subcommands 'crm corosync link ...' for managing multi-links in knet (jsc#PED-8083) #1471

Conversation

nicholasyang2022 commented Jun 24, 2024 • edited by zzhou1 Loading

codecov bot commented Jun 24, 2024 • edited Loading

Codecov Report

liangxin1300 commented Jun 28, 2024

liangxin1300 commented Jun 28, 2024

liangxin1300 commented Jun 28, 2024 • edited Loading

liangxin1300 commented Jul 1, 2024

liangxin1300 commented Jul 1, 2024

liangxin1300 commented Jul 1, 2024

nicholasyang2022 commented Jul 2, 2024

liangxin1300 commented Jul 8, 2024 • edited Loading

liangxin1300 commented Jul 8, 2024

nicholasyang2022 commented Aug 12, 2024

zzhou1 commented Aug 14, 2024

nicholasyang2022 commented Jun 24, 2024 •

edited by zzhou1

Loading

codecov bot commented Jun 24, 2024 •

edited

Loading

liangxin1300 commented Jun 28, 2024 •

edited

Loading

liangxin1300 commented Jul 8, 2024 •

edited

Loading