-
Notifications
You must be signed in to change notification settings - Fork 1.3k
UI: Add multiple management server support #4885
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
|
Think it would be good to show the management server to which the user is connected listed in the header next to his name. Thoughts @utchoang @rhtyd |
|
@rhtyd a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
|
@utchoang Did you face any CORS issue ? |
|
@davidjumani In the previous discussion at apache/cloudstack-primate#898 there was a discussion about the CORS issue, and I think it will be solved by Nginx proxy. I think so. |
|
@utchoang a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
Indeed. As long as a proxy in between is used to use URLs and not different domains for management servers CORS is not an issue, for example:
These would be the relative URLs to reach the different management servers. This means that you do not set apiHost, but only set apiBase for each Management Server. Otherwise you need to deal with CORS indeed and thus add headers which can be done by Nginx/Apache. |
wido
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.
LGTM
|
I have tested apache/cloudstack-primate#898, which worked. @utchoang are these two prs same ? |
|
@weizhouapache Yes. Two PRs same. |
weizhouapache
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.
@utchoang great. lgtm
Thanks, was playing around in dev mode but got it working |
GabrielBrascher
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.
LGTM
|
LGTM |
|
enough lgtm, but has this been tested? (@davidjumani ?) |
|
@utchoang can you send some doc PR on how users will use it; for example how to configure the reverse proxy/server etc and will the dropdown be hidden for most users, say who don't want to specify or use this feature? |
|
@DaanHoogland This has been tested and working. For anyone who needs to test it locally, Update Update |
|
@rhtyd a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
| @@ -1,5 +1,12 @@ | |||
| { | |||
| "apiBase": "/client/api", | |||
| "servers": [ | |||
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 feature shouldn't be enabled by default, wouldn't it confuse users/admins? @utchoang @davidjumani @wido @nvazquez @sureshanaparti @weizhouapache @Pearl1594 @shwstppr ?
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.
If the list only shows one server at the time, what would it confuse them? There is only one option to select
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.
@rhtyd Yes. I will set it to default hidden mode. If the user/admin wants to enable it, the configuration can be changed.
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.
Looks good with @utchoang's change, disabled by default. +1 for getting this in asap
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.
+1
thanks @utchoang
|
UI build: ✔️ |
|
@davidjumani a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
nvazquez
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.
LGTM
|
Merging based on approvals |

Description
Related apache/cloudstack-primate#898
This PR for Add multiple management server support
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?