Fix MiqRegion#remote_*_url returning invalid URL on podified #22668
Conversation
Move the bulk of the logic into MiqServer
| def remote_ui_ipaddress | ||
| server = remote_ui_miq_server | ||
| server.try(:ipaddress) | ||
| remote_ui_miq_server&.ui_ipaddress | ||
| end | ||
|
|
||
| def remote_ui_hostname | ||
| server = remote_ui_miq_server | ||
| server && (server.hostname || server.ipaddress) | ||
| remote_ui_miq_server&.ui_hostname | ||
| end |
There was a problem hiding this comment.
NOTE these methods don't appear to be used outside of the remote_*_address/url methods but since they aren't private I don't want to remove them especially for something we might backport.
remote_ui_ipaddress, remote_ui_hostname, remote_ws_ipaddress, remote_ws_hostname
agrare
force-pushed
the
remote_ui_ws_address_podified
branch
from
3623d1c
to
458338c
Compare
|
Checked commits agrare/manageiq@11bc6ae~...458338c with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
| if MiqEnvironment::Command.is_podified? | ||
| ENV.fetch("APPLICATION_DOMAIN") | ||
| else | ||
| contact_with == :hostname ? ui_hostname : ui_ipaddress | ||
| end |
There was a problem hiding this comment.
NOTE it is a bit of duplication to check for is_podified? in both these methods and in ui_address, but with the contact_with == :hostname and ::Settings.webservices.contactwith == 'hostname' checks it seemed cleaner than trying to deal with contact_with = :ipaddress but that returning nil on podified.
There was a problem hiding this comment.
The problem is it probably belongs in the kubernetes module for the MiqServer::WorkerManagement but we include kubernetes, process, or systemd regardless of the type of server we're running. I guess this is the only thing you can do.
There are these methods in worker_management though, although the interface is ugly from here: server.worker_manager.class.podified?
It feels like the worker_manager should know this answer for what type it is and cache the result. Alas, this code is fine for now.
There was a problem hiding this comment.
(it would have been nice to just override this method in kubernetes.rb but we include these modules regardless of what type of manager we are)
There was a problem hiding this comment.
Yeah I didn't think that WorkerManagement was the right place for this since it is more appliance-level but I agree it would be much cleaner to have a k8s subclass for this. Maybe we do a similar model for EnvironmentManagement (aka convert from simple module to a class then have systemd/kubernetes subclasses)?
|
Backported to |
Fix MiqRegion#remote_*_url returning invalid URL on podified (cherry picked from commit f6b1d9b)
On appliances the remote_ui_url/remote_ws_url methods find an active MiqServer and use that server's hostname/ipaddress to build a URL for access.
On podified the hostname is the name of the orchestrator pod which isn't usable to be able to access the UI.
We have the proper hostname to access the httpd route in the APPLICATION_DOMAIN env var, use that if we are on podified.