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

Add support for raw (non-formatted) volume #2651

Merged
merged 23 commits into from
Jul 9, 2020

Conversation

slaperche-scality
Copy link
Contributor

@slaperche-scality slaperche-scality commented Jun 29, 2020

Component:

operator, salt

Context:

We want to support the provisioning of non-formatted block device.

Summary:

Add a new parameter noFormat in the Volume CRD and update Salt code to adapt the provisioning accordingly.

Acceptance criteria:

Tests are passing (sparseLoopDevice)
Manual testing (rawBlockDevice)


Closes: #2421, #2963

@slaperche-scality slaperche-scality requested a review from a team as a code owner June 29, 2020 15:32
@bert-e
Copy link
Contributor

bert-e commented Jun 29, 2020

Hello slaperche-scality,

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
Copy link
Contributor

bert-e commented Jun 29, 2020

Conflict

A conflict has been raised during the creation of
integration branch w/2.6/feature/2421-add-support-for-raw-block-volume with contents from feature/2421-add-support-for-raw-block-volume
and development/2.6.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.6/feature/2421-add-support-for-raw-block-volume origin/development/2.6
 $ git merge origin/feature/2421-add-support-for-raw-block-volume
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.6/feature/2421-add-support-for-raw-block-volume

@slaperche-scality
Copy link
Contributor Author

Will do some more manual testing, but code is ready for review

Refs: #2615
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
Having a set of "KEP-style" docs is too heavy for our needs.

Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
This options allows to skip the formatting of the device.
It's optional and will be set to false by default (i.e., format the
device) to preserve the backward compatibility.

Refs: #2421
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
Refs: #2421
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
@slaperche-scality slaperche-scality force-pushed the feature/2421-add-support-for-raw-block-volume branch 2 times, most recently from ed7c3f1 to 2265621 Compare June 30, 2020 12:49
Copy link
Contributor

@gdemonet gdemonet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some early comments, most of it looks good to me :)

salt/metalk8s/volumes/prepared/init.sls Show resolved Hide resolved
docs/developer/architecture/volume.rst Outdated Show resolved Hide resolved
salt/_modules/metalk8s_volumes.py Outdated Show resolved Hide resolved
Refs: #2421
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
Refs: #2421
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
@slaperche-scality slaperche-scality force-pushed the feature/2421-add-support-for-raw-block-volume branch 4 times, most recently from dd66215 to 6b589f0 Compare July 1, 2020 06:49
@slaperche-scality
Copy link
Contributor Author

/reset

@bert-e
Copy link
Contributor

bert-e commented Jul 1, 2020

Reset complete

I have successfully deleted this pull request's integration branches.

@bert-e
Copy link
Contributor

bert-e commented Jul 1, 2020

Conflict

A conflict has been raised during the creation of
integration branch w/2.6/feature/2421-add-support-for-raw-block-volume with contents from feature/2421-add-support-for-raw-block-volume
and development/2.6.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.6/feature/2421-add-support-for-raw-block-volume origin/development/2.6
 $ git merge origin/feature/2421-add-support-for-raw-block-volume
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.6/feature/2421-add-support-for-raw-block-volume

@slaperche-scality slaperche-scality force-pushed the feature/2421-add-support-for-raw-block-volume branch 3 times, most recently from 807172b to 2ec24f5 Compare July 1, 2020 15:04
@slaperche-scality
Copy link
Contributor Author

/reset

@slaperche-scality slaperche-scality force-pushed the feature/2421-add-support-for-raw-block-volume branch from 2ec24f5 to 2355d00 Compare July 1, 2020 15:14
@bert-e
Copy link
Contributor

bert-e commented Jul 1, 2020

Reset complete

I have successfully deleted this pull request's integration branches.

@bert-e
Copy link
Contributor

bert-e commented Jul 1, 2020

Conflict

A conflict has been raised during the creation of
integration branch w/2.6/feature/2421-add-support-for-raw-block-volume with contents from feature/2421-add-support-for-raw-block-volume
and development/2.6.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.6/feature/2421-add-support-for-raw-block-volume origin/development/2.6
 $ git merge origin/feature/2421-add-support-for-raw-block-volume
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.6/feature/2421-add-support-for-raw-block-volume

@slaperche-scality slaperche-scality force-pushed the feature/2421-add-support-for-raw-block-volume branch 3 times, most recently from 12fd228 to c131560 Compare July 1, 2020 23:58
@slaperche-scality slaperche-scality force-pushed the feature/2421-add-support-for-raw-block-volume branch 6 times, most recently from 746d3d2 to 1ce9bb9 Compare July 3, 2020 21:28
This makes Volume closer to the underlying PersistentVolume (which
simplify the code and also helps the user already familiar with
PersistentVolume).

Refs: #2421
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
Will be less confusing.

Refs: #2421
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
By using sgdisk we can change the partition GUID to match the volume
UUID. Haven't found how to do it with parted…

Refs: #2421
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
In some case, this trigger an infinite reconciliation loop…

Refs: #2421
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
@slaperche-scality slaperche-scality force-pushed the feature/2421-add-support-for-raw-block-volume branch from 1ce9bb9 to b0eca3d Compare July 6, 2020 11:35
@slaperche-scality
Copy link
Contributor Author

Ready for a new round of review, I've applied all the requested changes and tested manually the different cases for Block mode (whole disk, partition and LVM volume) and they work.

That being said, unlike Filesystem mode, we now some distinct codepaths for Block mode (partition and LVM cases are specific to rawBlockDevice): at one point, we may want to have tests in the CI for rawBlockDevice as well.

@slaperche-scality
Copy link
Contributor Author

/reset

@bert-e
Copy link
Contributor

bert-e commented Jul 8, 2020

Reset complete

I have successfully deleted this pull request's integration branches.

@bert-e
Copy link
Contributor

bert-e commented Jul 8, 2020

Conflict

A conflict has been raised during the creation of
integration branch w/2.6/feature/2421-add-support-for-raw-block-volume with contents from feature/2421-add-support-for-raw-block-volume
and development/2.6.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.6/feature/2421-add-support-for-raw-block-volume origin/development/2.6
 $ git merge origin/feature/2421-add-support-for-raw-block-volume
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.6/feature/2421-add-support-for-raw-block-volume

@bert-e
Copy link
Contributor

bert-e commented Jul 8, 2020

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:

The following reviewers are expecting changes from the author, or must review again:

Copy link
Contributor

@alexandre-allard alexandre-allard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

salt/_modules/metalk8s_volumes.py Outdated Show resolved Hide resolved
Refs: #2421
Signed-off-by: Sylvain Laperche <sylvain.laperche@scality.com>
@slaperche-scality
Copy link
Contributor Author

/approve

@bert-e
Copy link
Contributor

bert-e commented Jul 8, 2020

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:

The following reviewers are expecting changes from the author, or must review again:

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Jul 8, 2020

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/2.5

  • ✔️ development/2.6

The following branches will NOT be impacted:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.3
  • development/2.0
  • development/2.1
  • development/2.2
  • development/2.3
  • development/2.4

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
Copy link
Contributor

bert-e commented Jul 9, 2020

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

  • ✔️ development/2.5

  • ✔️ development/2.6

The following branches have NOT changed:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.3
  • development/2.0
  • development/2.1
  • development/2.2
  • development/2.3
  • development/2.4

Please check the status of the associated issue None.

Goodbye slaperche-scality.

@bert-e bert-e merged commit b8f8cc5 into development/2.5 Jul 9, 2020
@bert-e bert-e deleted the feature/2421-add-support-for-raw-block-volume branch July 9, 2020 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants