salt,tests: Allow forcing the creation of an LV #3877

gdemonet · 2022-09-26T13:39:15Z

In case a formatted LV was removed without wiping its FS signature, creation from storage-operator will fail. We add support for an annotation (metalk8s.scality.com/force-lvcreate) to force the creation of the LV, at the risk of also wiping the previous FS contents.

bert-e · 2022-09-26T13:39:18Z

Hello gdemonet,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2022-09-26T13:39:22Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

JBWatenbergScality · 2022-09-26T15:20:42Z

ui/src/containers/CreateVolume.js

+                        <Input
+                          name="forceLVCreate"
+                          value={values.forceLVCreate}
+                          onChange={handleChange('forceLVCreate')}
+                          label={
+                            <>


I think this should be a toggle.

JBWatenbergScality · 2022-09-26T15:22:04Z

ui/src/containers/CreateVolume.js

+                                  </div>
+                                }
+                              >
+                                <InputWarning className="fas fa-triangle-exclamation"></InputWarning>


nit: fa-triangle-exclamation icon is being deprecated in favor of fa-exclamation-circle

JBWatenbergScality · 2022-09-26T15:23:06Z

ui/src/containers/CreateVolume.js

+                              <Tooltip
+                                placement="right"
+                                overlay={
+                                  <div style={{ minWidth: '200px' }}>


nit: the new form component is going to handle this, so you don't have to care about it

Suggested change

<div style={{ minWidth: '200px' }}>

<div style={{ minWidth: '80rem' }}>

JBWatenbergScality · 2022-09-26T15:28:00Z

ui/src/containers/CreateVolume.js

@@ -208,6 +208,10 @@ const InputQuestionMark = styled.i`
  color: ${(props) => props.theme.infoPrimary};
 `;

+const InputWarning = styled(InputQuestionMark)`


nit: InputWarning implies that it would be an input component simply with color changing to warning. However here it is the Icon that change. Moreover we have a new Icon component in core-ui that should be used for this purpose. <Icon color='statusWarning' name='Exclamation-circle' />

bert-e · 2022-09-30T09:46:37Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

tests/post/features/volume.feature

tests/post/steps/test_volume.py

TeddyAndrieux · 2022-10-05T06:54:20Z

tests/post/features/volume.feature

@@ -269,3 +269,36 @@ Feature: Volume management
        Then the Volume 'test-volume12-lvmlv' is 'Available'
        And the PersistentVolume 'test-volume12-lvmlv' has size '10Gi'
        And the device '/dev/test-vg-12/test-volume12-lvmlv' exists
+
+    Scenario: Test volume re-creation (lvmLogicalVolume force-lvcreate)


From the CI build it seems you got a failure on this test in 3-nodes (flaky ?, weird, I don't see how this test could be flaky and the one for the single node succeeded 😕 )

Yeah I need to debug this, haven't had time yet

For reference: this was caused by an older version of LVM in CentOS 7, which doesn't prevent LV creation even if signatures were detected and not wiped. Added a note about this in the CHANGELOG, and skipped the test on CentOS 7 / RHEL 7 (actually, just testing the osmajorrelease grain, but we only support RedHat-family OSes, so should be enough).

bert-e · 2022-10-06T07:06:29Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

tests/post/features/volume.feature

In case a formatted LV was removed without wiping its FS signature, creation from storage-operator will fail. We add support for an annotation (metalk8s.scality.com/force-lvcreate) to force the creation of the LV, at the risk of also wiping the previous FS contents.

TeddyAndrieux

LGTM

gdemonet · 2022-10-07T12:14:08Z

/approve

bert-e · 2022-10-07T12:14:14Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/124.0

The following branches will NOT be impacted:

development/123.0
development/2.0
development/2.1
development/2.10
development/2.11
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2022-10-07T12:14:19Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/124.0

The following branches have NOT changed:

development/123.0
development/2.0
development/2.1
development/2.10
development/2.11
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

Please check the status of the associated issue None.

Goodbye gdemonet.

gdemonet added topic:storage Issues related to storage topic:tests What's not tested may be broken topic:salt Everything related to SaltStack in our product labels Sep 26, 2022

gdemonet force-pushed the feature/force-lvcreate branch 2 times, most recently from 95df02c to c38b6d4 Compare September 26, 2022 14:50

JBWatenbergScality reviewed Sep 26, 2022

View reviewed changes

gdemonet force-pushed the feature/force-lvcreate branch 2 times, most recently from 9228cda to 140a700 Compare September 30, 2022 07:56

gdemonet marked this pull request as ready for review September 30, 2022 09:46

gdemonet requested a review from a team as a code owner September 30, 2022 09:46

TeddyAndrieux reviewed Sep 30, 2022

View reviewed changes

tests/post/features/volume.feature Show resolved Hide resolved

TeddyAndrieux reviewed Sep 30, 2022

View reviewed changes

tests/post/steps/test_volume.py Show resolved Hide resolved

gdemonet force-pushed the feature/force-lvcreate branch 3 times, most recently from 6a00bbb to fdf6c18 Compare October 3, 2022 14:53

TeddyAndrieux reviewed Oct 5, 2022

View reviewed changes

This comment was marked as resolved.

Sign in to view

gdemonet force-pushed the feature/force-lvcreate branch from fdf6c18 to bb28614 Compare October 6, 2022 07:06

gdemonet force-pushed the feature/force-lvcreate branch from bb28614 to b4b4965 Compare October 6, 2022 11:58

TeddyAndrieux reviewed Oct 6, 2022

View reviewed changes

tests/post/features/volume.feature Outdated Show resolved Hide resolved

gdemonet force-pushed the feature/force-lvcreate branch 2 times, most recently from 6e58d5b to 3a7f563 Compare October 7, 2022 09:49

gdemonet force-pushed the feature/force-lvcreate branch from 3a7f563 to e431996 Compare October 7, 2022 10:04

TeddyAndrieux approved these changes Oct 7, 2022

View reviewed changes

bert-e merged commit e431996 into development/124.0 Oct 7, 2022

bert-e deleted the feature/force-lvcreate branch October 7, 2022 12:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

salt,tests: Allow forcing the creation of an LV #3877

salt,tests: Allow forcing the creation of an LV #3877

gdemonet commented Sep 26, 2022

bert-e commented Sep 26, 2022

bert-e commented Sep 26, 2022

JBWatenbergScality Sep 26, 2022

JBWatenbergScality Sep 26, 2022

JBWatenbergScality Sep 26, 2022

JBWatenbergScality Sep 26, 2022

bert-e commented Sep 30, 2022

TeddyAndrieux Oct 5, 2022

gdemonet Oct 5, 2022

gdemonet Oct 6, 2022 •

edited

Loading

This comment was marked as resolved.

bert-e commented Oct 6, 2022

TeddyAndrieux left a comment

gdemonet commented Oct 7, 2022

bert-e commented Oct 7, 2022

bert-e commented Oct 7, 2022

	<div style={{ minWidth: '200px' }}>
	<div style={{ minWidth: '80rem' }}>

salt,tests: Allow forcing the creation of an LV #3877

salt,tests: Allow forcing the creation of an LV #3877

Conversation

gdemonet commented Sep 26, 2022

bert-e commented Sep 26, 2022

Hello gdemonet,

bert-e commented Sep 26, 2022

Waiting for approval

JBWatenbergScality Sep 26, 2022

Choose a reason for hiding this comment

JBWatenbergScality Sep 26, 2022

Choose a reason for hiding this comment

JBWatenbergScality Sep 26, 2022

Choose a reason for hiding this comment

JBWatenbergScality Sep 26, 2022

Choose a reason for hiding this comment

bert-e commented Sep 30, 2022

Waiting for approval

TeddyAndrieux Oct 5, 2022

Choose a reason for hiding this comment

gdemonet Oct 5, 2022

Choose a reason for hiding this comment

gdemonet Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

bert-e commented Oct 6, 2022

Waiting for approval

TeddyAndrieux left a comment

Choose a reason for hiding this comment

gdemonet commented Oct 7, 2022

bert-e commented Oct 7, 2022

In the queue

bert-e commented Oct 7, 2022

gdemonet Oct 6, 2022 •

edited

Loading