Support installation by non-root users

[hkcomori]

Closes #153

• Added support for running without become: true except for `rclone_mounts
• Added support for changing the location of manpages
• Added support for changing the location of binary
• Added support for avoiding installation of dependent packages (to avoid apt execution errors by non-root users)

[hkcomori]

Sorry, test-archlinux in the workflow fails, but I don't know how to fix it.
Any advice would be appreciated.

[gamezftw]

Hello @hkcomori , can you tell me if these tests are passing for you locally? I do not get these errors when I try to run it locally...

[hkcomori]

Sorry @gamezftw , I have never used Molecule locally.
I will try to prepare it now.

[stefangweichinger]

Thanks for your work to both of you. I currently try to have a bit of vacation time, so allow some delay on my side. Thanks!

[stefangweichinger]

Hello @hkcomori , can you tell me if these tests are passing for you locally? I do not get these errors when I try to run it locally...

maybe because you test on arch-linux?

The warning comes from the idempotence-test ....

[gamezftw]

Hello @hkcomori , can you tell me if these tests are passing for you locally? I do not get these errors when I try to run it locally...

maybe because you test on arch-linux?

The warning comes from the idempotence-test ....

From what I understand, molecule uses docker containers to run the tests, so I am hoping that host OS doesn't have much to do with the tests.
I ran all the tests and especially idempotence test mutiple times locally and it passed every time...
I am checking if there is something that is different in the workflow then when running locally, or if there is some package version that causes this, but it would be good to know if this test is failing locally in case of different OS.

[stefangweichinger]

From what I understand, molecule uses docker containers to run the tests, so I am hoping that host OS doesn't have much to do with the tests.

Sure, you are right: that's the idea behind molecule: use containers to provide the various OSes without dependency on the host system. I was too fast with my reply.

I ran all the tests and especially idempotence test mutiple times locally and it passed every time... I am checking if there is something that is different in the workflow then when running locally, or if there is some package version that causes this, but it would be good to know if this test is failing locally in case of different OS.

first: I stll would prefer to have all tests in one single scenario. Now we have that special case with arch linux and two scenarios to maintain and debug.

Aside from that: the problem in the idempotence test seems to be related to creating a temporary directory:

https://github.com/stefangweichinger/ansible-rclone/actions/runs/10111247840/job/27962820429?pr=162#step:5:573

the temp directory is re-created in the 2nd run of ansible which is a "change" in terms of ansible. Idempotence would mean: no change in the n-th run.

The create task is here

I wonder if we should use the tempfile module here instead.

Why that succeeds locally and fails on github: I assume github actions does things different somehow. That's not a very competent answer, I know ;-)

[stefangweichinger]

Also note this tag. This makes molecule skip the task in the idempotence test.

I have to think about what's the best solution here.
EDIT: we should try to not have to skip tests ...

[gamezftw]

first: I stll would prefer to have all tests in one single scenario. Now we have that special case with arch linux and two scenarios to maintain and debug.

I did not like this idea too much either but I was not able to make the tests work for arch because of issues with cgroups here. I will try to check again if this can be solved somehow so it uses the same scenario

Aside from that: the problem in the idempotence test seems to be related to creating a temporary directory:

https://github.com/stefangweichinger/ansible-rclone/actions/runs/10111247840/job/27962820429?pr=162#step:5:573

the temp directory is re-created in the 2nd run of ansible which is a "change" in terms of ansible. Idempotence would mean: no change in the n-th run.

The create task is here

Locally this tests returns ok instead of changed and that is probably why it is passing, and in the pipeline that you linked it returns changed... I will spend some more time to try to figure it out

I wonder if we should use the tempfile module here instead.

This looks interesting, and it looks like that it can replace rclone_setup_tmp_dir, but don't take my word for it since I did not try it.

Also note this tag. This makes molecule skip the task in the idempotence test.

I have to think about what's the best solution here. EDIT: we should try to not have to skip tests ...

Keep us posted on this since I don't know how to make sure that it would pass idempotence test and also how Create temporary working directory would pass it.

[stefangweichinger]

Trying the new module now in this PR ;-) and I didit wrong in the first try ...

EDIT: it seems that "ansible.builtin.tempfile" creates a random temp-dir below the given "path:"

Maybe we have to rethink the flow: removing the temp-dir before creating it should make sure it's empty, that's the intention.

[hkcomori]

Thanks for the advice, everyone.

changed_when: False can forces ignore changed for ansible.builtin.tempfile.
However, it is always ignored, which means that the contents of tempfile are excluded from the idempotence test.

I think it's no problem, because it's a "temporary" directory.
How about you?

[stefangweichinger]

I currently also test things in my local checkout of this PR. Wait a sec.

[stefangweichinger]

I should have done my changes in a second branch, sorry. Let's see if that tests now OK in gh.
I wonder if they use docker containers also, or if that's podman or so (might explain the differences).

[stefangweichinger]

Seems I broke something. 'rclone_setup_tmp_dir' is undefined.

[stefangweichinger]

76479bb is now problematic ... I pause here for a moment to not interfere with your patches ... let's sync ;-)

[stefangweichinger]

I currently think ansible.builtin.tempfile is not the module to use. Therefore we have to roll that back.

[hkcomori]

Ah, I remove 'rclone_setup_tmp_dir' in 76479bb .
But reverts is welcome.

Sorry to disturb you.

[stefangweichinger]

Ah, I remove 'rclone_setup_tmp_dir' in 76479bb . But reverts is welcome.

Sorry to disturb you.

no problem. I am not used to team-work on a PR so I am also disturbing you in your PR ...

[stefangweichinger]

Hm, archlinux-stuff fails again.

Couldn't archlinux go into molecule/default/molecule.yml as a second platform?

Also see what I added there (volumes:, tmpfs:, cgroupns_mode:): these settings might fix your cgroup-issues.

[hkcomori]

Actually, the local Molecule is not working well yet...
I'm developing it on Ubuntu in WSL2, but it doesn't seem to work with Docker in any way.

Back to the main topic, do we need to delete 'rclone_setup_tmp_dir' in the first place?
If we don't delete it, it doesn't re-create the directory or re-download the files, so I don't see any idempotence issues.

[stefangweichinger]

Back to the main topic, do we need to delete 'rclone_setup_tmp_dir' in the first place? If we don't delete it, it doesn't re-create the directory or re-download the files, so I don't see any idempotence issues.

Hm, maybe. I thought starting from scratch might be better. Just "a feeling" ;-)
Sure, if the downloaded files were different in a following run the should be replaced by ansible anyway.
Worth a try, sure.

[hkcomori]

Sure, if the downloaded files were different in a following run the should be replaced by ansible anyway.

Ah, your feeling is correct.
If the old files are still there, ansible.builtin.unarchive does not reflect the removal of files upstream.
If I could download them with ansible.builtin.git, I wouldn't have to worry about that.

[hkcomori]

In general, there is a problem with deletion, but as far as rclone is concerned, I don't see a problem with overwriting by unarchive.
Since this role only copies a fixed file, no other files are involved, and if the expected file is deleted, this role will have to be modified anyway.

[stefangweichinger]

OK, I am drifting off here and try to solve the problem with platforms/scenarios etc

I check https://github.com/CarloDePieri/docker-archlinux-ansible for examples etc

[stefangweichinger]

I have to pause now ... the direction is OK, but it seems it needs your "test-archlinux" in .github/workflows/molecule.yml.
Feel free to re-add that here and re-run the gh actions ... I think I am close.

edit: did so already ...

[stefangweichinger]

my local tests look promising, I don't know where the conflicts here are right now.
@hkcomori maybe you can take a look in the meantime. Out for a break ...

[gamezftw]

Sorry for jumping back and forth with the context of the conversation,
nice job with this one 1b5393b , for me for this to work locally I needed to add:

    - name: Update repositories cache on Archlinux
      community.general.pacman:
        update_cache: true
      when: ansible_facts.os_family == 'Archlinux'
      register: pacman_result
      changed_when: pacman_result.cache_updated
      become: true

to converge.yml of the default scenario, similar step as for Debian update cache.

[gamezftw]

What was the issue with this the change for workflows here, were both platforms running for each distribution and release and version?
If so, we should update test-archlinux to use default scenario now, and we need to pass MOLECULE_PLATFORM_NAME to both jobs, since test-archlinux should use MOLECULE_PLATFORM_NAME=rclone-arch and for test job I am not yet sure what to pass since the name is built from multiple variables, so maybe we can make that one fixed also?

[stefangweichinger]

I might have mixed up things while trying to consolidate the tests.
I consider stepping back and open a new PR for that consolidation issue, separating it from the topic here.

Getting the terms right isn't that easy: scenarios, platforms, distros ... phew!
(I am not that competent in designing molecule-tests ... but I would like to get that right for this role)

[stefangweichinger]

Yes, sounds good. Go for it. If I/we manage to get #164 working, we might at first merge that and rebase #162 based on that, ok?

[stefangweichinger]

Sorry for jumping back and forth with the context of the conversation, nice job with this one 1b5393b , for me for this to work locally I needed to add:

    - name: Update repositories cache on Archlinux
      community.general.pacman:
        update_cache: true
      when: ansible_facts.os_family == 'Archlinux'
      register: pacman_result
      changed_when: pacman_result.cache_updated
      become: true

to converge.yml of the default scenario, similar step as for Debian update cache.

This should go into a new commit, please. I might add it here or in #164 ... maybe a separate PR is overkill

[hkcomori]

Yes, sounds good. Go for it. If I/we manage to get #164 working, we might at first merge that and rebase #162 based on that, ok?

OK, let's follow that plan.

[hkcomori]

I rebased this PR to the main branch.
Up to 7a8292c is the commit when the PR was created, and after that are the fixes about testing problems.

[stefangweichinger]

I rebased this PR to the main branch. Up to 7a8292c is the commit when the PR was created, and after that are the fixes about testing problems.

Great. This merge should result in a new tag/version also, let me get that sorted before merging (and releasing to galaxy).

[hkcomori]

I found a mistake when I checked the README, so I fixed it.

[stefangweichinger]

I found a mistake when I checked the README, so I fixed it.

this won't go into release 0.2.0 anymore, the tag is already set ... ;-)

[hkcomori]

Oh...sorry, I thought I could still make it before the merge, but it was too late.

[stefangweichinger]

Let's open a new PR for some editing etc
I also have to remove a comment etc

Minor cleanups don't have to become a release IMO

[stefangweichinger]

I found a mistake when I checked the README, so I fixed it.

In branch main it's correct already, afai see (?)

[hkcomori]

I fixed it right before the merge.
But it was after I tagged it.

d33aaee

[stefangweichinger]

The release step wasn't executed. Now I finished that: 0.2.0 is on galaxy. Pls test, hopefully we didn't break anything ;-)

[hkcomori]

Thanks! It works fine in my usecase.