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

[BUG]: "--attr" of ephemeral-volume performance test doesn't support properties file #930

Closed
vincent1chen opened this issue Aug 3, 2023 · 7 comments
Assignees
Labels
area/cert-csi Issues pertaining to Cert CSI type/bug Something isn't working. This is the default label associated with a bug issue.
Milestone

Comments

@vincent1chen
Copy link

Bug Description

when use attributes file to "--attr" of ephemeral-volume performance test , I see the script panic. but same function test of "--attr" support attributes file. and the doc also show attributes file as example. it didn't note user that performance test can't use it.

to run performance, there only have to attach key/value to "--attr"

./cert-csi test ephemeral-volume --driver csi-powerstore.dellemc.com --fs-type ext4 --pods 5 --pod-name wrephe --attr arrayId=PS6096ea15a0b0 --attr protocol=ISCSI --attr size=20Gi --driver-ns csi-powerstore -ns csi-powerstore

[2023-08-03 10:28:26] INFO Starting cert-csi; ver. 0.8.1
[2023-08-03 10:28:26] INFO Using EVENT observer type
[2023-08-03 10:28:26] INFO Using default config
[2023-08-03 10:28:26] INFO Successfully loaded config. Host: https://192.168.30.2:6443
[2023-08-03 10:28:26] INFO Created new KubeClient
[2023-08-03 10:28:26] INFO Creating 5 pods, each with 1 volumes
... ...

function test:

cat ephemeral-config.properties

arrayId=PS6096ea15a0b0
protocol=ISCSI
size=20Gi

[root@registry ~]# ./cert-csi functional-test ephemeral-volume --driver csi-powerstore.dellemc.com --fs-type ext4 --attr ephemeral-config.properties --pods 5 --pod-name wrephe
[2023-08-03 09:55:08] INFO Starting cert-csi; ver. 0.8.1
[2023-08-03 09:55:08] INFO Using EVENT observer type
[2023-08-03 09:55:08] INFO Using default config
[2023-08-03 09:55:08] INFO Successfully loaded config. Host: https://192.168.30.2:6443
[2023-08-03 09:55:09] INFO Created new KubeClient
[2023-08-03 09:55:09] INFO Creating 5 pods, each with 1 volumes
... ...

Logs

]# ./cert-csi test ephemeral-volume --driver csi-powerstore.dellemc.com --fs-type ext4 --pods 5 --pod-name wrephe --attr ephemeral-config.properties --driver-ns csi-powerstore -ns csi-powerstore
[2023-08-03 10:04:11] INFO Starting cert-csi; ver. 0.8.1
[2023-08-03 10:04:11] ERROR Parameter ephemeral-config.properties incorrect.
panic: runtime error: index out of range [1] with length 1

goroutine 1 [running]:
cert-csi/pkg/cmd.getEphemeralCreationCommand.func1(0xc0000f4dc0)
/root/cert-csi/pkg/cmd/testcmd.go:960 +0x49c
github.com/urfave/cli.HandleAction({0x2143c00?, 0x26746f0?}, 0x10?)
/root/go/pkg/mod/github.com/urfave/cli@v1.22.2/app.go:523 +0x50
github.com/urfave/cli.Command.Run({{0x25548a8, 0x10}, {0x254546e, 0x9}, {0x0, 0x0, 0x0}, {0x2565c09, 0x18}, {0x0, ...}, ...}, ...)
/root/go/pkg/mod/github.com/urfave/cli@v1.22.2/command.go:174 +0x67b
github.com/urfave/cli.(*App).RunAsSubcommand(0xc0004de540, 0xc0000f4b00)
/root/go/pkg/mod/github.com/urfave/cli@v1.22.2/app.go:404 +0xe87
github.com/urfave/cli.Command.startApp({{0x2539a1a, 0x4}, {0x0, 0x0}, {0x0, 0x0, 0x0}, {0x2552c8e, 0xf}, {0x0, ...}, ...}, ...)
/root/go/pkg/mod/github.com/urfave/cli@v1.22.2/command.go:373 +0xb7f
github.com/urfave/cli.Command.Run({{0x2539a1a, 0x4}, {0x0, 0x0}, {0x0, 0x0, 0x0}, {0x2552c8e, 0xf}, {0x0, ...}, ...}, ...)
/root/go/pkg/mod/github.com/urfave/cli@v1.22.2/command.go:102 +0x865
github.com/urfave/cli.(*App).Run(0xc0004de1c0, {0xc000130000, 0x11, 0x12})
/root/go/pkg/mod/github.com/urfave/cli@v1.22.2/app.go:276 +0xbc7
main.main()
/root/cert-csi/cmd/cert-csi/main.go:95 +0xa85

