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

VMware: vmware_datastore_facts: empty list if none found #54872

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
3 participants
@goneri
Copy link
Contributor

commented Apr 4, 2019

SUMMARY

When vmware_datastore_facts does not fine any datastore, it raises an error.
This is not consistent with the other _facts modules. It should just return
an empty list instead.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

vmware_datastore_facts

@goneri

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

@ckotte and @garbled1, I would be happy have your opinion on this.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

@goneri

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

The fact that datastores is an empty list looks more like a bug. I get a totally different result if I use the CLI:

[root@esxi-65-1:~] esxcli storage filesystem list
Mount Point                                        Volume Name  UUID                                 Mounted  Type          Size          Free
-------------------------------------------------  -----------  -----------------------------------  -------  ----  ------------  ------------
/vmfs/volumes/3653e996-cae499eb                    nfs1         3653e996-cae499eb                       true  NFS   493718847488  196671176704
/vmfs/volumes/ba306a7a-96cb7c02                    esx_lab      ba306a7a-96cb7c02                       true  NFS   493718847488  196671176704
/vmfs/volumes/b1d6dd7a-b1483205-7589-65321c883a8b               b1d6dd7a-b1483205-7589-65321c883a8b     true  vfat     261853184     107098112
/vmfs/volumes/f13d1171-bf0d6603-5a0e-a7fac42c8777               f13d1171-bf0d6603-5a0e-a7fac42c8777     true  vfat     261853184     261844992
/vmfs/volumes/5c9a297e-35d29992-1262-5254002d3a24               5c9a297e-35d29992-1262-5254002d3a24     true  vfat     299712512      83836928
/vmfs/volumes/5c9a2985-bcfccfd4-737a-5254002d3a24               5c9a2985-bcfccfd4-737a-5254002d3a24     true  vfat    4293591040    4283498496

@goneri goneri changed the title vmware_datastore_facts: empty list if none found [WIP]vmware_datastore_facts: empty list if none found Apr 4, 2019

@goneri

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

My datastores are not part of the cluster/datacenter. And so, they were ignored. I pushed #54879 to also handle this case.

@goneri goneri changed the title [WIP]vmware_datastore_facts: empty list if none found vmware_datastore_facts: empty list if none found Apr 4, 2019

@goneri goneri force-pushed the goneri:vmware_datastore_facts-empty-list-if-none-found branch to d8ad8a2 Apr 4, 2019

@Akasurde Akasurde self-assigned this Apr 5, 2019

@Akasurde Akasurde removed the needs_triage label Apr 5, 2019

msg += " for %(name)s" % module.params
msg += " in datacenter %(datacenter)s" % module.params
module.fail_json(msg=msg)
module.exit_json(**result)

This comment has been minimized.

Copy link
@Akasurde

Akasurde Apr 9, 2019

Member

This changes the existing behaviour of module exit. Please add porting guide here and add a change log entry here.

This comment has been minimized.

Copy link
@Akasurde

Akasurde Apr 9, 2019

Member

Also, add test case for this change.

This comment has been minimized.

Copy link
@goneri

goneri Apr 15, 2019

Author Contributor

Thanks @Akasurde for the review, I added a changelog fragment and there is already a test-case in #54882 for this use-case.

@Akasurde Akasurde changed the title vmware_datastore_facts: empty list if none found VMware: vmware_datastore_facts: empty list if none found Apr 9, 2019

vmware_datastore_facts: empty list if none found
When `vmware_datastore_facts` does not fine any datastore, it raises an error.
This is not consistent with the other _facts modules. It should just return
an empty list instead.

@goneri goneri force-pushed the goneri:vmware_datastore_facts-empty-list-if-none-found branch from d8ad8a2 to 520b63f Apr 12, 2019

@goneri goneri removed the needs_revision label Apr 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.