Skip to content
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

Add support for VMware Cloud Director provider #4644

Conversation

Waseem826
Copy link
Contributor

@Waseem826 Waseem826 commented Jul 4, 2022

What does this PR do / Why do we need it:
This PR adds support for VMware Cloud Director as a cloud provider.

Create Preset

screenshot-localhost_8000-2022 07 07-21_37_15

Cluster Wizard

screenshot-localhost_8000-2022 07 07-21_37_35

Provider Settings

screenshot-localhost_8000-2022 07 07-21_35_44

Node Settings

screenshot-localhost_8000-2022 07 07-21_36_14

Which issue(s) this PR fixes :

Fixes #4536

Special notes for your reviewer:

Release Note:

Support for VMware Cloud Director as a cloud provider.

@kubermatic-bot kubermatic-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. labels Jul 4, 2022
@kubermatic-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubermatic-bot kubermatic-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 4, 2022
@Waseem826 Waseem826 force-pushed the 4536-add-support-for-vmware-cloud-director branch from f48a625 to 9e4a6fd Compare July 5, 2022 13:17
@kubermatic-bot kubermatic-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 5, 2022
@ahmedwaleedmalik ahmedwaleedmalik changed the title 4536 Add support for VMware Cloud Director provider Add support for VMware Cloud Director provider Jul 5, 2022
@Waseem826 Waseem826 force-pushed the 4536-add-support-for-vmware-cloud-director branch 7 times, most recently from 5e863c3 to 772452e Compare July 6, 2022 21:24
@Waseem826
Copy link
Contributor Author

/test all

@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #4644 (e126253) into master (a120fc0) will decrease coverage by 0.16%.
The diff coverage is 35.82%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4644      +/-   ##
==========================================
- Coverage   50.39%   50.22%   -0.17%     
==========================================
  Files         346      359      +13     
  Lines       12897    13377     +480     
  Branches     1734     1823      +89     
==========================================
+ Hits         6499     6719     +220     
- Misses       6050     6284     +234     
- Partials      348      374      +26     
Impacted Files Coverage Δ
...rvices/node-data/provider/vmware-cloud-director.ts 15.94% <15.94%> (ø)
.../services/wizard/provider/vmware-cloud-director.ts 16.39% <16.39%> (ø)
src/app/core/services/wizard/presets.ts 27.63% <33.33%> (+0.23%) ⬆️
...pp/core/services/provider/vmware-cloud-director.ts 40.00% <40.00%> (ø)
src/app/shared/entity/cluster.ts 78.23% <50.00%> (-1.63%) ⬇️
src/app/shared/entity/node.ts 44.23% <50.00%> (-0.22%) ⬇️
...ware-cloud-director-provider-settings/component.ts 88.46% <88.46%> (ø)
src/app/core/module.ts 98.57% <100.00%> (+0.04%) ⬆️
src/app/core/services/node-data/service.ts 77.31% <100.00%> (+0.97%) ⬆️
src/app/shared/entity/datacenter.ts 97.05% <100.00%> (+0.08%) ⬆️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a120fc0...e126253. Read the comment docs.

@Waseem826
Copy link
Contributor Author

/test all

@Waseem826 Waseem826 force-pushed the 4536-add-support-for-vmware-cloud-director branch from 772452e to 4223193 Compare July 7, 2022 16:20
@kubermatic-bot kubermatic-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 7, 2022
@Waseem826 Waseem826 marked this pull request as ready for review July 7, 2022 16:47
@kubermatic-bot kubermatic-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2022
@Waseem826
Copy link
Contributor Author

/retest

Copy link
Contributor

@KhizerRehan KhizerRehan left a comment

Choose a reason for hiding this comment

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

PR comments

@Waseem826 Waseem826 force-pushed the 4536-add-support-for-vmware-cloud-director branch 2 times, most recently from 87edfc7 to 26c3d1f Compare July 13, 2022 11:05
@Waseem826
Copy link
Contributor Author

/hold
Waiting for backend changes and perhaps another review from @Talha-Jamil-TJ would be better since this is a large PR.

@kubermatic-bot kubermatic-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 13, 2022
@Waseem826 Waseem826 force-pushed the 4536-add-support-for-vmware-cloud-director branch from 26c3d1f to e4055b6 Compare July 13, 2022 11:35
@kubermatic-bot
Copy link
Contributor

@Waseem826: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pre-dashboard-test-full-e2e e4055b6 link false /test pre-dashboard-test-full-e2e
pre-dashboard-test-full-e2e-ce e4055b6 link false /test pre-dashboard-test-full-e2e-ce

Full PR test history

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@Waseem826 Waseem826 force-pushed the 4536-add-support-for-vmware-cloud-director branch from e4055b6 to 1a08395 Compare July 14, 2022 09:54
Copy link
Contributor

@Talha-Jamil-TJ Talha-Jamil-TJ left a comment

Choose a reason for hiding this comment

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

Changes requested

@ahmedwaleedmalik
Copy link
Member

  • Alignment issue
    Screenshot 2022-07-19 at 12 00 34 AM

  • The alignment here looks off. Since the logo for VCD is playing a part in here, can you please fix this in your PR?

Screenshot 2022-07-18 at 11 52 07 PM

@ahmedwaleedmalik
Copy link
Member

ahmedwaleedmalik commented Jul 18, 2022

To everyone involved with reviewing this PR. Sorry but the review on this PR was done not in the nicest of ways. Could have been better and more simpler ;)

  1. If you have similar comments, instead of copy/pasting it to lots and lots of places. Just add a single comment and ask the person to update it everywhere.
  2. If you are proposing some change, please share some context. Unless it's a very simple and obvious change.

I'm not sig-ui so refraining from a lot of technical comments. ¯_(ツ)_/¯

/meow

@kubermatic-bot
Copy link
Contributor

@ahmedwaleedmalik: cat image

In response to this:

To everyone involved with reviewing this PR. Sorry but the review on this PR was done quite poorly.

  1. If you have similar comments, instead of copy/pasting it to 50 places. Just add a single comment and ask the person to update it everywhere.
  2. If you are proposing some change, please share some context. Unless it's a very simple and obvious change.

I'm not sig-ui so refraining from a lot of technical comments ¯_(ツ)_/¯

/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Waseem826 Waseem826 force-pushed the 4536-add-support-for-vmware-cloud-director branch from 1a08395 to e126253 Compare July 20, 2022 11:41
@Waseem826
Copy link
Contributor Author

/unhold

@kubermatic-bot kubermatic-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 20, 2022
@KhizerRehan
Copy link
Contributor

/approve
/lgtm

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2022
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: dc7b81c3a24505a642f44a36783a17a02b56479e

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: KhizerRehan, Waseem826

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [KhizerRehan,Waseem826]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubermatic-bot kubermatic-bot merged commit 820d914 into kubermatic:master Jul 20, 2022
@Waseem826 Waseem826 deleted the 4536-add-support-for-vmware-cloud-director branch July 20, 2022 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for VMware Cloud Director as cloud provider
5 participants