Screenshots

No response

Additional Environment Information

No response

Steps to Reproduce

create attributes file firstly and use it in ephemeral-volume performance test

./cert-csi test ephemeral-volume --driver csi-powerstore.dellemc.com --fs-type ext4 --pods 5 --pod-name wrephe --attr ephemeral-config.properties --driver-ns csi-powerstore -ns csi-powerstore

Expected Behavior

the performance test of ephemeral-volume case could support attribute file same as function test.

CSM Driver(s)

CSI powerstore

Installation Type

helm 3.0

Container Storage Modules Enabled

No response

Container Orchestrator

K8s 1.24.4

Operating System

debian 11

@vincent1chen vincent1chen added needs-triage Issue requires triage. type/bug Something isn't working. This is the default label associated with a bug issue. labels Aug 3, 2023
@csmbot
Copy link
Collaborator

csmbot commented Aug 3, 2023

@vincent1chen: Thank you for submitting this issue!

The issue is currently awaiting triage. Please make sure you have given us as much context as possible.

If the maintainers determine this is a relevant issue, they will remove the needs-triage label and assign an appropriate priority label.


We want your feedback! If you have any questions or suggestions regarding our contributing process/workflow, please reach out to us at container.storage.modules@dell.com.

@adarsh-dell adarsh-dell self-assigned this Aug 4, 2023
@adarsh-dell
Copy link
Contributor

Hi @vincent1chen , Thanks for reporting this issue.

Based on the documentation, it appears that only the "functional-test" command accepts a file name as an argument for the "attribute" attribute.

image

image

./cert-csi functional-test ephemeral-volume --driver csi-powerstore.dellemc.com --fs-type NFS --pods 5 --pod-name wrephe --attr ephemeral-config.properties -ns csi-powerstore

I have successfully executed the above commands, and it seems like they also worked for you, as you mentioned in your initial statement.

Now, regarding the command you provided, I tested it out, and I encountered the same issue you mentioned. After investigation, I found that the "cert-csi" tool expects users to pass arguments in the exact format you specified, and it seems you have correctly identified the root cause of the issue as the test suite is not expecting user to pass the file name.

Before proceeding with fixing this problem, I'd like to clarify your requirement:

Do you want to pass the file name as "--attr" argument only, or do you also want to support both formats?
Since I need to assess the consequences of accommodating both formats in the "certify" command, I would like to understand the potential impact of this change.

@vincent1chen
Copy link
Author

@adarsh-dell , i just realize both test case should support same argument format. otherwise it's easy to confuse user. regarding to the format, I am ok for both. if there has problem to code it, I prefer to file name.

@adarsh-dell adarsh-dell added the area/cert-csi Issues pertaining to Cert CSI label Aug 8, 2023
@adarsh-dell
Copy link
Contributor

Hi @vincent1chen We appreciate your input.

After consulting with others, we've determined that using the "fileName" option for both the "test" and "function-test" commands is more straightforward. Therefore, we're actively working on a solution to maintain consistent argument formatting across both suites.

Regards,
Adarsh

@vincent1chen vincent1chen changed the title [BUG]: "--attr" of ephemeral-volume performance test doesn't support properties file [BUG][cert-csi] : "--attr" of ephemeral-volume performance test doesn't support properties file Aug 9, 2023
@adarsh-dell
Copy link
Contributor

Hi @vincent1chen I hope this is what you are exaclty looking for?
./cert-csi test ephemeral-volume --driver csi-powerstore.dellem c.com --fs-type ext4 --attr attr-file --pods 5 --pod-name wrephe
image

@adarsh-dell adarsh-dell added this to the v1.8.0 milestone Aug 9, 2023
@vincent1chen
Copy link
Author

@adarsh-dell , yes that is it. you fix it:-) thanks

@adarsh-dell
Copy link
Contributor

Closing since the fix is as per the expectations and will be available from CSM v1.8.0 release.

@adarsh-dell adarsh-dell removed the needs-triage Issue requires triage. label Aug 9, 2023
@shaynafinocchiaro shaynafinocchiaro changed the title [BUG][cert-csi] : "--attr" of ephemeral-volume performance test doesn't support properties file [BUG]: "--attr" of ephemeral-volume performance test doesn't support properties file Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cert-csi Issues pertaining to Cert CSI type/bug Something isn't working. This is the default label associated with a bug issue.
Projects
None yet
Development

No branches or pull requests

3 participants