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
Change ManagementState for image registry for non cloud platforms #1437
Change ManagementState for image registry for non cloud platforms #1437
Conversation
Fixes: red-hat-storage#1436 Signed-off-by: Petr Balogh <pbalogh@redhat.com>
b867caf
to
3f5004f
Compare
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.
LGTM
Verification running here: https://ocs4-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/job/qe-deploy-ocs-cluster/4327/console |
@@ -37,6 +37,15 @@ def change_registry_backend_to_ocs(): | |||
resource_name=constants.IMAGE_REGISTRY_RESOURCE_NAME, params=param_cmd, format_type='json' | |||
), f"Registry pod storage backend to OCS is not success" | |||
|
|||
if(config.ENV_DATA['platform'] not in constants.CLOUD_PLATFORMS): | |||
run_cmd( |
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.
Is the OK for OCP 4.2 on VMware, the docs in Fixes is pointing to OCP 4.3?
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 do not think that it will have any impact on 4.2 as this is suppose to be default value and in 4.2 it's working with such value by default:
spec:
defaultRoute: true
disableRedirect: false
httpSecret: 868041c963e68caa8eb5b8e7564fffe1c33400336c2db02dc8d4e8a006716b17c6ea74655a1da7ac066e7bfaf6a745f2a47c16491c1ae5b7cbee12869c04c40c
logging: 2
managementState: Managed
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.
Approved. Please see my comment about sampling. Let's not hold this PR on it as we want to proceed with deployment over vmware
logger.info( | ||
"Waiting 30 seconds after change managementState of image-registry." | ||
) | ||
time.sleep(30) |
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.
is there something we can sample in order to avoid sleep?
Fixes: #1436
Signed-off-by: Petr Balogh pbalogh@redhat.com