sapcar_extract: Add overwrite mode and improve exist validation

[marcelmamula]

Changes

• Add new optional parameter overwrite which allows extraction regardless if files are present. We cannot validate that extracted files are what is in archive, only file names. Overwrite mode allows users to trigger overwrite like mentioned in sapcar_extract - bad check_if_present #71 by @crysaki @nbttmbrg

• Add path validation and clearer messages, instead of incorrect message that files are already present. Mentioned in sapcar_extract - "OK: already unpacked" when path not exists #72 by @crysaki

  • Additional validation for permissions is added.

• Small refactoring for clarity and readability, not code change.

Tests

Tested on SLES_SAP 16.0 with BW4HANA system.

path points to wrong archive, no longer success message:

msg: 'File missing: File defined in the "path" parameter does not exist: /software/SAPEXEDB_51-70007806.SARx'

dest contains expected file names, overwrite mode is disabled (default):

stdout: Expected file names were found in /tmp/sapcar_extract folder. No extraction needed.

dest contains expected file names, overwrite mode is enabled:

        stdout: |-
            SAPCAR: processing archive /software/SAPEXEDB_51-70007806.SAR (version 2.01)
            x db4cncl
            x dbdb2slib.so
            x dbdb4slib.so
            x dbdb6slib.so

[sean-freeman]

LGTM functionally.... however I'd prefer an altered stdout that specifically states files were overwritten? I can foresee issues with end users and an overwrite param

[marcelmamula]

LGTM functionally.... however I'd prefer an altered stdout that specifically states files were overwritten? I can foresee issues with end users and an overwrite param

@sean-freeman stdout is complete output from command. I can change our comments to output into msg where modules usually output and leave stdout for command output.
Or prepend SAPCAR message SAPCAR: processing archive /software/SAPEXEDB_51-70007806.SAR (version 2.01) with our comment.

[marcelmamula]

Updated output after Sean's comment:

        changed: true
        msg: Files extracted to /tmp/sapcar_extract
        stderr: ''
        stderr_lines: []
        stdout: |-
            SAPCAR: processing archive /software/SAPEXEDB_51-70007806.SAR (version 2.01)
            x db4cncl
            x dbdb2slib.so
            x dbdb4slib.so

        changed: false
        msg: Expected file names were found in /tmp/sapcar_extract. No extraction needed.
        stderr: ''
        stderr_lines: []
        stdout: ''
        stdout_lines: []

        changed: true
        msg: Files extracted to /tmp/sapcar_extract (overwrite mode enabled)
        stderr: ''
        stderr_lines: []
        stdout: |-
            SAPCAR: processing archive /software/SAPEXEDB_51-70007806.SAR (version 2.01)

[sean-freeman]

@marcelmamula Good to merge 🙂 Thank you for taking my concern into consideration

[marcelmamula]

@nbttmbrg @crysaki Are you able to double check if it solves issues that were raised?

[nbttmbrg]

Tested the following and LGTM too :

TASK [First extract] *******************************************************************************************************************************************************
changed: [localhost]

TASK [Second extract, this should not change anything because overwrite is false] **********************************************************************************************************
ok: [localhost]

TASK [Delete one of the extracted files to simulate a change in the dest] ******************************************************************************************************************
changed: [localhost]

TASK [Extract again, this should change because one file is missing but overwrite is false] ************************************************************************************************
changed: [localhost]

TASK [Third extract, this should change because overwrite is true] ************************************************************************************************************************
changed: [localhost]

[nbttmbrg]

Wait, something is not exactly on par with what was planed, this is the result with overwrite: true ; shouldn't it specify it in the msg ? :

    "sapcar_extract_result": {
        "changed": true,
        "command": "/tmp/sapcartest/SAPCAR -xvf /tmp/sapcartest/dw_1514-70000596.sar -R /tmp/sapcartest/destdir",
        "failed": false,
        "failed_when_result": false,
        "msg": "Files extracted to /tmp/sapcartest/destdir",

[marcelmamula]

@nbttmbrg This message is generated under 2 cases:

• dest is empty
• dest is missing some of files in archive

Overwrite message is always addition, so it must be missing in your parameters.

        if params['overwrite']:
            msg += " (overwrite mode enabled)"

[nbttmbrg]

@marcelmamula Sorry my bad, typo in my register for my last task, I was debugging the wrong var.

I see the correct message :
"msg": "Files extracted to /tmp/sapcartest/destdir (overwrite mode enabled)"