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

adding --config-file param functionality to deployment #10724

Closed

Conversation

shivamdurgbuns
Copy link
Member

@shivamdurgbuns shivamdurgbuns requested a review from a team as a code owner October 24, 2024 05:23
@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines label Oct 24, 2024
Copy link

openshift-ci bot commented Oct 24, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shivamdurgbuns

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

@shivamdurgbuns
Copy link
Member Author

@shivamdurgbuns shivamdurgbuns added the Verified Mark when PR was verified and log provided label Oct 24, 2024
params (str): Parameter to pass to exporter script

Returns:
str: absolute path to config.ini gile
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
str: absolute path to config.ini gile
str: absolute path to config.ini file

cmd = f"{python_version} {script_path} {params}"
ocs_version = version.get_semantic_ocs_version_from_config()
# if condition is for the new feature introduced in
# 4.17 in OCSQE-2249 where the this covers the test case
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# 4.17 in OCSQE-2249 where the this covers the test case
# 4.17 in OCSQE-2249 where this covers the test case

params (str): Parameter to pass to be converted into config.ini file.

Returns:
config_ini_file.name (str): Path to the config.ini file.
Copy link
Contributor

Choose a reason for hiding this comment

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

leave blank line after this

config_ini_file.name (str): Path to the config.ini file.
"""
data = "[Configurations]\n"
import re
Copy link
Contributor

Choose a reason for hiding this comment

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

move to import section at the beginning of file

)
with open(config_ini_file.name, "w") as fd:
fd.write(data)
log.info(f"config.ini fike for cluster is located at {config_ini_file.name}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log.info(f"config.ini fike for cluster is located at {config_ini_file.name}")
log.info(f"config.ini file for cluster is located at {config_ini_file.name}")

params (str): Parameter to pass to be converted into config.ini file.

Returns:
config_ini_file.name (str): Path to the config.ini file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
config_ini_file.name (str): Path to the config.ini file.
str: Path to the config.ini file.

@@ -5198,3 +5199,38 @@ def extract_image_urls(string_data):
# Find all URLs that start with 'registry.redhat.io'
image_urls = re.findall(r'registry\.redhat\.io[^\s"]+', string_data)
return image_urls


def params_to_configini_file(params):
Copy link
Contributor

Choose a reason for hiding this comment

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

rename the function which resembles the work of function. In this case it creates config ini file. ( e.g: create_config_ini_file()

@PrasadDesala PrasadDesala added the Customer defects Defects automated aspart of GSS closed loop label Nov 28, 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:
PR Test Suite: tier1
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.16-ga
OCS VERSION: 4.16
tested against branch: master

Job FAILED (installation failed, tests not executed).

@@ -0,0 +1,3 @@
---
ENV_DATA:
use_config_file: true
Copy link
Member

Choose a reason for hiding this comment

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

The name here is too generic doesn't tell anything about this use_confi_file.

As it's config file for exporter script to external RHCS cluster, more reasonable sounds something like:
rhcs_external_use_config_file

Also, please document this value in:
https://github.com/red-hat-storage/ocs-ci/blob/master/conf/README.md

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:
PR Test Suite: tier1
PR Test Path: tests/
Additional Test Params:
OCP VERSION: 4.16-ga
OCS VERSION: 4.16
tested against branch: master

Job FAILED (installation failed, tests not executed).

@shivamdurgbuns
Copy link
Member Author

Reopening this PR here #11160

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Customer defects Defects automated aspart of GSS closed loop size/M PR that changes 30-99 lines Verified Mark when PR was verified and log provided
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants