Variable for shell profile user

[mhadam]

When running this role with sudo, I'm unable to write the shell profile to the proper user since sudo makes the ansible_user_id root. It would be nice if I could optionally set a user for the shell profile or just add a full path to the file I want written.

[fubarhouse]

@mhadam I completely agree.

It was moved away from a pretty convoluted logic system previously but I'll have a think about reimplementing it. Did you have any specific ideas?

edit: Perhaps it could be that shell profiles are ignored when a given user (the optional variable) is not defined? That would mean it wouldn't break compatibility or increase needless logic. Ideas?

edit: Perhaps it would be easier by simply providing an absolute path to a shell profile (presumably .bash_profile) which is used instead of a specified user. It would be optional and far less complex.

[fubarhouse]

@mhadam,

Any chance you can test branch issue-84 out?

The results are promising!

You can test the shell profiles by specifying golang_shell_profile as an absolute path to the desired shell profile (such as a .bash_profile found at /root/.bash_profile. It will however fail if you specify a file that does not exist.

Those tasks also only run when a different Go version is installed. I'd like to improve the idempotence to allow them to run on every run - stay tuned.

[mhadam]

Thanks so much! I'll try to test this soon and get back with results.

[fubarhouse]

@mhadam,

FYI I've noticed this is failing all tests on a particular task when the variable is undefined. Should you run into that problem, I already have a fix on the way - but it's just on the shell restart task. You can place a failed_when: false on that as an interim fix if you need to.

[fubarhouse]

The branch issue-85 includes the flow control fixes described above.

It depends on this issue merging, but it's working exactly as intended and should fix the issue originally described. It doesn't however prevent the use of sudo, but I'd like to explore that independently.

As previously discussed, I am waiting for a response - however it would be equally acceptable if you were to test against branch issue-85.

Edit: See #85

Edit: The playbook example is forced sudo, however there's no physical requirement for sudo imposed or recommended by the role. Could you please describe the need you've found to use sudo?

[mhadam]

Sorry, finally got around to this. Here's the relevant part of the settings I have right now:

GOPATH: /opt/gopath
GOROOT: /usr/local/share/go
golang_shell_profile:
  - /root/.bash_profile
  - /home/vagrant/.bash_profile

The situation is that the role is being used to setup a VM environment to test code. Honestly it may not need to be put in /opt/gopath, but that's how the project was setup when I started working on it.

An error I found - it assumes that the specified .bash_profile exists, so it throws an error when they don't. I'll patch that now.

Edit: I ended up using .profile, which exists by default it seems. So I didn't need to add create: yes to lineinfile, but perhaps that would be wise to add (I'd make the commit but I don't have permissions and forking for a one-line change seemed overkill).

[fubarhouse]

@mhadam,

It should be noted that golang_shell_profile is expected to be a string, not an array. So if it's a string which is the absolute path to the profile you want configured in the way the role executes, you can set it to /root/.bash_profile (assuming the file exists).

I'll get a commit in tonight to do a basic stat check which will prevent the tasks from executing if they don't exist. I'll update the readme as well, it's a logical decision despite the fact the variable doesn't have a default value.

So if that is in place, is any more work required here to solve the problem, am I missing the mark for what's needed, or are we able to line a new release up? I'm keen to get this one closed as it's last on the list for the next release, but I want to make sure this is completely fixed before-hand.

[fubarhouse]

The changes in #85 will be migrated into this issue after the tests have run.

Brand issue-85 should now be ready for tests, or feel free to test issue-84 after those changes come back here.

After you've run your test again, I'd appreciate it if you shared your results and what else is needed to progress this issue.

[fubarhouse]

@mhadam,

Could you please provide the results of your test in the next 7 days?

I'm keen to get this merged and released then. If there are lingering issues, we can put this off for the next release.

[mhadam]

Yup everything seems to be working fine! Thanks, and sorry for not getting back sooner!

[fubarhouse]

All good, thank you!

[fubarhouse]

#92 is closed, #93 has been opened to facilitate release of this feature.