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

fixed generating single quote erlang config file #5

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Raffaello
Copy link
Collaborator

this fixes #4

also please after merging tag a new version and upload to ansible-galaxy.

it is reported here:

picotrading/ansible-rabbitmq#1

thanks

@Raffaello
Copy link
Collaborator Author

Raffaello commented Aug 1, 2017

replaced rpm to apt in the Debian install task strings. (tag 0.0.6)

@Raffaello
Copy link
Collaborator Author

my latest changes i marked this version as 0.1.0 fixed the cluster and some orders of operations.
also removed the code of the role-dependency and added as a dependecy in the role definition. easier to keep it updated.

@aignas
Copy link
Collaborator

aignas commented Oct 12, 2017

@jasonroyle, @Raffaello, I was wondering what was the status on this. I am keen on seeing the cluster creation parts improved in this role and it would be great if these improvements could be merged into the main package.

Is there anything in particular that is holding this up?

Copy link
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to sort out the vhost and the user management issues before merging this. This might be difficult to test unless running the role on a clean VM, so it would be good to have a test setup similar to how it is described in https://www.ansible.com/blog/testing-ansible-roles-with-docker, but this may out of scope for this PR.


- include: vhosts.yml

- include: users.yml
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I noted in #6, this will fail when creating new users (at least it did for me). It would be good to only run this on the master node.

owner: rabbitmq
group: rabbitmq
mode: 0400
when: rabbitmq_erlang_cookie is defined
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this. It will make it easy to make the erlang_cookie setting optional. You only really want to set it once, during the initial setup and when doing maintenance updates it might be a good idea to leave it out.

This of course may introduce a problem when adding new nodes to the cluster. But I guess that this is a good enough trade-off in this case.

Copy link
Collaborator Author

@Raffaello Raffaello Oct 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it could be a problem adding new nodes. It should be more parametrized, but still for a new node you still can define that cookie from ansible CLI, so it should be possible.


- include: node_fullstart.yml

- include: vhosts.yml
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may fail when creating new hosts. Similar to #6, however, I have opened a separate issue for this: #7.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now with ansible 2.4 there should be improvements on these rabbitmq functions. Also it should support TLS that was a requirements for me with this role, so until ansible 2.3 there were issues with TLS and certificates. Now those should be fixed. Not sure. I suspended working on this one.

@Raffaello
Copy link
Collaborator Author

@aignas I suspended the work on this role at the moment, i might reprise if there are new goals to achieve.
Also it should be review with ansible 2.4 that has improvement on the RabbitMQ functions.
I think it is a "dead" role this one. If you have tried to use my fork of this role it might worth moving forward from there and create a new ansible galaxy role and so on... just wait some times about a better answer from @jasonroyle

@jasonroyle
Copy link
Owner

Hi guys, I like to be quite thorough when accepting pull requests and testing it all first but I just haven't got round to doing this just yet, I do still intend to come back to it at some point but my concentration has been elsewhere.
If you would like to help out by becoming a collaborator I would be willing to invite you.
Let me know 👍

@aignas
Copy link
Collaborator

aignas commented Oct 14, 2017

@jasonroyle, I'd like to become a collaborator, so let me know how we can work on it. :)

@Raffaello
Copy link
Collaborator Author

@jasonroyle me too, I would like to be a collaborator.


@aignas I do not have really too much time now to work on those changes you have requested. I do not neither remember now and i do not have your perspective on those one. It means at this present time it is impossible for me to commit those changes.


I will be back to work on this in the near future probably, not sure when might be a week or a little bit longer. I can try in free time, but those

@Raffaello
Copy link
Collaborator Author

@aignas I think: The solution it would be merging this one, tag this actual version publish to ansible galaxy, and moving forward from this. so you will work from this point on the changes you requested.

We can do evolutionary, version based role. canot do to many changes in 1 PR.

@Raffaello
Copy link
Collaborator Author

Raffaello commented Mar 6, 2018

Hi @aignas, could you please fix the user issue reported here so it would be possible to make it ready to merge this one? I do not have time now.


@jasonroyle Should we consider to make PR against ansible >= 2.4 only? because for rabbitMQ modules there are some improvements with 2.4 especially for TLS

@Raffaello Raffaello mentioned this pull request May 1, 2018
@Raffaello
Copy link
Collaborator Author

@aignas for the test related: i might can do with a full VM, docker container and rabbitmq managed with systemd doesn't work (it is stuck on systemd start rabbitmq-server) and anyway when reach that level make sense using the rabbitmq docker image not a "clean OS" to provision. Also it would not be a real test scenario, i guess the scope and goal of this Role is to provision a full OS not a container.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rabbitmq_config error generating single quote value
3 participants