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

Mdr upgrade scenario #8699

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open

Conversation

shylesh
Copy link
Contributor

@shylesh shylesh commented Oct 17, 2023

This PR addresses the following

  1. Upgrade of clusters in MDR scenario
  2. DR operator upgrades: MCO and DR hub operator
  3. ACM operator upgrade
  4. Pytest changes for accommodating Multicluster upgrade scenarios

@shylesh shylesh self-assigned this Oct 17, 2023
@shylesh shylesh requested a review from a team as a code owner October 17, 2023 14:54
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines label Oct 17, 2023
@shylesh shylesh added DR Metro and Regional DR related PRs team/ecosystem Ecosystem team related issues/PRs Needs Testing Run tests and provide logs link labels Oct 17, 2023
@shylesh shylesh force-pushed the mdr-upgrade branch 3 times, most recently from b40218f to 834b2c9 Compare October 17, 2023 21:58
tests/conftest.py Outdated Show resolved Hide resolved
@shylesh
Copy link
Contributor Author

shylesh commented Oct 24, 2023

Rebased on latest master

@shylesh shylesh requested a review from a team as a code owner October 24, 2023 19:51
@shylesh shylesh force-pushed the mdr-upgrade branch 3 times, most recently from e06f779 to 5f90af0 Compare October 26, 2023 09:43
@shylesh
Copy link
Contributor Author

shylesh commented Oct 26, 2023

Rebased on latest master

@shylesh shylesh force-pushed the mdr-upgrade branch 4 times, most recently from 85a9521 to 3965bef Compare October 27, 2023 22:40
@shylesh
Copy link
Contributor Author

shylesh commented Nov 6, 2023

Rebased on latest master

(MultiClusterUpgradeParametrize.MULTICLUSTER_UPGRADE_MARKERS)
)
)
and ocsci_config.multicluster_scenario
Copy link
Contributor Author

Choose a reason for hiding this comment

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

change this to ocsci_config.multicluster

@pull-request-size pull-request-size bot added size/XL and removed size/L PR that changes 100-499 lines labels Nov 7, 2023
self.pre_upgrade_images = self.get_pre_upgrade_image(self.csv_name_pre_upgrade)
self.load_version_config_file(self.upgrade_version)

with CephHealthMonitor(self.ceph_cluster):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to instantiate self.ceph_cluster

Copy link
Member

Choose a reason for hiding this comment

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

Suggest dropping out the ceph health monitor for now, as this will require more adjustment in the ceph cluster object and also in the health monitor context manager to enforce it to run on specific cluster and propagate specific kubeconfig for all of underlaying calls.

Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
1. Catsrc name patching issue
2. Format icsp yaml
3. patch channel name

Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
@shylesh
Copy link
Contributor Author

shylesh commented Dec 13, 2024

rebased on the latest master

self.acm_registry_image = config.UPGRADE.get("upgrade_acm_registry_image", "")
self.zstream_upgrade = False

def get_acm_version_before_upgrade(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity, what is the reason to not use directly get_running_acm_version and instead use this wrapper?

I'm not sure, if there might be a situation, where we would like to call this method on other stages of the workflow, but if it will be called after the upgrade, it will return the actual upgraded version, not the version before upgrade as suggested by the name. Or did I miss anything there?

(MultiClusterUpgradeParametrize.MULTICLUSTER_UPGRADE_MARKERS)
)
)
and ocsci_config.multicluster
Copy link
Contributor

Choose a reason for hiding this comment

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

this part of the condition and ocsci_config.multicluster is probably not necessary here as it is also part of the condition on line 479:

if ocsci_config.multicluster and ocsci_config.UPGRADE.get("upgrade", False):

dahorak
dahorak previously approved these changes Dec 17, 2024
Copy link
Contributor

@dahorak dahorak left a comment

Choose a reason for hiding this comment

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

I have just two questions mentioned above (but they are more questions or notes, not thinks which have to be necessarily fixed).
I don't have so deep knowledge in the existing upgrade automation, so I'm not able to easily distinguish some possible technical gaps if there are any, so overall LGTM.

dahorak
dahorak previously approved these changes Dec 19, 2024
@openshift-ci openshift-ci bot added the lgtm label Dec 19, 2024
Copy link

openshift-ci bot commented Dec 20, 2024

New changes are detected. LGTM label has been removed.

Copy link

openshift-ci bot commented Dec 20, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shylesh

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
Copy link

openshift-ci bot commented Dec 20, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shylesh

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DR Metro and Regional DR related PRs Needs Testing Run tests and provide logs link size/XXL team/ecosystem Ecosystem team related issues/PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants