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

support volume expansion #16

Open
lijhnihaoa opened this issue Sep 28, 2022 · 34 comments
Open

support volume expansion #16

lijhnihaoa opened this issue Sep 28, 2022 · 34 comments

Comments

@lijhnihaoa
Copy link

I would like to ask what you think about the high availability of CSI. For example, if SPDK and CSI-Node are restarted at the same time, how can they be restored to the state before the restart.

@cyb70289
Copy link
Contributor

No strong feeling from me. But welcome contributions.

@lijhnihaoa
Copy link
Author

I wonder if storing this information in etcd will affect the impact of the k8s cluster, if it is used in the actual production environment. etcd is generally the best memory in 8G, Too big or too small can easily cause performance degradation.I feel that my question is a bit redundant, but I still want to hear more professional opinions.

@lijhnihaoa
Copy link
Author

If I want to contribute the code of expand volume, what should I do?

@cyb70289
Copy link
Contributor

You can follow the guide in below link to setup gerrit and push code.
https://spdk.io/development/
Please note the guide refers to spdk repo, the spdk-csi repo is at https://review.spdk.io/spdk/spdk-csi.

@lijhnihaoa
Copy link
Author

To https://review.spdk.io/spdk/spdk-csi ! [remote rejected] HEAD -> refs/for/master (commit de65fdf: not Signed-off-by author/committer/uploader in message footer) error: failed to push some refs to 'https://review.spdk.io/spdk/spdk-csi' What should I do?

@cyb70289
Copy link
Contributor

not Signed-off-by author/committer/uploader in message footer

Try git commit --amend -s to add sign-off-by line.

@lijhnihaoa
Copy link
Author

nice

@lijhnihaoa
Copy link
Author

How often submitted code is generally evaluated

@lijhnihaoa lijhnihaoa reopened this Oct 13, 2022
@cyb70289
Copy link
Contributor

I've left comment to your PR yesterday.
Normally there will be response in a week. Not guaranteed if reviewer is busy or simply missed your patch.
If you'd like it to be faster, join spdk slack channel and ping reviewers directly. Comments in gerrit are often ignored by reviewers.
https://spdk.io/community/

@lijhnihaoa
Copy link
Author

thank you . i got it

@cyb70289
Copy link
Contributor

For a major feature, there's often non-trivial feed backs and change request to the original commit.

@lijhnihaoa
Copy link
Author

I think mine is a secondary feature, haha.

@lijhnihaoa
Copy link
Author

root@localhost:~/go/src/spdk-csi# git status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git restore ..." to discard changes in working directory)
modified: pkg/spdk/controllerserver.go
modified: pkg/util/iscsi.go
modified: pkg/util/iscsi_test.go
modified: pkg/util/jsonrpc.go
modified: pkg/util/nvmf.go
modified: pkg/util/nvmf_test.go
root@localhost:/go/src/spdk-csi# make test
=== runnng go mod verify
all modules verified
=== running unit test
? github.com/spdk/spdk-csi/cmd [no test files]
? github.com/spdk/spdk-csi/pkg/csi-common [no test files]
ok github.com/spdk/spdk-csi/pkg/spdk 6.478s coverage: 24.4% of statements
ok github.com/spdk/spdk-csi/pkg/util 1.410s coverage: 47.8% of statements
root@localhost:
/go/src/spdk-csi#

I added functionality for volume expansion and performed unit tests. This indicate that the test passed?

@lijhnihaoa
Copy link
Author

