Skip to content

UI: Add multiple management server support#4885

Merged
nvazquez merged 9 commits into
apache:4.15from
utchoang:feature/multiple-server
Aug 11, 2021
Merged

UI: Add multiple management server support#4885
nvazquez merged 9 commits into
apache:4.15from
utchoang:feature/multiple-server

Conversation

@utchoang

@utchoang utchoang commented Apr 1, 2021

Copy link
Copy Markdown

Description

Related apache/cloudstack-primate#898
This PR for Add multiple management server support

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

image

How Has This Been Tested?

@utchoang utchoang changed the title add multiple management server support UI: Add multiple management server support Apr 1, 2021
@yadvr yadvr added this to the 4.16.0.0 milestone Apr 4, 2021
@davidjumani

davidjumani commented Jun 1, 2021

Copy link
Copy Markdown
Contributor

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

Screenshot from 2021-06-01 14-04-57

@yadvr

yadvr commented Jun 9, 2021

Copy link
Copy Markdown
Member

@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4885 (SL-JID-229)

Comment thread ui/src/components/header/UserMenu.vue Outdated
@davidjumani

Copy link
Copy Markdown
Contributor

@utchoang Did you face any CORS issue ?

@utchoang

Copy link
Copy Markdown
Author

@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

Copy link
Copy Markdown
Author

@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

@utchoang a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4885 (SL-JID-273)

@wido

wido commented Jun 22, 2021

Copy link
Copy Markdown
Contributor

@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.

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:

  • /proxy/management-server-1
  • /proxy/management-server-2
  • /proxy/management-server-3

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 wido left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@weizhouapache

Copy link
Copy Markdown
Member

I have tested apache/cloudstack-primate#898, which worked.
I used nginx or pfsense/haproxy to solve CORS issue.

@utchoang are these two prs same ?

@utchoang

Copy link
Copy Markdown
Author

@weizhouapache Yes. Two PRs same.

@weizhouapache weizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@utchoang great. lgtm

Comment thread ui/src/components/widgets/Console.vue Outdated
@davidjumani

Copy link
Copy Markdown
Contributor

@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.

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:

* /proxy/management-server-1

* /proxy/management-server-2

* /proxy/management-server-3

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.

Thanks, was playing around in dev mode but got it working

@GabrielBrascher GabrielBrascher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nvazquez

Copy link
Copy Markdown
Contributor

LGTM
@utchoang there is only one open comment, can you please check?

@DaanHoogland

Copy link
Copy Markdown
Contributor

enough lgtm, but has this been tested? (@davidjumani ?)

@yadvr

yadvr commented Jul 15, 2021

Copy link
Copy Markdown
Member

@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?

@davidjumani

Copy link
Copy Markdown
Contributor

@DaanHoogland This has been tested and working. For anyone who needs to test it locally,

Update ui/public/config.json

  "servers": [
    {
      "name": "QA-Server",
      "apiHost": "",
      "apiBase": "/qa/api"
    },
    {
      "name": "Localhost",
      "apiHost": "",
      "apiBase": "/client/api"
    }
  ],

Update ui/vue.config.js

proxy: {
      '/qa': {
        target: 'http://qa.cloudstack.cloud:8080/',
        secure: false,
        ws: false,
        pathRewrite: {
          '/qa/api': '/client/api',
          '/qa/console': '/console'
        },
        cookiePathRewrite: {
          '/qa/api': '/client/api',
          '*': '/'
        },
        changeOrigin: true,
        proxyTimeout: 10 * 60 * 1000 // 10 minutes
      },
      '/client': {
        target: process.env.CS_URL || 'http://localhost:8080',
        secure: false,
        ws: false,
        changeOrigin: true,
        proxyTimeout: 10 * 60 * 1000 // 10 minutes
      }
    }

@yadvr

yadvr commented Aug 9, 2021

Copy link
Copy Markdown
Member

@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

Comment thread ui/public/config.json
@@ -1,5 +1,12 @@
{
"apiBase": "/client/api",
"servers": [

Copy link
Copy Markdown
Member

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 ?

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Author

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.

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
thanks @utchoang

@blueorangutan

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4885 (SL-JID-495)

@davidjumani

Copy link
Copy Markdown
Contributor

@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

@davidjumani a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4885 (SL-JID-500)

@nvazquez nvazquez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nvazquez

Copy link
Copy Markdown
Contributor

Merging based on approvals

@nvazquez nvazquez merged commit 1182051 into apache:4.15 Aug 11, 2021
@utchoang utchoang deleted the feature/multiple-server branch August 16, 2021 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants