Create hosted cluster deploy - destroy fixtures #9957
Conversation
DanielOsypenko
commented
Jun 17, 2024
DanielOsypenko
commented
- added fixture to create Hosted cluster and add configuration to multicluster job
- added fixture that abruptly destroys hosted clusters (leaving OCS resources)
DanielOsypenko
force-pushed
the
create_hosted_cluster_destroy_hosted_cluster_fixtures
branch
from
ba9942b to
266ac11
Compare
pull-request-size
bot
added
size/XL
and removed
size/L
DanielOsypenko
force-pushed
the
create_hosted_cluster_destroy_hosted_cluster_fixtures
branch
from
d0ba084 to
c9df3a7
Compare
DanielOsypenko
changed the title
| Returns: | ||
| str: random cluster name | ||
| """ | ||
| # getting the cluster name from the env data, fo instance "ibm_cloud_baremetal3; mandatory conf field" |
There was a problem hiding this comment.
| # getting the cluster name from the env data, fo instance "ibm_cloud_baremetal3; mandatory conf field" | |
| # getting the cluster name from the env data, for instance "ibm_cloud_baremetal3; mandatory conf field" |
ocs_ci/deployment/hosted_cluster.py
Outdated
| Check if the storage client installation was requested in the config | ||
|
|
||
| Args: | ||
| cluster_name: str: Name of the cluster |
There was a problem hiding this comment.
You can add Returns section to the docstring
ocs_ci/utility/utils.py
Outdated
|
|
||
| class CustomJSONEncoder(json.JSONEncoder): | ||
| """ | ||
| Custom JSON encoder to handle set objects |
There was a problem hiding this comment.
Please describe in docstring what this custom json encoder does differently and maybe change its name to reflect it. Also please consider if it should be in its own file (e.g. ocs_ci/utility/json.py).
ocs_ci/utility/utils.py
Outdated
| return super().default(obj) | ||
|
|
||
|
|
||
| def get_odf_tag_from_redhat_catsrc(): |
There was a problem hiding this comment.
It would be better to have this function in ocs/resources/catalog_source.py.
There was a problem hiding this comment.
moved, thanks
| self, create_hypershift_clusters, destroy_hosted_cluster | ||
| ): | ||
| """ | ||
| Test create hosted cluster with fixture |
There was a problem hiding this comment.
Docstring is identical to the one of the previous test. I'd write something like "create and destroy hosted cluster", to avoid confusion.
DanielOsypenko
force-pushed
the
create_hosted_cluster_destroy_hosted_cluster_fixtures
branch
from
a3a9072 to
0338af9
Compare
Signed-off-by: Daniel Osypenko <[email protected]>
Signed-off-by: Daniel Osypenko <[email protected]>
DanielOsypenko
dismissed stale reviews from ebenahar, fbalak, and ebondare
via
4d121c6
DanielOsypenko
force-pushed
the
create_hosted_cluster_destroy_hosted_cluster_fixtures
branch
from
39f5653 to
4d121c6
Compare
| # getting the cluster name from the env data, for instance "ibm_cloud_baremetal3; mandatory conf field" | ||
| bm_name = config.ENV_DATA.get("baremetal").get("env_name") | ||
| ocp_version = get_latest_release_version() | ||
| hcp_version = "".join([c for c in ocp_version if c.isdigit()][:3]) | ||
| match = re.search(r"\d+$", bm_name) | ||
| if match: |
There was a problem hiding this comment.
I understood, that it is good to be able to match the hosted cluster somehow with the hosting cluster, but I'm not sure about this approach, because it is quite specific for the particular environment. I'm not sure, if it is possible, that we would like to run this on octo-argo environment (maybe that is not even sufficient, not sure), or wouldn't this be later used on other platforms as well - e.g. vSphere?
What about at least make the bm_name part optional? So it will not fail if the env_name will not match the expected form or will be missing. Since the last part of the cluster name is anyway random, there shouldn't be any conflict in the cluster name and the only drawback will be the fact, that it will not be directly visible where the cluster belongs to.
Another option which might be sufficient would be to make the whole part bmX configurable and add this configuration to the environment specific conf files.
There was a problem hiding this comment.
you're right, thanks. I've added a ticket for this.
#10279
Signed-off-by: Daniel Osypenko <[email protected]>
ocs_ci/deployment/hosted_cluster.py
Outdated
| Check if the storage client installation was requested in the config | ||
|
|
||
| Args: | ||
| cluster_name: str: Name of the cluster |
There was a problem hiding this comment.
| cluster_name: str: Name of the cluster | |
| cluster_name (str): Name of the cluster |
ocs_ci/deployment/hosted_cluster.py
Outdated
| """ | ||
| Deploy multiple hosted OCP clusters on Provider platform | ||
|
|
||
| Args: | ||
| cluster_names_list (list, optional): List of cluster names to deploy. If not provided, all clusters |
There was a problem hiding this comment.
| cluster_names_list (list, optional): List of cluster names to deploy. If not provided, all clusters | |
| cluster_names_list (list): List of cluster names to deploy. If not provided, all clusters (optional argument) |
Signed-off-by: Daniel Osypenko <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dahorak, DanielOsypenko, petr-balogh 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 |
