fog / fog-vsphere

Fog for vSphere
MIT License
36 stars 63 forks source link

Add support for list_vm_interfaces to be able to return the id of an … #248

Closed timogoebel closed 4 years ago

timogoebel commented 4 years ago

…NSX-T based portgroup

Replaces #246.

fwdIT commented 4 years ago

Hi @robsee, what did this pr exactly fix for you ? I am still seeing the same issue as reported in this bug: https://projects.theforeman.org/issues/28916

I used the latest commit on fog-vsphere including your pr

robsee commented 4 years ago

Just to ask a silly question, did you restart the foreman/katello services after you applied the PR ? This PR should fix the issue you are talking about in the foreman issue. I am currently using it successfully.

fwdIT commented 4 years ago

@robsee yup, pulled the git repo, rsynced all content on master to my gem folder (removing any other files) and restarted foreman. are you on foreman 1.24.2 ?

robsee commented 4 years ago

@fwdIT Maybe do a find on the changed files and make sure there aren't duplicates someplace else on your system ? I know the patch works for me. Just to rule things out, you could try using my original ugly version of the patch from https://github.com/robsee/fog-vsphere/commit/18396a3ff8e73ebe86eabfcc58be74b38668c503 (which is the one I am still running on)

fwdIT commented 4 years ago

@robsee thx, will give it a shot tomorrow and keep you updated

fwdIT commented 4 years ago

