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

Can we remove the hardcoded clusterloader2 folder name appended to $ARTIFACTS in --report-dir ? #151

Closed
piyushgupta1551 opened this issue Jun 30, 2021 · 3 comments
Labels
priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/testing Categorizes an issue or PR as relevant to SIG Testing. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@piyushgupta1551
Copy link
Contributor

Current situation -

  • when a perf test is executed using kubetest2-tester-clusterloader2 via a prow job, the results of the test are created in $artifacts/clusterloader2 which cannot be read by perfdash (perfdash looks for results inside the artifacts folder only)
  • Upon reading further, it seems that the concatenation of clusterloader2 to $artifacts is hardcoded and cannot be bypassed in any way and --report-dir is not longer a valid argument for the clusterloader2 tester.
    Refer -
    "--report-dir=" + filepath.Join(os.Getenv("ARTIFACTS"), "clusterloader2"),

Why is this required?
We are trying to execute these perf tests on ppc64le arch Cloud VMs and have perfdash read the test result data.
With the mandatory clusterloader2 folder being created inside the $arifatcs folder, perfdash is no longer able to read test data.

I understand this may have been done originally with a purpose but wanted to check if we can make this concatenation optional so that someone can bypass the clusterloader folder creation inside $ARTIFACTS folder if and when required?

Note: I could not find a way to bypass this but please let me know if there is any way to do so in the existing code.

@piyushgupta1551 piyushgupta1551 changed the title Can we remove the hardcoded clusterloader2 folder name appended to report-dir (Artifacts directory) ? Can we remove the hardcoded clusterloader2 folder name appended in report-dir (Artifacts directory) ? Jun 30, 2021
@piyushgupta1551 piyushgupta1551 changed the title Can we remove the hardcoded clusterloader2 folder name appended in report-dir (Artifacts directory) ? Can we remove the hardcoded clusterloader2 folder name appended to $ARTIFACTS in --report-dir ? Jun 30, 2021
@amwat
Copy link
Contributor

amwat commented Jul 8, 2021

I don't have any strong objections. IIRC I don't think there was a specific reason for it to be named under sub directory that way. That being said I'm not sure if anything should assume that all generated artifacts would be in the top-level directory in the first place. But if perfdash is expecting in such a way I think that's a sane default.

I think we should solve it by doing both i.e. change the default to be top-level $ARTIFACTS and also expose the flag on the --report-dir clusterloader2 tester.

/triage accepted
/priority important-soon

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jul 8, 2021
@piyushgupta1551
Copy link
Contributor Author

Tx @amwat . Should I go ahead and raise the PR for these changes? Will remove the clusterloader2 folder name (make $ARTIFACTS folder as default) and will enable --report-dir as an arg to the tester.

@amwat
Copy link
Contributor

amwat commented Jul 8, 2021

That sounds great! Thank you

@spiffxp spiffxp added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/testing Categorizes an issue or PR as relevant to SIG Testing. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

4 participants