-
Notifications
You must be signed in to change notification settings - Fork 166
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
add ocp install log, delete cluster #10703
add ocp install log, delete cluster #10703
Conversation
DanielOsypenko
commented
Oct 20, 2024
- tuned create_openshift_install_log_file and added to rosahcp
- added delete resources such as oidc-config, account-roles, etc into destroy_cluster
5388585
to
9512c39
Compare
Signed-off-by: Daniel Osypenko <dosypenk@redhat.com>
9512c39
to
2346f9f
Compare
Signed-off-by: Daniel Osypenko <dosypenk@redhat.com>
Signed-off-by: Daniel Osypenko <dosypenk@redhat.com>
60a0f39
to
ff96098
Compare
Signed-off-by: Daniel Osypenko <dosypenk@redhat.com>
ff96098
to
b86e840
Compare
All cycle is clean - username is updated from static |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, I would just propose to update the message in the .openshift_install.log
to make it more generic also for other types of deployment, not only via Assisted Installer.
ocs_ci/utility/deployment.py
Outdated
fd.writelines( | ||
[ | ||
"W/A for our CI to get URL to the cluster in jenkins job. " | ||
"Cluster is deployed via Assisted Installer API!\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth to update this line based on the deployment type (or make it more generic), as this is not true for all the cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not change this line, I only pushed create_openshift_install_log_file function to common space from ocs_ci/deployment/assisted_installer.py
as far as I understand it is a mock-up for our log-parser on Jenkins pipelines.
TBH, correcting code(mocked-up log message) for our custom-built pipelines that are not validating this log message other than retrieving the console should have a very low priority. Until it can be important for something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the Jenkins job, the important line is only the last one ... Access the OpenShift web-console here ...
.
The think is, that initially it was used only for the Assisted Installer deployment, where it make sense the message, but since you are moving it to the common space and re-using it for the ROSA deployment also, it will be worth to update the message to avoid confusion.
I think it is not necessary to make it dynamic based on the actual deployment type, but just more generic. What about something like this?
"Cluster is deployed via some kind of managed deployment (Assisted Installer API or ROSA). OpenShift Installer (IPI or UPI deployment) were not used!\n"
Signed-off-by: Daniel Osypenko <dosypenk@redhat.com>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dahorak, DanielOsypenko, jilju 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 |