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.

Sign up for GitHub

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

Jump to bottom

Multicluster configuration context management #4875

Merged
merged 12 commits into from Oct 28, 2021
Merged

Multicluster configuration context management #4875

merged 12 commits into from Oct 28, 2021

Conversation

shylesh
Copy link
Contributor

@shylesh shylesh commented Sep 23, 2021

No description provided.

@shylesh shylesh requested a review from a team as a code owner September 23, 2021 06:35
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines label Sep 23, 2021
@shylesh shylesh force-pushed the ACM-UI-DEPLOYMENT branch 2 times, most recently from f4f2674 to cbc36e8 Compare October 4, 2021 18:33
@shylesh
Copy link
Contributor Author

shylesh commented Oct 4, 2021

Rebased on latest master

@shylesh shylesh self-assigned this Oct 5, 2021
@shylesh shylesh added the Needs Testing Run tests and provide logs link label Oct 5, 2021
@shylesh shylesh force-pushed the ACM-UI-DEPLOYMENT branch 2 times, most recently from 4d34ea5 to c744212 Compare October 7, 2021 20:09
@shylesh
Copy link
Contributor Author

shylesh commented Oct 7, 2021

Rebased on latest master

@shylesh
Copy link
Contributor Author

shylesh commented Oct 11, 2021

Regression:
vsphere: https://ocs4-jenkins-csb-ocsqe.apps.ocp4.prod.psi.redhat.com/job/qe-deploy-ocs-cluster/6496
aws: https://ocs4-jenkins-csb-ocsqe.apps.ocp4.prod.psi.redhat.com/job/qe-deploy-ocs-cluster/6497
azure: https://ocs4-jenkins-csb-ocsqe.apps.ocp4.prod.psi.redhat.com/job/qe-deploy-ocs-cluster/6500

@pull-request-size pull-request-size bot added size/XL and removed size/L PR that changes 100-499 lines labels Oct 13, 2021
clacroix12
clacroix12 reviewed Oct 13, 2021
View reviewed changes
ocs_ci/framework/__init__.py Outdated Show resolved Hide resolved
ocs_ci/framework/__init__.py Outdated Show resolved Hide resolved
ocs_ci/framework/tests/test_config.py Show resolved Hide resolved
@shylesh shylesh force-pushed the ACM-UI-DEPLOYMENT branch 2 times, most recently from 086b118 to 5e758b3 Compare October 14, 2021 03:58
@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines size/XL and removed size/XL size/L PR that changes 100-499 lines labels Oct 14, 2021
@shylesh
Copy link
Contributor Author

shylesh commented Oct 14, 2021

Resolved conflicts and rebased on latest master

@shylesh
Copy link
Contributor Author

shylesh commented Oct 15, 2021

Rebased on latest master

@shylesh
Copy link
Contributor Author

shylesh commented Oct 18, 2021

rebased on latest master

@shylesh shylesh changed the title WIP: DNM: Multicluster configuration context management Multicluster configuration context management Oct 18, 2021
clacroix12
clacroix12 reviewed Oct 19, 2021
View reviewed changes
ocs_ci/framework/main.py Outdated
f"--cluster{i+1}",
required=True,
action="store_true",
help=f"cluster{i}-conf",
Copy link
Contributor

Choose a reason for hiding this comment

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

This help text looks incorrect. Shouldn't this describe that this is the start of the cluster{i} specific CLI arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the help text.

ocs_ci/framework/main.py Show resolved Hide resolved
ocs_ci/framework/main.py Outdated
Comment on lines 189 to 192
for i in range(nclusters):
framework.config.switch_ctx(i)
process_ocsci_conf(common_argv + multicluster_conf[i][1:])
for j in range(len(multicluster_conf[i][1:])):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you think of more descriptive variables than i and j?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

ocs_ci/framework/main.py Outdated
Comment on lines 244 to 256
arguments.extend(
[
"-p",
"ocs_ci.framework.pytest_customization.ocscilib",
"-p",
"ocs_ci.framework.pytest_customization.marks",
"-p",
"ocs_ci.framework.pytest_customization.reports",
"--logger-logsdir",
pytest_logs_dir,
"--rp-launch",
launch_name,
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to do this per cluster? These seem like common arguments to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now we are iterating over all cluster, I haven't thought about launch_name or any other parameters pertaining to report portal and how this needs to be handled. I guess current report portal parameters should be generated as is for reporting because we only refer to first cluster's report portal config. Am I missing something here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved report related lines as common lines.

petr-balogh
petr-balogh reviewed Oct 20, 2021
View reviewed changes
docs/usage.md Outdated
If you would like to run multicluster environment tests and deployments, use `multicluster` subcommand for run-ci
```bash
run-ci multicluster 2/
tests/ -m tier1 \
Copy link
Member

Choose a reason for hiding this comment

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

Let's add also common --ocsci-conf and note that if it's defined the front of --cluster1 that it's loaded to both configs contexts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@shylesh
WIP: multicluster configuration
3004e06 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
CLI parsing changes for multicluster
a059f45 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
Fix tox issues
577a8b4 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
fix single cluster config handling
439f77a 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
Fix collectonly failure issues
d11b20c 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
simple unit test for config validation
7bf9717 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
add looping after conflict resolution
cea6e22 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
Addressing review comments by Coady and Petr
9783972 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
Unit tests for multicluster
547b444 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
Add documentation for multicluster subcommand
b164743 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
Copy link
Contributor Author

shylesh commented Oct 25, 2021

Rebased on latest master

@shylesh
Addressing Coady's comments: 1. Rename the temp variables
8c4d153 
2. Seperate common code for report generation

Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
@shylesh
Add documentation example for passing common config to all clusters
f0cc954 
Signed-off-by: Shylesh Kumar Mohan <shmohan@redhat.com>
clacroix12
clacroix12 approved these changes Oct 27, 2021
View reviewed changes
Copy link
Contributor

@clacroix12 clacroix12 left a comment

Choose a reason for hiding this comment

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

LGTM

@shylesh
Copy link
Contributor Author

shylesh commented Oct 28, 2021

Ran another round of regression:
vsphere: https://ocs4-jenkins-csb-ocsqe.apps.ocp4.prod.psi.redhat.com/job/qe-deploy-ocs-cluster/7123/
AWS: https://ocs4-jenkins-csb-ocsqe.apps.ocp4.prod.psi.redhat.com/job/qe-deploy-ocs-cluster/7076
Azure: https://ocs4-jenkins-csb-ocsqe.apps.ocp4.prod.psi.redhat.com/job/qe-deploy-ocs-cluster/7082/

@shylesh shylesh added Verified Mark when PR was verified and log provided and removed Needs Testing Run tests and provide logs link labels Oct 28, 2021
@vasukulkarni vasukulkarni merged commit 2e29f94 into red-hat-storage:master Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clacroix12 clacroix12 clacroix12 approved these changes

@vasukulkarni vasukulkarni vasukulkarni approved these changes

@petr-balogh petr-balogh petr-balogh approved these changes

Assignees

@shylesh shylesh

Labels
size/XL Verified Mark when PR was verified and log provided
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@shylesh @clacroix12 @vasukulkarni @petr-balogh