StorminStanley / st2workroom

Vagrant environment used to play with StackStorm, develop StackStorm for your environment, or develop on StackStorm itself!
Apache License 2.0
23 stars 21 forks source link

Don't restart st2installer uwsgi app during the installer run #332

Closed Kami closed 8 years ago

Kami commented 8 years ago

I have finally been able to track down the root cause for the installer convergence issue.

The problem is that st2installer uwsgi app is restarted during installer puppet run which kills the puppet run process since it doesn't run in the background which means the system doesn't fully converge. From what it looks like, this issue was there since the beginning.

Future improvement would also be to run puprun inside installer in the background, but baby steps.

Kami commented 8 years ago

Confirmed it fixes the convergence issue.

Testing it again, if it works I'll go ahead and merge it into master.

Kami commented 8 years ago

Confirmed for the second time this fixes the convergence issue, but another problem just popped up.

We run puprun in debug mode now which produces tons of output and this totally freezes up my browser. The current polling approach and DOM manipulation we use can't handle it.

We need to find a solution which allow us to keep debug mode enabled since without it it's hard / impossible to debug issues exactly like the convergence one.

Another option would be to log debug log messages to a different file than other, info level log messages, but dunno if it's possible to do that.

dzimine commented 8 years ago

What about enable debug but filter it out by installer backend? This way, debug will still available on the var/log/puppet.log?

Kami commented 8 years ago

@dzimine I think this should work, but we need to try it out. Good thing is that all debug messages are prefixed with "Debug: " so it should be easy to filter it out on the installer server side.

Kami commented 8 years ago

Actually, I just had look at the code and this won't work since the current approach is not scalable. It basically reads whole file up to line number in memory and only returns lines which number is larger than the provided line.

            for i, logline in enumerate(logfile):
                if i >= int(line):
                    data += logline.strip() + '\n'

One of the solutions would be to use the byte offset and .seek() and .tell() operations. That's more scalable and should work for this particular use case, but it would also need changes to the client.

In any case, I will leave this up to @emedvedev to work on tomorrow.

jfryman commented 8 years ago

lgtm. :+1:

Kami commented 8 years ago

@emedvedev Just a heads up - to make sure DEBUG stuff doesn't block others I branched installer and pinned test-v1.3.0 workroom branch to that branch where DEBUG is disabled.

If you want to test DEBUG mode you need to use master branch / v0.1.2 tag of st2installer.

Kami commented 8 years ago

@jfryman Btw, any ideas what I should use instead of exit 0 there?

It's not a big issue since failure is not fatal and run continues and completes just fine, just the error in the logs is slightly annoying.

Error: /Stage[main]/Profile::St2server/Adapter::St2_uwsgi_init[st2installer]/Service[st2installer]: Failed to call refresh: Could not restart Service[st2installer]: Execution of 'exit 0' returned 1:
Error: /Stage[main]/Profile::St2server/Adapter::St2_uwsgi_init[st2installer]/Service[st2installer]: Could not restart Service[st2installer]: Execution of 'exit 0' returned 1:
jfryman commented 8 years ago

true

Kami commented 8 years ago

Hah, thanks.

I was just editing my comment and wanted to say maybe it's expecting a path so maybe I should use /usr/bin/true or similar? Heh

jfryman commented 8 years ago

do this for path

path => $::path,

This used to be default behavior, but not anymore. This will fix you up.

Kami commented 8 years ago

Confirmed, /bin/true change is working, merging into master.