upload iso to datastore #120
Conversation
OrrBG
requested review from
shellymiron,
elsapassaro,
prabinovRedhat,
anna-savina,
bardielle,
machacekondra and
mikemorency
as code owners
tests/integration/targets/vmware_ops_upload_iso_to_datastore_test/tasks/main.yml
Outdated
Show resolved
Hide resolved
tests/integration/targets/vmware_ops_upload_iso_to_datastore_test/tasks/main.yml
Outdated
Show resolved
Hide resolved
tests/integration/targets/vmware_ops_upload_iso_to_datastore_test/tasks/mock_vcsa_binary.yml
Outdated
Show resolved
Hide resolved
tests/integration/targets/vmware_ops_upload_iso_to_datastore_test/tasks/mock_vcsa_binary.yml
Outdated
Show resolved
Hide resolved
| src: "{{ upload_iso_to_datastore_src }}" | ||
| datacenter: "{{ upload_iso_to_datastore_datacenter }}" | ||
| datastore: "{{ upload_iso_to_datastore_datastore }}" | ||
| path: "{{ upload_iso_to_datastore_dst}}" |
There was a problem hiding this comment.
maybe here we can add an option to upload a file to a specific directory inside the datastore?
we can do a task before this one, that creates a folder in a specific datastore, and then upload the iso to that folder
There was a problem hiding this comment.
upload_iso_to_datastore_dst has no default value so it needs to be checked as a required variable like the others. also theres a missing space before the closing brackets
There was a problem hiding this comment.
also i think shelly's idea is good. we should have a task before this one that creates the dst path in the datastore
OrrBG
force-pushed
the
feature/upload_iso_to_datastore
branch
from
67afab8 to
8ad0e4d
Compare
| src: "{{ upload_iso_to_datastore_src }}" | ||
| datacenter: "{{ upload_iso_to_datastore_datacenter }}" | ||
| datastore: "{{ upload_iso_to_datastore_datastore }}" | ||
| path: "{{ upload_iso_to_datastore_dst}}" |
There was a problem hiding this comment.
upload_iso_to_datastore_dst has no default value so it needs to be checked as a required variable like the others. also theres a missing space before the closing brackets
| src: "{{ upload_iso_to_datastore_src }}" | ||
| datacenter: "{{ upload_iso_to_datastore_datacenter }}" | ||
| datastore: "{{ upload_iso_to_datastore_datastore }}" | ||
| path: "{{ upload_iso_to_datastore_dst}}" |
There was a problem hiding this comment.
also i think shelly's idea is good. we should have a task before this one that creates the dst path in the datastore
| - upload_iso_to_datastore_src is defined | ||
| - upload_iso_to_datastore_dst is defined | ||
| - upload_iso_to_datastore_datastore is defined | ||
| quiet: true |
There was a problem hiding this comment.
upload_iso_to_datastore_datacenter is also required
| upload_iso_to_datastore_datacenter: "{{ vcenter_datacenter }}" | ||
| upload_iso_to_datastore_datastore: "{{ shared_storage_01 }}" | ||
| upload_iso_to_datastore_src: "/tmp/{{ tiny_prefix }}-test.iso" | ||
| upload_iso_to_datastore_dst: "/{{ tiny_prefix }}-upload_iso_to_datastore/test.iso" |
There was a problem hiding this comment.
I think it's better to save the folder in a separate variable and use it here and in Delete a VM folder on given datacenter task
There was a problem hiding this comment.
i agree, you should be using a variable in the folder cleanup task instead of hardcoding the folder name. You can have two seperate variables like anna suggested or use the dirname filter:
folder_name: "{{ upload_iso_to_datastore_dst | dirname }}"
| password: '{{ upload_iso_to_datastore_password }}' | ||
| datacenter_name: "{{ upload_iso_to_datastore_datacenter }}" | ||
| folder_name: "/{{ tiny_prefix }}-upload_iso_to_datastore" | ||
| folder_type: vm |
There was a problem hiding this comment.
not required parameter for absent state
There was a problem hiding this comment.
vcenter_folder is not the correct module for what your trying to do. This module manages folders in vcenter, which are used to organize vcenter objects like vms, datastores, hosts, and networks.
You want to use community.vmware.vsphere_file with state: directory (or state: absent for cleanup).
There was a problem hiding this comment.
the vsphere_file module does not allow me to remove a directory
|
|
Orr and I talked about this, and there is no module that allows you to delete a folder in a datastore. I think having the functionality in the role is good idea. So maybe we can add a module to delete a folder in the next release, and then update this test to include that? |
I think in this case it is better to add this test only in the next release, since a large number of unnecessary folders will be created on the datastore |
| - The ISO will be uploaded into this Datastore. | ||
| - Please see the examples for more usage. | ||
|
|
||
| - **upload_iso_to_datastore_dst_folder** (str, required) |
There was a problem hiding this comment.
Is this parameter mandatory? Or can it can be set to an empty string to create the ISO in the root?
There was a problem hiding this comment.
Previously I only thought about adding a folder var to a test, not to a role itself.
|
|
||
| - name: Attempt to recreate ISO file | ||
| community.vmware.vsphere_file: | ||
| hostname: '{{ upload_iso_to_datastore_hostname }}' |
There was a problem hiding this comment.
Where did you define upload_iso_to_datastore_hostname, upload_iso_to_datastore_username, and other Auth role vars? I don't think that it is a good idea to use vars defined in some other role, not in a test itself.
No description provided.