-
Notifications
You must be signed in to change notification settings - Fork 897
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
Get container statuses during refresh #18016
Get container statuses during refresh #18016
Conversation
@agrare do you know who could review this? |
app/models/host.rb
Outdated
containers = MiqLinux::Utils.parse_docker_ps_list(containers) | ||
end | ||
|
||
(services + containers).each do |service| |
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.
Are docker containers the same thing as services? This might be true for this specific openstack case but I doubt it is true universally for all host types.
IMO if openstack is returning some services as systemd units and some as docker containers this should be in the openstack host model.
We can split out a method here called collect_services
which in openstack you could do basically super.concat(docker_services)
8b9d26d
to
493d0ab
Compare
@agrare Hi, is it better now? :) |
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.
Yeah this looks good, just the one change needed
app/models/host.rb
Outdated
@@ -1091,7 +1091,7 @@ def refresh_patches(ssu) | |||
Patch.refresh_patches(self, patches) | |||
end | |||
|
|||
def refresh_services(ssu) | |||
def collect_services(ssu) | |||
xml = MiqXml.createDoc(:miq).root.add_element(:services) |
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 think this should be in the refresh_services
method not in here
493d0ab
to
ad7fb6f
Compare
Checked commit alexander-demicev@ad7fb6f with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
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
Get container statuses during refresh of host. We need this if openstack was deployed with containerized services. I decided to keep attributes like 'systemd_load' for containers because they are used for scopes which are passed to UI.
https://bugzilla.redhat.com/show_bug.cgi?id=1573507