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: fix for vmware_guest_file_operation binary file #57614

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
5 participants
@stravassac
Copy link
Contributor

commented Jun 10, 2019

SUMMARY

Fix bug #57475 for copy binary file withon python 3.X

#57475

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

vmware_guest_file_operation

@ansibot

This comment has been minimized.

@stravassac stravassac changed the title fix for vmware_guest_file_operation bianry file fix for vmware_guest_file_operation binary file Jun 10, 2019

@Akasurde Akasurde changed the title fix for vmware_guest_file_operation binary file VMware: fix for vmware_guest_file_operation binary file Jun 10, 2019

@Akasurde Akasurde requested review from Akasurde, goneri and jillr Jun 10, 2019

@Akasurde Akasurde removed the needs_triage label Jun 10, 2019

@Akasurde

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

@stravassac Thanks for the PR. Could you please write test case(s) for this change ? Thanks.

needs_info

@ansibot ansibot added needs_info and removed community_review labels Jun 10, 2019

@stravassac

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2019

@Akasurde, is it enough for you ?

- name: test
  hosts: all
  vars:
    ansible_python_interpreter: "/usr/bin/env python"
    var_my_file: "hostnamectl"
  tasks:
    - name: file
      vmware_guest_file_operation:
        hostname: XXXXXXX
        username: "{{ lookup('env','VMWARE_USER') }}"
        password: "{{ lookup('env','VMWARE_PASSWORD') }}"
        datacenter: XXXXX
        validate_certs: False
        folder: /vm/
        vm_id: test1
        vm_username: root
        vm_password: XXXX
        copy:
          src: "/usr/bin/{{var_my_file}}"
          dest: "/root/{{var_my_file}}"
          #overwrite: True

      delegate_to: localhost

Before fix
TASK [file] ************************************************************************************************************************************************************************************************
fatal: [127.0.0.1]: FAILED! => {"changed": false, "msg": "'utf-8' codec can't decode byte 0xf8 in position 96: invalid start byte"}

after
TASK [file] ************************************************************************************************************************************************************************************************
ok: [127.0.0.1]

It's also work with text file

@ansibot ansibot added community_review and removed needs_info labels Jun 10, 2019

@jillr

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

@stravassac Integration tests should look similar to the other vmware modules, using the prepare_vmware_tests role to setup the test environment and asserts to ensure test outcomes. For example: https://github.com/ansible/ansible/tree/devel/test/integration/targets/vmware_guest_facts. The directory here would be in test/integration/targets/vmware_guest_file_operation.

@goneri

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

I would prefer an unit-test for this kind of thing, it will be more efficient and faster. This being said, the fix is obvious.

@goneri

goneri approved these changes Jun 11, 2019

@ansibot ansibot added the stale_ci label Jun 19, 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.