Skip to content

ui: Fix CreateKubernetesCluster for ha - #6106

Merged
nvazquez merged 1 commit into
apache:mainfrom
shapeblue:ui-cks
Mar 15, 2022
Merged

nvazquez merged 1 commit into
apache:mainfrom
shapeblue:ui-cks

Conversation

@davidjumani

Copy link
Copy Markdown
Contributor

Description

Fixes the issue of always sending 2 control plan nodes in the CreateKubernetesCluster vue

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)

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@davidjumani

Copy link
Copy Markdown
Contributor Author

@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/6106 (SL-JID-1254)

@davidjumani
davidjumani requested a review from utchoang March 14, 2022 06:22

@utchoang utchoang left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM! Thanks @davidjumani

@shwstppr

Copy link
Copy Markdown
Contributor

@davidjumani @utchoang this could be a common issue with other forms as well (saw it in add network offering form as well). Earlier when v-if was used, check for form items on which v-if was added was not needed. Now even though the form-item is not shown, value must still be there in values during form submit. Anything we can do address all of them?

@nvazquez

Copy link
Copy Markdown
Contributor

Thanks @davidjumani @utchoang @shwstppr - if possible let's continue with this PR for this specific issue and address the rest of the common issues on a separate PR

@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
nvazquez merged commit a4c6b12 into apache:main Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants