Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

feat2RFC(vcd_vapp_vm_nic): Do a module invocation better #131

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

westsouthnight
Copy link

Issue details, RFC

Pre conditions and Info:

  • Module need to be refactored - vcd_vapp_vm_nic

  • Module role example must to be relevant and updated which we update the modules, not after few mouth after make changes: Folder & Role tasks

  • We must so much possible do threads are is parts of invocation in module shortly, simplificated on data target operations for update / get / set, without "between" temporary vars and workarounds such as run external executes module - itself. for able check / validate / update something in VM, vApp, vNetwork or other resource, in our target case that correct use as example - [Role tasks] (https://github.com/vmware/ansible-module-vcloud-director/blob/master/roles/vcd_vapp_vm_nic/tasks/main.yml)

  • Currently, in parent library - pyvcloud possible (but im find them and already checked) module contains presents Issue, which not give able work updated and proposed module directly complete success now, because itself pyvcloud library contain bug, - when we add the first adapter and try to return list of nics, we get Exception with message "primary_index referenced before assigment", but NIC will be added (see at Currently Results Screens).

Example and describes:

  • When i have a presented vApp with one VM, and after adding second VM maked from other "Template VM Image" (possible that template no have any adapter when his have on born as template, in process of convert), - we wants have network, for that we will be need add/validate Network adapters and check/perform attaching they to second VM, - but in invocations running with differents behaviors, and types of parameters we everytime get exceptions with big precent of happens of anything, what not can be possible to use on production codebase.

  • Way which proposed in this module example is not are native and need to multiple calls, save the temporary between tasks results for able check what we need do next. In initial works with VM NICs we need try to use one call check for prevents no needed time leak, strange and not native for most people way for do a simple thing.

Expected and wants results:

  • When we set nic_id, for operate some NIC on VM, that Network adapter will be added if his not are exists
  • No cases and crashes when adding first networks adapter
  • Validate the NIC which wants to update/add/check with adapter index compared con all readed nics indexes list, for check already presents and attached to VM. If not are presents, go add needed NIC with presented index.

Currently results:

ORIGINAL vcloud CLASS  settings - vcd library VM class which give exception on usage

ORIGINAL vcloud CLASS  exception image - vcd library VM class which give exception on usage

ORIGINAL vcloud CLASS  state after exception - vcd library VM class which give exception on usage

  • Different cases - give different behavior, no a simple way to give always same behavior and success completes validate/check/set/get/update for everything possible configuration of parameters on operation with NIC.

Pull Request wants:

  1. Resolve parent pyvcloud library Issue, because its blocks mainstream fix of ansible module - When we add_nic() are first, after successful complete adding module fail with error about referenced variable which called before assigment.

  2. Replace/add the code with validation by one call to ansible-module-vcloud-director after cleans PR/changes/updates on stages which supposed in that RFC about PR.

  3. Have an updated examples and documentation.

Usage after (in test case i'm used local copy of VM class with my fix of primary_index)

- name: '[eth0] Attach NIC to VM on Direct Network in vCloud vApp Zone'
  vcd_vapp_vm_nic_validate:
    org: "FBI"
    user: "api_username"
    password: "api_username7supers6"
    host: "vcloud.example.com"
    vm_name: "{{ item['value']['name'] }}"
    vapp: "matreshka-production-dc911rs-moscow"
    vdc: "FBI_VDC23"
    network: "vZone_Sector_Network_Exchange_matreshka_routed_production_dc911rs"
    ip_allocation_mode: "DHCP"
    is_connected: True
    verify_ssl_certs: True
    api_version: '31.0'
    nic_id: "0"
    is_primary: 0
    nic_ids: 
      - "0"
      - "1"
    adapter_type: "E1000"
    state: "present"
    verification: "meta"
    operation: "validate"
  with_dict: "{{ cloud_bootstrap.oz_second_group }}"

- name: '[eth1] Attach NIC to VM on Direct Network in vCloud vApp Zone'
  vcd_vapp_vm_nic_validate:
    org: "FBI"
    user: "api_username"
    password: "api_username7supers6"
    host: "vcloud.example.com"
    vm_name: "{{ item['value']['name'] }}"
    vapp: "matreshka-production-dc911rs-moscow"
    vdc: "FBI_VDC23"
    network: "vZone_Sector_Network_Exchange_matreshka_direct_production_dc911rs"
    ip_allocation_mode: "DHCP"
    is_connected: True
    verify_ssl_certs: False
    api_version: '31.0'
    nic_id: "1"
    nic_ids: 
      - "1"
      - "0"
    is_primary: 0
    adapter_type: "E1000"
    state: "present"
    verification: "meta"
    operation: "validate"
  with_dict: "{{ cloud_bootstrap.oz_second_group }}"

MODIFIED vcloud CLASS  module load with primary_index fix - import and loading for use  - vcd library after fix VM class works fine

MODIFIED vcloud CLASS  module output with primary_index fix - create first NIC without error - vcd library after fix VM class works fine

MODIFIED vcloud CLASS  module output (2) with primary_index fix - create first NIC without error - vcd library after fix VM class works fine

MODIFIED vcloud CLASS  state after success complete - create first NIC without error - vcd library after fix VM class works fine

Short way for conversate in raw way:

telegram - @westsouthnight

1. Do complete validation needed NIC by nic_id and add if his not are presents
2. Do a 3-steps way - 
     I. Verififaction (meta) - read_nics() and return states, if no are adapters presents on exec, add the first one.
     II. Check and manage wants state - "present" as default by the way - go add NIC only if his not in read_nics returned list
     III. Operation (validate): Last third invocation which operate NICs and "operation" key have for validation flow value = "validate" - it's say module go to validate_nics(), - gathering results, merge and view executed earlier steps (I,II)
@vmwclabot
Copy link

@westsouthnight, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

@westsouthnight
Copy link
Author

@westsouthnight
Copy link
Author

Please, dear community and all devs! Please, if it possible look to this, really, little but big problem - which we can resolve now and close Issue faster then speed of light - merge that PR. No want stack on stuck after so long investigation, asking to your hearts, please without regredness. ASAP!

@mukultaneja
Copy link
Collaborator

mukultaneja commented Jul 29, 2020

@westsouthnight thanks for the PR. I have looked into the PR and would like to request two things,

  1. It would be so helpful if you could raise an issue and explain the case there. It really would help me to understand what use case we want to target through this PR.
  2. Could we add the verification nic functionality in the existing module instead of creating a new module?

@westsouthnight
Copy link
Author

In each type of api request which we ask vcloud director about something (vm, vm_nic, vm_disc, etc) - in each module firsts you must split invocation to 3 times: Read, Validate, Perform. On this 'pipe' of asks and responses to itself object you must detect cases when library pyvcloud (SDK) trow uncatchable exception, you need know this cases for each over, and implement it. So like for example im describe and explain in my RFC request.

Yes, you need and must (i hope you can with help refactor this modules to perfect level) add this functionality to current have in repository module, by the way im only try to show you correct way for able every time to get a good exits instead of unhandled exceptions after known types of api requests and know list of possible exceptions. You can use my code for example.

Sometimes without good output we lost a basic code navigation, which give misunderstanding invocation pipe.

https://github.com/vmware/ansible-module-vcloud-director/pull/131/files#diff-7818a6307bd2b2a6134138acbd7e43ecR475


module = VappVMNIC(argument_spec=argument_spec, supports_check_mode=True)

if module.params.get('verification'):
Copy link
Author

Choose a reason for hiding this comment

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

remark one: SPLIT TO
PIPE Read, Validate, Perform.

  1. .manage_meta() - get the whole object, - and try must exit from any possible case, because all this cases are knows by PyVcloud SDK interface which we use for operate object - this is classes Vapp & VM which we use in modules. Basicly must every time out, if nothing happen : simple "No data"'

  2. .manage_state() - by reason getted from previous step (mange_meta()) this function perform needed tasks and calls, for able this module exit correct in must of cases.

  3. .mange_operation() - based, on my example i call to this by 'validate' - at that place we can go around with some method like back to meta with other thing, for able run from first module step with cointain information what we need if first are go down.

Copy link
Author

@westsouthnight westsouthnight Aug 4, 2020

Choose a reason for hiding this comment

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

[THIS ONLY FOR EXAMPLE HOW YOU CAN SPLIT INSIDE, BUT WORKS] @mukultaneja

@westsouthnight
Copy link
Author

@mukultaneja so because parent fix are a merged in pyvcloud library in next week i have a few days of free dev time, and do some couple of fixes for current issues.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants