Dev: ui_node: refactor do_clearstate

[aleksei-burlakov]

Parse the cibadmin output depending on the pacemaker version.
(ref: ClusterLabs/pacemaker#3031,
cibadmin has little changed its format at Version-2.1.7).

[codecov]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (e3d1170) 52.95% compared to head (a2de06b) 52.95%.

Files	Patch %	Lines
crmsh/ui_node.py	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1323      +/-   ##
==========================================
- Coverage   52.95%   52.95%   -0.01%     
==========================================
  Files          79       79              
  Lines       25056    25058       +2     
==========================================
  Hits        13269    13269              
- Misses      11787    11789       +2     

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

[gao-yan]

Thanks, @aleksei-burlakov .

Considering the scenario of rolling upgrade where there are mixed-version of cluster nodes, even if we have the updated stack on local node, crmsh has to deal with the old form of CIB since DC is still an old version.

So we don't really need to check against the pacemaker version here. We can just make crmsh recognize both old and new forms so that it handles either form that appears.

[gao-yan]

BTW, there' another reference to the "crmd" attribute for the command "crm node show":

https://github.com/ClusterLabs/crmsh/blob/master/crmsh/ui_node.py#L338

[aleksei-burlakov]

BTW, there' another reference to the "crmd" attribute for the command "crm node show":

https://github.com/ClusterLabs/crmsh/blob/master/crmsh/ui_node.py#L338

Thank you, good catch!

[aleksei-burlakov]

Thanks, @aleksei-burlakov .

Considering the scenario of rolling upgrade where there are mixed-version of cluster nodes, even if we have the updated stack on local node, crmsh has to deal with the old form of CIB since DC is still an old version.

So we don't really need to check against the pacemaker version here. We can just make crmsh recognize both old and new forms so that it handles either form that appears.

It makes sense.

Btw, I thought to move this logic outside, so that parsing the cib file would be agnostic to its version. I've found, that there is a class CibStatus(object): in the cibstatus.py which from its name hints to do exactly this, but after some investigation I figured out

• first, it does a different thing
• second, it doesn't work
• last, there is no test covering this cibstatus.py

So I'd like also to raise these issues here in the discussion (we can emit an issue later)

[gao-yan]

Apart from the missing test coverage, the code itself looks good to me though.

[liangxin1300]

Apart from the missing test coverage, the code itself looks good to me though.

The CI image for the master branch seems to be broken now, I'm fixing it
Then maybe we can add a related functional test for this to make CI happy

[liangxin1300]

Hi @aleksei-burlakov , could you please backport this to crmsh-4.6 branch? which is for 15SP6

And the commit log should be "Fix: ..... (bsc#1219831)"

Thanks!

[liangxin1300]

I will close this PR since #1334 already merged to crmsh-4.6 and we have a plan to merge crmsh-4.6 into master in recent

[nicholasyang2022]

Close as this is merged into master with #1334 and #1340.