iSCSI:
{\"jsonrpc\":\"2.0\",\"id\":4,\"method\":\"iscsi_get_portal_groups\"} {\"jsonrpc\":\"2.0\",\"id\":4,\"result\":[{\"tag\":1,\"portals\":[{\"host\":\"127.0.0.1\",\"port\":\"3260\"}],\"private\":false}]}\n 127.0.0.1 - - [24/Apr/2023 08:48:30] "POST / HTTP/1.1" 200 - {\"jsonrpc\":\"2.0\",\"id\":5,\"method\":\"iscsi_get_initiator_groups\"} {\"jsonrpc\":\"2.0\",\"id\":5,\"result\":[{\"tag\":1,\"initiators\":[\"ANY\"],\"netmasks\":[\"ANY\"]}]}\n 127.0.0.1 - - [24/Apr/2023 08:48:30] "POST / HTTP/1.1" 200 - {\"jsonrpc\":\"2.0\",\"id\":6,\"method\":\"iscsi_create_target_node\",\"params\":{\"luns\":[{\"lun_id\":0,\"bdev_name\":\"9fda7a14-f428-484d-8018-d67b2123e279\"}],\"name\":\"9fda7a14-f428-484d-8018-d67b2123e279\",\"alias_name\":\"iscsi-9fda7a14-f428-484d-8018-d67b2123e279\",\"pg_ig_maps\":[{\"ig_tag\":1,\"pg_tag\":1}],\"disable_chap\":true,\"queue_depth\":64}} {\"jsonrpc\":\"2.0\",\"id\":6,\"result\":true}\n 127.0.0.1 - - [24/Apr/2023 08:48:30] "POST / HTTP/1.1" 200 - {\"jsonrpc\":\"2.0\",\"id\":7,\"method\":\"iscsi_get_target_nodes\"} {\"jsonrpc\":\"2.0\",\"id\":7,\"result\":[{\"name\":\"iqn.2016-06.io.spdk:9fda7a14-f428-484d-8018-d67b2123e279\",\"alias_name\":\"iscsi-9fda7a14-f428-484d-8018-d67b2123e279\",\"pg_ig_maps\":[{\"pg_tag\":1,\"ig_tag\":1}],\"luns\":[{\"bdev_name\":\"9fda7a14-f428-484d-8018-d67b2123e279\",\"lun_id\":0}],\"queue_depth\":64,\"disable_chap\":true,\"require_chap\":false,\"mutual_chap\":false,\"chap_group\":0,\"header_digest\":false,\"data_digest\":false}]}\n 127.0.0.1 - - [24/Apr/2023 08:48:30] "POST / HTTP/1.1" 200 - {\"jsonrpc\":\"2.0\",\"id\":8,\"method\":\"bdev_lvol_resize\",\"params\":{\"name\":\"9fda7a14-f428-484d-8018-d67b2123e279\",\"size\":1069547520}} {\"jsonrpc\":\"2.0\",\"id\":8,\"result\":true}\n

nvmf:
{\"jsonrpc\":\"2.0\",\"id\":4,\"method\":\"nvmf_create_transport\",\"params\":{\"trtype\":\"TCP\"}} {\"jsonrpc\":\"2.0\",\"id\":4,\"error\":{\"code\":-32603,\"message\":\"Transport type 'TCP' already exists\"}}\n 127.0.0.1 - - [24/Apr/2023 08:48:31] "POST / HTTP/1.1" 200 - {\"jsonrpc\":\"2.0\",\"id\":5,\"method\":\"nvmf_create_subsystem\",\"params\":{\"nqn\":\"nqn.2020-04.io.spdk.csi:uuid:adb8bfad-29ad-41da-b134-37c28bd8f6e9\",\"allow_any_host\":true,\"serial_number\":\"spdkcsi-sn\",\"model_number\":\"adb8bfad-29ad-41da-b134-37c28bd8f6e9\"}} {\"jsonrpc\":\"2.0\",\"id\":5,\"result\":true}\n 127.0.0.1 - - [24/Apr/2023 08:48:31] "POST / HTTP/1.1" 200 - {\"jsonrpc\":\"2.0\",\"id\":6,\"method\":\"nvmf_subsystem_add_ns\",\"params\":{\"nqn\":\"nqn.2020-04.io.spdk.csi:uuid:adb8bfad-29ad-41da-b134-37c28bd8f6e9\",\"namespace\":{\"bdev_name\":\"adb8bfad-29ad-41da-b134-37c28bd8f6e9\"}}} {\"jsonrpc\":\"2.0\",\"id\":6,\"result\":1}\n 127.0.0.1 - - [24/Apr/2023 08:48:31] "POST / HTTP/1.1" 200 - {\"jsonrpc\":\"2.0\",\"id\":7,\"method\":\"nvmf_subsystem_add_listener\",\"params\":{\"nqn\":\"nqn.2020-04.io.spdk.csi:uuid:adb8bfad-29ad-41da-b134-37c28bd8f6e9\",\"listen_address\":{\"trtype\":\"TCP\",\"adrfam\":\"IPv4\",\"traddr\":\"127.0.0.1\",\"trsvcid\":\"4420\"}}} {\"jsonrpc\":\"2.0\",\"id\":7,\"result\":true}\n 127.0.0.1 - - [24/Apr/2023 08:48:31] "POST / HTTP/1.1" 200 - {\"jsonrpc\":\"2.0\",\"id\":8,\"method\":\"nvmf_get_subsystems\"} {\"jsonrpc\":\"2.0\",\"id\":8,\"result\":[{\"nqn\":\"nqn.2014-08.org.nvmexpress.discovery\",\"subtype\":\"Discovery\",\"listen_addresses\":[],\"allow_any_host\":true,\"hosts\":[]},{\"nqn\":\"nqn.2020-04.io.spdk.csi:uuid:adb8bfad-29ad-41da-b134-37c28bd8f6e9\",\"subtype\":\"NVMe\",\"listen_addresses\":[{\"transport\":\"TCP\",\"trtype\":\"TCP\",\"adrfam\":\"IPv4\",\"traddr\":\"127.0.0.1\",\"trsvcid\":\"4420\"}],\"allow_any_host\":true,\"hosts\":[],\"serial_number\":\"spdkcsi-sn\",\"model_number\":\"adb8bfad-29ad-41da-b134-37c28bd8f6e9\",\"max_namespaces\":32,\"min_cntlid\":1,\"max_cntlid\":65519,\"namespaces\":[{\"nsid\":1,\"bdev_name\":\"adb8bfad-29ad-41da-b134-37c28bd8f6e9\",\"name\":\"adb8bfad-29ad-41da-b134-37c28bd8f6e9\",\"nguid\":\"ADB8BFAD29AD41DAB13437C28BD8F6E9\",\"uuid\":\"adb8bfad-29ad-41da-b134-37c28bd8f6e9\"}]},{\"nqn\":\"nqn.2020-04.io.spdk.csi:uuid:88388769-a0ea-4656-a19d-5d4cbcb5fd3c\",\"subtype\":\"NVMe\",\"listen_addresses\":[{\"transport\":\"TCP\",\"trtype\":\"TCP\",\"adrfam\":\"IPv4\",\"traddr\":\"127.0.0.1\",\"trsvcid\":\"4420\"}],\"allow_any_host\":true,\"hosts\":[],\"serial_number\":\"spdkcsi-sn\",\"model_number\":\"88388769-a0ea-4656-a19d-5d4cbcb5fd3c\",\"max_namespaces\":32,\"min_cntlid\":1,\"max_cntlid\":65519,\"namespaces\":[]}]}\n 127.0.0.1 - - [24/Apr/2023 08:48:31] "POST / HTTP/1.1" 200 - {\"jsonrpc\":\"2.0\",\"id\":9,\"method\":\"bdev_lvol_resize\",\"params\":{\"name\":\"adb8bfad-29ad-41da-b134-37c28bd8f6e9\",\"size\":1069547520}} {\"jsonrpc\":\"2.0\",\"id\":9,\"result\":true}\n

@cyb70289
Copy link
Contributor

Not sure of your question. Do you add volume expansion specific unit test? The test itself should verify the function.

@lijhnihaoa
Copy link
Author

Not sure of your question. Do you add volume expansion specific unit test? The test itself should verify the function.

Yes, Can the above output prove that the function is feasible. i think it's ok, bdev_lvol_resize return is true.

@cyb70289
Copy link
Contributor

Honestly, it's hard for me to read the above output.
If you want to make sure the volume size is successfully changed, you can read back and verify the size with bdev_get_bdevs.

@lijhnihaoa
Copy link
Author

lijhnihaoa commented Apr 24, 2023

{"jsonrpc":"2.0","id":9,"method":"bdev_lvol_resize","params":{"name":"adb8bfad-29ad-41da-b134-37c28bd8f6e9","size":1069547520}}
{"jsonrpc":"2.0","id":9,"result":true}\n
127.0.0.1 - - [24/Apr/2023 08:48:31] "POST / HTTP/1.1" 200 -
{"jsonrpc":"2.0","id":10,"method":"bdev_get_bdevs","params":{"name":"adb8bfad-29ad-41da-b134-37c28bd8f6e9"}}
{"jsonrpc":"2.0","id":10,"result":[{"name":"adb8bfad-29ad-41da-b134-37c28bd8f6e9","aliases":["lvs0/csi-13dc1d37-9ff8-485a-b253-1ca453b1bb6e"],"product_name":"Logical Volume","block_size":4096,"num_blocks":261120,"uuid":"adb8bfad-29ad-41da-b134-37c28bd8f6e9","assigned_rate_limits":{"rw_ios_per_sec":0,"rw_mbytes_per_sec":0,"r_mbytes_per_sec":0,"w_mbytes_per_sec":0},"claimed":true,"zoned":false,"supported_io_types":{"read":true,"write":true,"unmap":true,"write_zeroes":true,"flush":false,"reset":true,"compare":false,"compare_and_write":false,"abort":false,"nvme_admin":false,"nvme_io":false},"driver_specific":{"lvol":{"lvol_store_uuid":"4817477f-1a73-4620-b64a-da82c2d036a7","base_bdev":"Malloc0","thin_provision":true,"snapshot":false,"clone":false}}}]}\n

Sorry, I didn't think carefully. The above may be the key information.

@cyb70289
Copy link
Contributor

This looks right. Of course, you should make sure the original size is not the same as resized.

@lijhnihaoa
Copy link
Author

Do I have to add the e2e_test test case before submitting the code?

@cyb70289
Copy link
Contributor

Do I have to add the e2e_test test case before submitting the code?

Yes.

@lijhnihaoa
Copy link
Author

Do I have to add the e2e_test test case before submitting the code?

Yes.

OK

@lijhnihaoa
Copy link
Author

I have a question to ask you, after the success of the volume extension, how to verify the success of it. I want to get the persistent volume size, but I can't find an interface in the framework to get the persistent volume resources. Is there any other way?

@lijhnihaoa
Copy link
Author

# git pull
Already up to date.

What should I do if the submitted code conflicts?

@cyb70289
Copy link
Contributor

Fix the conflict and push again?

@lijhnihaoa
Copy link
Author

The pulled code is the latest. When I submitted it for the first time, I found that the changed Id was the same. I deleted it and submitted it again. Although the submission was successful, the status on the web side showed a merge conflict.

@lijhnihaoa
Copy link
Author

Is it because the change of the person who committed the code before me has not yet passed?

@cyb70289
Copy link
Contributor

You patch is not based on latest tip (95a1911). Pull and rebase again.

@lijhnihaoa
Copy link
Author

Yes, thank you, it's resolved now.

@cyb70289
Copy link
Contributor

cyb70289 commented May 8, 2023

@lijhnihaoa , thanks for the patch. Will you change issue title to something like "support volume expansion" to match current scope.

@lijhnihaoa lijhnihaoa changed the title HA support volume expansion May 14, 2023
@taoyunxing
Copy link

Is the feature of volume expansion available? I need it, even though give me some suggestion are welcome.

@lijhnihaoa
Copy link
Author

https://review.spdk.io/gerrit/c/spdk/spdk-csi/+/17748
You can take a look at this submitted code

@xinydev
Copy link
Contributor

xinydev commented Oct 24, 2023

Hi,@lijhnihaoa are you still working on this?

@lijhnihaoa
Copy link
Author

lijhnihaoa commented Oct 28, 2023 via email

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

No branches or pull requests

4 participants