Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

UI primarystorage linstor fixes#6481

Merged
yadvr merged 2 commits intoapache:4.17apache/cloudstack:4.17from
LINBIT:ui-primarystorage-linstor-fixesLINBIT/cloudstack:ui-primarystorage-linstor-fixesCopy head branch name to clipboard
Jun 22, 2022
Merged

UI primarystorage linstor fixes#6481
yadvr merged 2 commits intoapache:4.17apache/cloudstack:4.17from
LINBIT:ui-primarystorage-linstor-fixesLINBIT/cloudstack:ui-primarystorage-linstor-fixesCopy head branch name to clipboard

Conversation

@rp-
Copy link
Contributor

@rp- rp- commented Jun 21, 2022

Description

If Linstor protocol is selected it makes no sense to show other
providers as Linstor only works with the Linstor provider.
And also the install wizard doesn't use the correct provider for linstor.

This changes were already merged for 4.16.1.0 see: #5672
But I don't know why they weren't merged to main back than, maybe I don't know how cloudstack's
merge/PR's work.
But this should be definitely merged to 4.17.* and main

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

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Clicked through local served vui ui -> add primary storage

@acs-robot
Copy link

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

@yadvr
Copy link
Member

yadvr commented Jun 21, 2022

Hi @rp- can you send the PR for the relevant branch (4.16 or 4.17)? (edit base branch of the PR or rebase your PR branch as necessary)

@rp-
Copy link
Contributor Author

rp- commented Jun 21, 2022

Hi @rp- can you send the PR for the relevant branch (4.16 or 4.17)? (edit base branch of the PR or rebase your PR branch as necessary)

It was/is already merged for 4.16.
I want it to be in dev(main) branch and also probably in following 4.17.* releases.

So is it enough to keep this PR and make another one for the 4.17 branch??

@sureshanaparti
Copy link
Contributor

sureshanaparti commented Jun 21, 2022

@rp- I think, the PR #5672 commit (e93d674) is merged to 4.16.1, and forward merged to main / 4.17. Please check. cc @nvazquez

@rp-
Copy link
Contributor Author

rp- commented Jun 21, 2022

@sureshanaparti
Well no, it wasn't included in 4.17, nor is it in the main branch. I just got a report today(for 4.17.0) about the problems this PR will fix again.
Here this commit removed it:
d258da5

Maybe my commits weren't the only ones lost...

@sureshanaparti
Copy link
Contributor

@rp- I think, the PR #5672 commit (e93d674) is merged to 4.16.1, and forward merged to main / 4.17. Please check. cc @nvazquez

@rp- All the changes in PR #5672 that are forward merged to main (from 4.16.1), were updated to support Vue3 library (in 4.17.0, PR #5151 - https://github.com/apache/cloudstack/pull/5151/files#diff-09eec531afd195f4299deb9387fd66366eb87828571eaeed5ee625c6273cdf2a). Check whether the changes here are inline with Vue3 or not. cc @utchoang @nvazquez

@nvazquez
Copy link
Contributor

Hi @rp- the fix was merged forward to main but as you pointed unfortunately it got reverted by PR #5151. Can you please rebase and target this PR to branch 4.17 so it also contains the fixes? (then will be forward merged to main branch)

@nvazquez nvazquez added this to the 4.17.1.0 milestone Jun 21, 2022
@rp- rp- force-pushed the ui-primarystorage-linstor-fixes branch from 444ae8d to 0972183 Compare June 21, 2022 13:25
@acs-robot
Copy link

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@rp- rp- changed the base branch from main to 4.17 June 21, 2022 13:26
@blueorangutan
Copy link

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

rp- added 2 commits June 21, 2022 15:30
If Linstor protocol is selected it makes no sense to show other
providers as Linstor only works with the Linstor provider.
@rp- rp- force-pushed the ui-primarystorage-linstor-fixes branch from 0972183 to 29a68fb Compare June 21, 2022 13:30
@rp-
Copy link
Contributor Author

rp- commented Jun 21, 2022

@nvazquez done

@acs-robot
Copy link

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Copy link

@utchoang utchoang left a comment

Choose a reason for hiding this comment

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

LGTM!

@yadvr yadvr merged commit 355fdaa into apache:4.17 Jun 22, 2022
@rp- rp- deleted the ui-primarystorage-linstor-fixes branch February 5, 2024 08:39
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.

7 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.