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.

Sign up for GitHub

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

Jump to bottom

Reconfigure VM: Add / Remove Network Adapters #163

Merged
merged 5 commits into from
Feb 19, 2018
Merged

Reconfigure VM: Add / Remove Network Adapters #163

merged 5 commits into from
Feb 19, 2018

Conversation

kruge002
Copy link
Contributor

@kruge002 kruge002 commented Dec 20, 2017 •
edited
Loading

This PR adds support for reconfiguring network adapters for VMware VMs.

This PR is part of a set of PRs:
ManageIQ/vmware_web_service#25 (merged by @agrare)
#163
ManageIQ/manageiq#16700
ManageIQ/manageiq-ui-classic#3121

More info: ManageIQ/manageiq-ui-classic#3119

@kruge002 kruge002 mentioned this pull request Dec 20, 2017
Closed
@agrare agrare self-assigned this Dec 20, 2017
This was referenced Dec 20, 2017
Merged
Merged
Merged
@kruge002
Copy link
Contributor Author

kruge002 commented Jan 2, 2018

@agrare I will add a commit solving the rubocop offenses. The Travis CI build fails because this PR depends on PR ManageIQ/manageiq#16700 as part of the set of 4 PRs for this new functionality.

agrare
agrare reviewed Jan 17, 2018
View reviewed changes
app/models/manageiq/providers/vmware/infra_manager/vm/reconfigure.rb Outdated
con.connected = "true"
con.startConnected = "true"
end
dev.backing = VimHash.new("VirtualEthernetCardDistributedVirtualPortBackingInfo") do |bck|
Copy link
Member

Choose a reason for hiding this comment

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

You need to check the Lan that has been selected to see if it is a distributed or standard network. We store that as the :shared property on the switch so once you find the lan you can check lan.switch.shared

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the check for lan.switched.shared.

agrare
agrare reviewed Jan 17, 2018
View reviewed changes
app/models/manageiq/providers/vmware/infra_manager/vm/reconfigure.rb Outdated
add_device_config_spec(vmcs, VirtualDeviceConfigSpecOperation::Add) do |vdcs|
vdcs.device = VimHash.new("VirtualVmxnet3") do |dev|
dev.key = rand(-9999..-100) # negative integer as temporary key
dev.unitNumber = 0
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to pick a unitNumber that is available and give it a controllerKey as well. For the key we usually just use -100 * options[:unit_number]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using unitNumber 0 instructs VMware to choose the next available unit number. The key must be a unique negative number (temporary key). The controllerKey will be chosen by VMware.

@miq-bot
Copy link
Member

miq-bot commented Jan 18, 2018

This pull request is not mergeable. Please rebase and repush.

@agrare
Copy link
Member

agrare commented Feb 19, 2018 •
edited
Loading

Tested adding and removing both dvPortGroup and VM Network backings and all works 👍
@kruge002 can you squash just the rubocop commits?

@kruge002
Copy link
Contributor Author

kruge002 commented Feb 19, 2018 •
edited
Loading

@agrare Please use 'Squash and Merge' to merge this feature branch as one commit.

@agrare
Copy link
Member

agrare commented Feb 19, 2018

We don't use that feature, I'll just fixup locally

@kruge002
Copy link
Contributor Author

@agrare I have squashed the last two commits.

@miq-bot
Copy link
Member

miq-bot commented Feb 19, 2018

Checked commits kruge002/manageiq-providers-vmware@946a0b8~...d618d2e with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@agrare agrare merged commit a8b0c37 into ManageIQ:master Feb 19, 2018
@agrare agrare added this to the Sprint 80 Ending Feb 26, 2018 milestone Feb 19, 2018
@agrare
Copy link
Member

agrare commented Feb 19, 2018

Thanks so much @kruge002 ! Great enhancement

@simaishi
Copy link
Contributor

simaishi commented Jun 1, 2018 •
edited
Loading

@agrare Can this be gaprindashvili/yes? #272 needs reconfigure network feature in G-branch..

@agrare
Copy link
Member

agrare commented Jun 1, 2018

@simaishi Done

simaishi pushed a commit that referenced this pull request Jun 1, 2018
@agrare @simaishi
Merge pull request #163 from kruge002/ReconfigureNetwork
c91f431 
Reconfigure VM: Add / Remove Network Adapters
(cherry picked from commit a8b0c37)
@simaishi
Copy link
Contributor

simaishi commented Jun 1, 2018

Gaprindashvili backport details:

$ git log -1
commit c91f431d009a99ea13a06e59501bb792b803940d
Author: Adam Grare <agrare@redhat.com>
Date:   Mon Feb 19 10:17:39 2018 -0500

    Merge pull request #163 from kruge002/ReconfigureNetwork
    
    Reconfigure VM: Add / Remove Network Adapters
    (cherry picked from commit a8b0c376d8dc232c3e1e8727a6b104b117bf998c)

agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Apr 15, 2019
@gmcculloug
Merge pull request ManageIQ#163 from billfitzgerald0120/ae_retry_inte…
101e8d0 
…rval

Set retry interval to 1 minute for generic service state-machine.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agrare agrare agrare approved these changes

Assignees

@agrare agrare

Labels
blocker enhancement gaprindashvili/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kruge002 @miq-bot @agrare @simaishi