-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
Updating to use deb packages #42
Conversation
This is awesome! |
Thanks. Don't know why Circle CI is failing though... |
@johandahlberg It failed on https://github.com/StackStorm/ansible-st2/blob/master/circle.yml#L28
If you login you might be able to re-run with debug ssh and can ssh to the container to see why that failure happens. |
It seems that the Circle CI still has old rabbit-mq configurations files lying around. Hopefully this will fix that.
@manasdk I've been investigating this a bit closer now. One thing that I noticed now is that Circle CI is using Ubuntu 12.04 as the base image, while the install instructions for Ubuntu / Debian states that Ubuntu 14.04 is supported. When trying this our in my own repo, triggering Circle CI everything worked when I updated the configuration to build on a Ubuntu 14.04 box. Would it be possible to make those changes the configuration of the main repo as well? This should also make the builds consistent with the supplied Vagrant file. |
- name: Install exact st2-web package | ||
sudo: yes | ||
apt: | ||
name: st2web={{ st2_version }}.{{ st2_revision }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
st2web
has independent revision number.
Eg. revision number for st2
could be 150
, while for st2web
latest revision number could be 6
.
The change itself looks logically sound to me and I can confirm the playbook produces working copy of st2 (including mistral and nginx deployment of st2web, but no chatops). What I would like you to do is to clean up the code a little bit: remove extra empty lines, readme.md files in roles (or fill them appropriately), empty files. I'm also kind of worried that name of the company in role meta may raise questions. I'm all for attribution, but since I'm myself struggling to figure out who we are now, we should probably escalate it to @dzimine. |
@armab Thanks for the comments. I'll make the changes you suggest. @enykeev I've not added that chatops things since we're not using that part of the StackStorm functionality around here. I can for sure clean things up a bit. When it comes to the readme's do you have any template for that I could have a look at to see what to include? I have no problem changing meta files - or removing them completely for the time being (since I guess they are only really necessary when publishing to galaxy). Do you have any contribution guidelines somewhere detailing what to do with those things? |
@johandahlberg I don't think we have any kind of guidelines or templates for such things. Deleting meta and readme for now seem like a good idea, though. |
@@ -1,4 +1,4 @@ | |||
mistral_version: 0.13 | |||
mistral_version: 1.3.2-99 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.4.0
is latest mistral stable version from https://packagecloud.io/StackStorm/stable. Let's stick to some revision from that version.
On related note, I would prefer to avoid such pinning in default config values. So if version is not explicitly set, playbook installs latest stable by default (with accordance with st2
version).
Anyway, it's something to rethink later. For now it's good enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you mean something like what's used here: https://github.com/johandahlberg/ansible-st2/blob/use_deb_packages/roles/st2/tasks/main.yml? If so I can fix it now, shouldn't be to tricky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, something like that would be definitely better.
@armab Thank you for all the great comments. I've addressed them now and pushed the changes. |
Another very minor issue: can you also delete that big WARNING (first lines) from the |
Not a problem. 😃 |
Everything else looks good for me 👍
For now @enykeev will come back later and review changes once again & merge. |
@armab No problem at all. I'm just happy to be able to contribute, and that once this is merged there it makes it it easier for us to deploy the latest st2 version around here. 😄 |
with_items: st2_services | ||
command: st2ctl restart | ||
|
||
- name: restart st2api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to also restart st2stream along with st2api for pretty much every case. See https://github.com/StackStorm/st2-packages/pull/293/files. If you don't do that, attempt to connect to stream with token will result in an error, something along the lines 'unexpected query parameter x-auth-token'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And while you at it, I'd also ask you to change st2 restart command to reflect https://github.com/StackStorm/st2-packages/pull/285/files
The rest looks good to me 👍 |
Thanks for the feedback @enykeev. I've made sure the |
Synchronize docs with recent #42 which installs new packages
Merged, |
I've update the roles and playbook to use the new deb-packages to install st2. Essentially I've converted the install guide to a playbook.
Sorry for the massive changes. I've tried as much as possible to dived this into commits that make sense, but I might have mixed things up in some places. I can now build this in the supplied Vagrant box, and I've also added a small smoke test role, which should confirm that some basic things are working as expected.
There are probably some things which still need fixing, especially in relation to making sure that all customization relating to the variables are working, but this is at least a working draft.