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

Multus Upgarde 4.15->4.16 - dropping holder design #9924

Merged
merged 12 commits into from
Jul 10, 2024

Conversation

OdedViner
Copy link
Contributor

No description provided.

@OdedViner OdedViner requested a review from a team as a code owner June 10, 2024 09:35
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines label Jun 10, 2024
Signed-off-by: oviner <[email protected]>
@OdedViner OdedViner changed the title WIP: Multus Upgarde 4.15->4.16 - dropping holder design Multus Upgarde 4.15->4.16 - dropping holder design Jun 15, 2024
Signed-off-by: oviner <[email protected]>
Signed-off-by: oviner <[email protected]>
Signed-off-by: oviner <[email protected]>
Signed-off-by: oviner <[email protected]>
Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml
PR Test Suite: deployment
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job PASSED.

Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml conf/ocsci/upgrade.yamlupgrade
PR Test Suite: deployment
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job FAILED (installation failed, tests not executed).

Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml conf/ocsci/upgrade.yaml
PR Test Suite: deployment
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job FAILED (installation failed, tests not executed).

Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml conf/ocsci/upgrade.yaml
PR Test Suite: deployment
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job FAILED (installation failed, tests not executed).

Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml
PR Test Suite: ocp_upgrade or ocs_upgrade
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job FAILED (installation failed, tests not executed).

Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml
PR Test Suite: ocp_upgrade or ocs_upgrade
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job FAILED (installation failed, tests not executed).

Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml
PR Test Suite: ocp_upgrade or ocs_upgrade
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job FAILED (installation failed, tests not executed).

Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml
PR Test Suite: ocp_upgrade or ocs_upgrade
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job FAILED (installation failed, tests not executed).

Signed-off-by: oviner <[email protected]>
Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml
PR Test Suite: ocp_upgrade or ocs_upgrade
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job FAILED (installation failed, tests not executed).

Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml
PR Test Suite: ocp_upgrade or ocs_upgrade
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job PASSED.

Comment on lines 789 to 795
ocs_install_verification(
timeout=600,
skip_osd_distribution_check=True,
ocs_registry_image=upgrade_ocs.ocs_registry_image,
post_upgrade_verification=False,
version_before_upgrade=upgrade_ocs.version_before_upgrade,
)
Copy link
Member

Choose a reason for hiding this comment

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

This can be deleted most likely as we run ocs_install_verification on line 796

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

Comment on lines 4829 to 4847
# nad_obj = OCP(
# kind=constants.NETWORK_ATTACHEMENT_DEFINITION,
# namespace="default",
# resource_name="public-net",
# )
# params = f'{{"spec": {{"config": "a"}}}}'
# nad_obj.patch(params=params, format_type="merge")
# nad_obj = get_network_attachment_definitions(nad_name=nad_name, namespace=namespace)
# nad_config_str = nad_obj.data["spec"]["config"]
# nad_config_dict = json.loads(nad_config_str)
# nad_config_dict["ipam"]["routes"] = [{"dst": config.ENV_DATA["multus_destination_route"]}]
# nad_config_dict_string = json.dumps(nad_config_dict)
# params = f"""[{{ "op": "replace", "path": "/spec/config",
# "value": {str(nad_config_dict_string)}}}]"""
# nad_obj.patch(
# resource_name=nad_name,
# params=params.strip("\n"),
# format_type="json",
# )
Copy link
Member

Choose a reason for hiding this comment

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

? Leftovers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

Signed-off-by: oviner <[email protected]>
petr-balogh
petr-balogh previously approved these changes Jul 3, 2024
AviadP
AviadP previously approved these changes Jul 7, 2024
schedule_nodes([worker_node_name])


def configure_node_network_configuration_policy_on_all_worker_nodes():
Copy link
Contributor

Choose a reason for hiding this comment

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

is this applicable to compact mode clusters, where each node serves as both worker and master?

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 dont know, we dont run multus on compact mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

@OdedViner can you add a comment in the code saying this might require changes for compact mode?

"""
Verify csi holder pods do not exist

Returns:
Copy link
Contributor

Choose a reason for hiding this comment

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

Raises

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

@@ -782,6 +782,10 @@ def run_ocs_upgrade(
# in pending state
is_all_csvs_succeeded = check_all_csvs_are_succeeded(namespace=namespace)
assert is_all_csvs_succeeded, "Not all CSV's are in succeeded state"
if config.ENV_DATA.get("is_multus_enabled"):
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we also check if the target cluster version is 4.16?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are working with branches so we dont need to add condition for ODF version

Copy link
Contributor

Choose a reason for hiding this comment

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

but this will remain the same also in 4.17, where this change won't be required

Copy link
Contributor Author

@OdedViner OdedViner Jul 8, 2024

Choose a reason for hiding this comment

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

I added a configuration file. because we need to test it on ODF4.15->4.16 and on ODF4.16-4.17:

Test case 1:
Deploy OCP4.15+ODF4.15+Multus [cluster-net+public_net, cluster_net_only, public_net_only]
upgrade OCP4.15->16
upgrade ODF4.15->16 [holder pods exist but we don't delete them]
upgrade OCP4.16->17
upgrade ODF4.16-> 17
Run " forcefully disable holder pods"

Test case 2:
Deploy OCP4.15+ODF4.15+Multus [cluster-net+public_net, cluster_net_only, public_net_only]
upgrade OCP4.15->16
upgrade ODF4.15->16
Run " forcefully disable holder pods"
upgrade OCP4.16->17
upgrade ODF4.16-> 17 [holder pods do not exist]

Signed-off-by: oviner <[email protected]>
@OdedViner OdedViner dismissed stale reviews from AviadP and petr-balogh via 4cf0320 July 7, 2024 14:16
@openshift-ci openshift-ci bot removed the lgtm label Jul 7, 2024
OdedViner added 2 commits July 8, 2024 10:41
Signed-off-by: oviner <[email protected]>
Signed-off-by: oviner <[email protected]>
ebenahar
ebenahar previously approved these changes Jul 8, 2024
AviadP
AviadP previously approved these changes Jul 8, 2024
Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml conf/ocsci/multus_delete_csi_holder_pods.yaml
PR Test Suite: ocp_upgrade or ocs_upgrade
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job PASSED.

Signed-off-by: oviner <[email protected]>
@OdedViner OdedViner dismissed stale reviews from AviadP and ebenahar via 0abfcbf July 9, 2024 14:11
@openshift-ci openshift-ci bot removed the lgtm label Jul 9, 2024
Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration: conf/deployment/baremetal/upi_1az_rhcos_multus_nvme_intel_3m_3w.yaml conf/ocsci/multus_delete_csi_holder_pods.yaml
PR Test Suite: ocp_upgrade or ocs_upgrade
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.15
OCS VERSION: 4.15
tested against branch: master

Job PASSED.

@OdedViner OdedViner added the Verified Mark when PR was verified and log provided label Jul 10, 2024
@openshift-ci openshift-ci bot added the lgtm label Jul 10, 2024
Copy link

openshift-ci bot commented Jul 10, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AviadP, ebenahar, OdedViner

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

@ebenahar ebenahar merged commit 058e533 into red-hat-storage:master Jul 10, 2024
6 of 7 checks passed
amr1ta pushed a commit to amr1ta/ocs-ci that referenced this pull request Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm size/L PR that changes 100-499 lines Verified Mark when PR was verified and log provided
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants