-
Notifications
You must be signed in to change notification settings - Fork 113
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
Allow running ironic API, conductor and exporter separately #62
Conversation
We want to stop running several processes in one container, since it violates container good practices and makes collecting logs and health checking much harder.
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/818/ |
. /bin/configure-ironic.sh | ||
|
||
# TODO(dtantsur): merge these two scripts when/if we no longer support | ||
# all-in-one container |
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.
Its not quite clear to me what this means, is this referring to when we use a different image for ironic-prometheus-exporter?
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.
At this point I'd prefer to avoid adding any new images - we already have two critical tasks (machine-api-opearator integration of the baremetal operator, and cluster hosted ironic) blocked on getting forks of the existing images into openshift, adding any more right at this moment is likely to further delay things, which we really, really do need to avoid :)
That said we can start planning for new images to be used in future, but just not right now when those critical tasks are still pending.
I didn't really mean splitting the image here, just the eventual removal of the all-in-one ironic entry point.
Note, however, that there is certain risk that openshift folks won't accept the way we build images, but that's a different story.
…-------- Original Message --------
On Jul 1, 2019, 09:55, Steven Hardy wrote:
@hardys commented on this pull request.
---------------------------------------------------------------
In [runironic-exporter.sh](#62 (comment)):
> @@ -0,0 +1,7 @@
+#!/usr/bin/bash
+
+. /bin/configure-ironic.sh
+
+# TODO(dtantsur): merge these two scripts when/if we no longer support
+# all-in-one container
At this point I'd prefer to avoid adding any new images - we already have two critical tasks (machine-api-opearator integration of the baremetal operator, and cluster hosted ironic) blocked on getting forks of the existing images into openshift, adding any more right at this moment is likely to further delay things, which we really, really do need to avoid :)
That said we can start planning for new images to be used in future, but just not right now when those critical tasks are still pending.
—
You are receiving this because you authored the thread.
Reply to this email directly, [view it on GitHub](#62?email_source=notifications&email_token=AAEB3U2LCANPWKBMJTNN633P5GZ6NA5CNFSM4H33Q2IKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB5CQH4Q#discussion_r298920485), or [mute the thread](https://github.com/notifications/unsubscribe-auth/AAEB3U6FPMTLKUWUFISSAPLP5GZ6NANCNFSM4H33Q2IA).
|
I'd like to merge this change this change since the CI works on it, and it alone is backward compatible with how things are run right now. Opinions? |
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.
Ok lgtm provided we expect to maintain backwards compatibility for now - provided this passes CI then I think we can go ahead and merge.
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/841/ |
Bug 1810145: Add WORKDIR to builder
We want to stop running several processes in one container, since it
violates container good practices and makes collecting logs and
health checking much harder.