-
Notifications
You must be signed in to change notification settings - Fork 199
Check that IPA images have finished downloading as well #712
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
Conversation
dhellmann
left a comment
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.
One nit, but LGTM
|
|
||
| # Wait for images to be downloaded/ready | ||
| while ! curl --fail http://localhost:80/images/rhcos-ootpa-latest.qcow2.md5sum ; do sleep 1 ; done | ||
| while ! curl --fail --head http://localhost/images/ironic-python-agent.initramfs ; do sleep 1; done |
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.
The port is explicit in the call on line 76. I don't know if that's important, but the difference stood out.
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 noticed that, but there's no reason to specify port 80 for http. I assume the URL was copy and pasted from somewhere that included the port. For consistency, I removed it from the rhcos url as well.
04_setup_ironic.sh
Outdated
| while ! curl --fail http://localhost:80/images/rhcos-ootpa-latest.qcow2.md5sum ; do sleep 1 ; done | ||
| while ! curl --fail http://localhost/images/rhcos-ootpa-latest.qcow2.md5sum ; do sleep 1 ; done | ||
| while ! curl --fail --head http://localhost/images/ironic-python-agent.initramfs ; do sleep 1; done | ||
| while ! curl --fail --head http://localhost/images/ironic-python-agent.headers ; do sleep 1; done |
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.
The headers URL should be http://localhost/images/ironic-python-agent.tar.headers
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.
Thanks, good catch... https://github.com/metal3-io/ironic-ipa-downloader/blob/master/get-resource.sh#L7-L9
I didn't notice the difference between $FFILENAME and $FILENAME
We currently only wait for RHCOS to finish, but we've seen at least one case where IPA wasn't available yet which causes the baremetal hosts to go into Maintenance mode in Ironic. fixes openshift-metal3#711
|
Build ABORTED, see build http://10.8.144.11:8080/job/dev-tools/972/ |
|
Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/974/ |
|
Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/978/ |
mcornea
left a comment
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.
Tested and it worked fine on my environment. Thanks!
|
I tried to apply this PR locally and I got another error I do not know if it related to this PR or not, but maybe someone saw this error message before? |
I don't think it's related exactly, your Ironic conductor service wasn't fully up. I'd file an issue and we should make sure Ironic API and the conductor is up in dev-scripts, like we did in openshift-metal3/terraform-provider-ironic#32. |
|
This isn't needed any more, it was already added as part of #726. |
We currently only wait for RHCOS to finish, but we've seen at least one
case where IPA wasn't available yet which causes the baremetal hosts to
go into Maintenance mode in Ironic.
fixes #711