Added mce.py file for mce installation related methods #11038
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: amr1ta 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 |
ocs_ci/deployment/hosted_cluster.py
Outdated
| @@ -477,6 +488,8 @@ def deploy_dependencies( | |||
| self.deploy_lb() | |||
| if download_hcp_binary: | |||
| self.update_hcp_binary() | |||
| if deploy_mce: | |||
There was a problem hiding this comment.
we should probably check, if deploy_acm_hub: True, leaving ACM as a general option
| if deploy_mce: | |
| if deploy_mce and not deploy_acm_hub: |
There was a problem hiding this comment.
Addressed in latest commit.
ocs_ci/deployment/mce.py
Outdated
| Creates a catalogsource for mce operator. | ||
|
|
||
| """ | ||
| logger.info("Adding CatalogSource for MCE") |
There was a problem hiding this comment.
we need to check if MCE catalogSource is not already installed. This way we insure, the Deployment can be rerun again from any installation stage.
In opposite if we do not do this check, job will abort with error
There was a problem hiding this comment.
Addressed in latest commit.
ocs_ci/deployment/mce.py
Outdated
| Raises: | ||
| CommandFailed: If the 'oc create' command fails. | ||
| """ | ||
| try: |
There was a problem hiding this comment.
it is better to check if resource exist than using try-except, to not pollute logs with false-errors
There was a problem hiding this comment.
Addressed in latest commit.
ocs_ci/deployment/mce.py
Outdated
| """ | ||
| operatorgroup_yaml_file = templating.load_yaml(constants.MCE_OPERATOR_YAM) | ||
| operatorgroup_yaml = OCS(**operatorgroup_yaml_file) | ||
| try: |
There was a problem hiding this comment.
it is better to check if resource exist than using try-except, to not pollute logs with false-errors
There was a problem hiding this comment.
Addressed in latest commit.
| mce_subscription_yaml_data, mce_subscription_manifest.name | ||
| ) | ||
| logger.info("Creating subscription for mce operator") | ||
| run_cmd(f"oc create -f {mce_subscription_manifest.name}") |
There was a problem hiding this comment.
please check if subscription exists in the beginning of the method; if exists skip execution of the method
There was a problem hiding this comment.
Addressed in latest commit.
There was a problem hiding this comment.
Hi Amrita. In case if subscription exists we will need to return True or something but not execute creation of the subscription second time.
I see you checked it here, but there is no else in this block, so it will be executed in any way
if not self.subs.is_exist(resource_name=constants.MCE_OPERATOR):
mce_subscription_yaml_data = templating.load_yaml(
constants.MCE_SUBSCRIPTION_YAML
)
ocs_ci/deployment/mce.py
Outdated
| "-n {self.namespace}" | ||
| ) | ||
| cmd_res = exec_cmd(cmd, shell=True) | ||
| assert cmd_res.returncode == 0 |
There was a problem hiding this comment.
overall it its better not use assert in deployments, but in tests. This way we'll have errors in log showing that deployment failed, instead of assertion signaling about a product bug
There was a problem hiding this comment.
Addressed in latest commit.
ocs_ci/deployment/mce.py
Outdated
| "]}}}' --type=merge" | ||
| ) | ||
| cmd_res = exec_cmd(cmd, shell=True) | ||
| assert cmd_res.returncode == 0 |
There was a problem hiding this comment.
same as above, assertion on this would not mean Product bug;
we have module with ocs-ci exceptions ocs_ci.ocs.exceptions, please use existing or add another with description
There was a problem hiding this comment.
Addressed in latest commit.
ocs_ci/deployment/mce.py
Outdated
| namespace=constants.HYPERSHIFT_NAMESPACE, | ||
| ) | ||
|
|
||
| assert configmaps_obj.is_exist( |
There was a problem hiding this comment.
we have module with ocs-ci exceptions ocs_ci.ocs.exceptions, please use existing or add another with description instead of using assert
There was a problem hiding this comment.
Addressed in latest commit.
| @@ -386,6 +390,7 @@ def deploy_ocp( | |||
| deploy_acm_hub=True, | |||
| deploy_metallb=True, | |||
| download_hcp_binary=True, | |||
| deploy_mce=True, | |||
There was a problem hiding this comment.
we will need introduce this new parameter in configurations and .md file.
There was a problem hiding this comment.
Addressed in latest commit.
amr1ta
force-pushed
the
create-hcp-cluster-without-acm
branch
2 times, most recently
from
a42dd98 to
d372a22
Compare
…btest to check mce installation and hcp cluster creation without acm operator Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
amr1ta
force-pushed
the
create-hcp-cluster-without-acm
branch
from
c26ce41 to
c57871e
Compare
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
Signed-off-by: Amrita Mahapatra <[email protected]>
amr1ta
force-pushed
the
create-hcp-cluster-without-acm
branch
from
19c4efe to
e68de9c
Compare
Signed-off-by: Amrita Mahapatra <[email protected]>
amr1ta
force-pushed
the
create-hcp-cluster-without-acm
branch
from
e68de9c to
2250b23
Compare
Added mce.py file for mce installation related methods and created libtest to check mce installation and hcp cluster creation without acm operator