@robsee did some more tests. first of all, I always got the same error result due to having a new rpm release of fog-vsphere installed (3.2.5), I tested that version when I wrongfully thought the patch was already included in that release but when this was clarified (it's not in there), I forgot to uninstall it facepalm. so you had a valid point, first looking for other occurrences of the patched files. I kept syncing the github repo master branch to the gem folder of 3.2.1 (foreman 1.24.2) but off course it kept getting ignored due to the newer 3.2.5 gem which was also present. thats corrected again now.

first of all, which foreman/katello version are you exactly using ?

when I test with the master of your fog-vsphere fork, I get this error: Failed to create a compute xxx (VMware) instance xxx: undefined local variable or method `options' for #Fog::Vsphere::Compute::Real:0x00007f5cfaa8dbb8

probable reason (diff) /lib/fog/vsphere/requests/compute/create_vm.rb:

cluster = get_raw_cluster(options[:cluster], attributes[:datacenter]) cluster = get_raw_cluster(attributes[:cluster], attributes[:datacenter])

when I test with the master of the official fog-vsphere repo I get: Query instance details for xxx task failed with the following error: wrong number of arguments (given 1, expected 2) which is weird since the actual nsx-t patches are the same in your repo compared to fog-vsphere master

Both now finally show something new, meaning the new code is active but still no fix in my Foreman 1.24.2 sadly.

Wonder what I am missing since it does work for you.

robsee commented 4 years ago

I'm running Katello 3.14.1 which uses foreman 1.24.2. Maybe you are better off doing a stock install, and manually patching the fog-vsphere using the changes from my commit. They should be pretty self contained. Actually, thinking about it, that's exactly what I did. I only did the patches against master to get them upstreamed.

fwdIT commented 4 years ago

@robsee will give it another shot next week. how did you debug fog-vsphere specifically while making the code adjustments? for now I am just doing a manual host create and looking in the debug logs for the Fog::Vsphere::Compute::Interface response but perhaps there are more optimal ways while writing code :)

robsee commented 4 years ago

Basically just brute forced it (kept manually creating a machine). I'm a sys/netadmin by trade, not a programmer. Luckily I found other projects that had run into this problem, so I just borrowed the logic from them

fwdIT commented 4 years ago

@robsee seems we are similar in our struggle :) not a programmer either, also sysadmin your suggestion worked, by the way. I started from the original gem and just added the 2 code blocks in the 2 related files. so, the actual fix for the nsx-t worked. must be another commit in fog-vsphere on current master which broke it again.

timogoebel commented 4 years ago

@robsee, @fwdIT: This is what I use: https://gist.github.com/timogoebel/0809d97444f3fc614b0a50f7dc057b61

The Gemfile looks like this:

source 'https://rubygems.org'
gem 'fog'
gem 'fog-vsphere', :path => '../fog-vsphere'
gem 'awesome_print'
gem 'pry'

You can then execute the script with: bundle exec create.rb and it will use your locally checked out version of fog-vsphere.

timogoebel commented 4 years ago

must be another commit in fog-vsphere on current master which broke it again.

Can you figure out the exact commit so we can fix it?

fwdIT commented 4 years ago

@timogoebel that is indeed still on my to do, will try to test further this week or otherwise next week @robsee did you also update fog-core ?

Wondering about 1 difference already though, I am on Foreman 1.24.2 and started the nsx-t patches in the fog-vsphere which came with this install. These are my package versions: tfm-rubygem-fog-core-2.1.0-1.el7.noarch tfm-rubygem-fog-vsphere-3.2.1-1.el7.noarch robsee his fork including the patches is based on fog-vsphere-3.2.3 I saw but that repo content did not work for me on my Foreman, perhaps due to the fog-core version I was still on ?

I did read the upgrade notes on fog-vsphere which said: In version 3.0.0 we have changed the namespacing of Compute service. Fog::Compute::Vsphere became Fog::Vsphere::Compute as recommended by fog-core. But since we are already on v3.* this is unrelated, I would suspect

However when I do a diff between my patched gem and the version of robsee or my patched gem and the current master of fog-vsphere I see this difference in fog-vsphere-kto/lib/fog/vsphere/compute.rb

I have: RbVmomi::VIM::ClusterComputeResource Others have: RbVmomi::VIM::ComputeResource

while in vm_relocate.rb RbVmomi::VIM::ClusterComputeResource is still used in all repos

Anyone have some background on this change ? Aside of this I will go back to the current master of fog-vsphere and try to locate which part of the code is stopping nsx-t from working. Was busy just now setting up an rbenv to test fog-vsphere on it's own. Having a local patch is not really future proof for future updates of our setup so we all benefit of getting it fixed upstream :)

fwdIT commented 4 years ago

@timogoebel https://github.com/fog/fog-vsphere/pull/249 was something small :)

fwdIT commented 4 years ago

@timogoebel concerning the debugging (your setup above worked, I can now debug fog-vsphere with pry, thx!), I wonder how to set / get the extraConfig & flags advanced options of a VM object

vm = compute.get_virtual_machine('vm_name')

this gives me a set of basic key/values for the vm but none of the advanced options are visible. there are extra calls for interfaces etc but I don't see any public methods for getting/setting any of the advanced options

I have a need to set the following, which I now hardcoded in fog-vsphere:

vm_cfg = { name: attributes[:name], annotation: attributes[:annotation], guestId: attributes[:guest_id], version: attributes[:hardware_version], files: { vmPathName: vm_path_name(attributes) }, numCPUs: attributes[:cpus], numCoresPerSocket: attributes[:corespersocket], memoryMB: attributes[:memory_mb], deviceChange: device_change(attributes), extraConfig: extra_config(attributes), flags: { enableLogging: false }, }

I can set the flags and it gets correctly applied but I don't know how I can query these values from pry.

Also, towards future updates of this gem, how would I implement it correctly in the fog-vsphere module/foreman? Likely not everyone will want logging to be disabled by default for new VMs so it should be customizable in case it is allowed to be added, I guess?

timogoebel commented 4 years ago

I don't know how I can query these values from pry.

I believe you have to extend this hash: https://github.com/fog/fog-vsphere/blob/74fbbc9241b746c30ee64d2e54cd0310b8b6a6bf/lib/fog/vsphere/compute.rb#L144-L169 and add a new attribute here: https://github.com/fog/fog-vsphere/blob/74fbbc9241b746c30ee64d2e54cd0310b8b6a6bf/lib/fog/vsphere/models/compute/server.rb#L54

Please see this commit for reference: https://github.com/fog/fog-vsphere/commit/1399a4ab56bf1986e4ee95741282b62326011bad#diff-d6fc86ca52058206163ca8ae5265f5e2

fwdIT commented 4 years ago

@timogoebel thx, will have a look

any pointers on how I would request this to be added to fog-vsphere: flags: { enableLogging: false }, perhaps as a parameter but then the parameter should be brought into foreman also as a setting which can be selected in the gui

timogoebel commented 4 years ago

any pointers on how I would request this to be added to fog-vsphere: flags: { enableLogging: false },

If you add the attribute here: https://github.com/fog/fog-vsphere/blob/1399a4ab56bf1986e4ee95741282b62326011bad/lib/fog/vsphere/models/compute/server.rb#L54

It should be available in create_vm in the attributes hash, e.g. here https://github.com/fog/fog-vsphere/blob/055fb8c7a6679780f3856516c74515580a043231/lib/fog/vsphere/requests/compute/create_vm.rb#L17

I believe you know how this works from there.

fwdIT commented 4 years ago

@timogoebel thanks for the pointers, will have a look at it