lambdacd icon indicating copy to clipboard operation
lambdacd copied to clipboard

Manual trigger doesn't work sometimes (with ring-reload)

Open kwladyka opened this issue 7 years ago • 12 comments

12:40:49.838 [qtp1400469930-68] INFO  lambdacd.steps.manualtrigger - received parameterized trigger with id  c2823d46-93ee-4253-8d7a-8dc74fc43922  with data  {}

I am testing lambda cd with lein ring server and a lot of time i see this message after click on manual trigger to run and nothing is happening. I have to turn down everything and run again. Then new build is set by default after start the app and works.

Edit: Probably each time when i change pipeline. How to deal with it to not restart app all the time?

kwladyka avatar Jan 16 '17 11:01 kwladyka

Hmm, if I understand you correctly, you are using lein-rings auto-reload, keeping the process running and reloading changed source-files on the fly. I actually never tried this but I can imagine things don't just work out of the box in this case.

It's an interesting feature though so if you share some sample code that reproduces the issue I'd be happy to play around with it, see if there's something we can do about it.

flosell avatar Jan 16 '17 19:01 flosell

(def sb
  `(
     (alias "Triggers"
            (either
              (steps/wait-for-git sb-repo sb-refs)
              manualtrigger/wait-for-manual-trigger))
     (alias "Test and build"
            (with-workspace
              (steps/clone sb-repo)
              git/list-changes
              steps/lein-test))
     (alias "Deploy"
            steps/info)
     #_(alias "Post-deploy"
              steps/slack)
     ))

But from what i saw it happens with whatever pipeline. Especially when change pipeline code, but maybe also steps. Not really sure, because on the end i have to restart the app all the time to be sure it is actual. It happens all the time.

kwladyka avatar Jan 16 '17 23:01 kwladyka

Could you provide the whole project? I'm interested in how LambdaCD is configured and initialized and how lein-ring is configured

flosell avatar Jan 17 '17 08:01 flosell

(defproject sb-pipeline "0.1.0-SNAPSHOT"
  :description "FIXME: write description"
  :url "http://example.com/FIXME"
  :dependencies [[lambdacd "0.12.1"]
                 [ring-server "0.4.0"]
                 [org.clojure/clojure "1.8.0"]
                 [org.clojure/tools.logging "0.3.1"]
                 [org.slf4j/slf4j-api "1.7.22"]
                 [ch.qos.logback/logback-core "1.1.8"]
                 [ch.qos.logback/logback-classic "1.1.8"]

                 [lambdacd-git "0.2.0"]
                 [http-kit "2.2.0"]
                 [cheshire "5.7.0"]]
  :profiles {:uberjar {:aot :all}}
  :plugins [[lein-ring "0.9.7"]]
  :ring {:handler sb-pipeline.core/app
         :init sb-pipeline.core/start-pipelines}
  :main sb-pipeline.core)
(ns sb-pipeline.core
  (:require [sb-pipeline.pipeline :as pipeline]
            [ring.server.standalone :as ring-server]
            [lambdacd.ui.ui-server :as ui]
            [lambdacd.runners :as runners]
            [lambdacd.util :as util]
            [lambdacd.core :as lambdacd]
            [clojure.tools.logging :as log]

            [compojure.core :as compojure]
            [ring.util.response :refer [response not-found]]
            [lambdacd.event-bus :as event-bus]
            [lambdacd-git.core :as git]
            [clojure.core.async :as async]

            [sb-pipeline.slack :as slack])
  (:gen-class))

(defn failure-notifications [ctx]
  (let [subscription (event-bus/subscribe ctx :step-finished)
        steps-finished (event-bus/only-payload subscription)]
    (async/go-loop []
      (let [event (async/<! steps-finished)]
        (when (= :failure (get-in event [:final-result :status]))
          (slack/send-message event)))
      (recur))))

(def pipelines
  {:sb (lambdacd/assemble-pipeline pipeline/sb {:home-dir "builds/sb"
                                                :name "hidden"
                                                :max-builds 3
                                                :ui-config {:expand-active-default true
                                                            :expand-failures-default true}})})

(def routes (compojure/routes
              (compojure/context "/sb" [] (ui/ui-for (:sb pipelines)))))

(def app routes)

(defn start-pipelines []
  (git/init-ssh!)
  (runners/start-one-run-after-another (:sb pipelines))
  ;(failure-notifications (:context (:sb pipelines)))
   )

(defn -main [& args]
  (start-pipelines)
  (ring-server/serve app {:open-browser? true
                          :port 8080}))

For now it looks like that. Please let me know if this is what you want or should i push on github whole project for a while.

kwladyka avatar Jan 17 '17 18:01 kwladyka

Thanks, looks good, I hope I’ll have some time to play around with it over the weekend.

flosell avatar Jan 18 '17 07:01 flosell

I now had some time today playing with this and I think I figured out what happens there: When you make a change, lein-ring reloads the source (sb-pipeline.core among them), like you probably expected. This means assemble-pipeline is called again, instantiating a new instance of LambdaCD and (as routes are also reloaded) a new instance of the UI, pointing to the new instance of LambdaCD.

You might a couple of questions here:

  • Why do I still see the old builds? This is because the new LambdaCD instance initially loads the history from the same home-dir where the other LambdaCD instance just wrote to.
  • Then why does the old build look stuck and I can't interact with it? This is because the new LambdaCD instance also has a new event-bus and knows nothing of the old one, therefore doesn't know about updates still happening in the old instance and manual-trigger events go to the new instance that doesn't know about the old build still waiting there.
  • Then, if I get a new LambdaCD instance, why don't I also get a fresh build? This is because the runners are responsible for this and since lein-ring doesn't call :init on reload, no new runners are created.

With this in mind, we need to do a few things to get this right:

  • we need to make sure old LambdaCD instances are shut down properly so that we don't have dangling builds that no longer do anything
  • we need to make sure that a new runner is spawned, giving us a new pipeline

Here is my approach to make this happen:

  • defonce an atom to hold the pipelines so we can access the old pipeline and shut it down before starting a new instance
  • use a let inside the def app to call the old pipelines shutdown-hook, initialize a new pipeline and reset the atom
  • Move the ring-specific parts into a separate namespace to keep on-load side-effects out of the normal code

You'll find the full, working example here. Mostly a normal LambdaCD project, the reloading-part is in ring_reload.clj. The example only has one pipeline but it should be easy to adapt it to multiple pipelines as well (probably just calling the shutdown sequence for every pipeline).

flosell avatar Jan 22 '17 12:01 flosell

Also added a reference to the example in the wiki.

flosell avatar Jan 22 '17 13:01 flosell

I refactored your solution with https://github.com/tolitius/mount. What do you think about it?

(defn start-pipelines [pipelines]
  (doseq [pipeline (vals pipelines)]
    (runners/start-one-run-after-another pipeline)))

(defn stop-pipelines [pipelines]
  (doseq [pipeline (vals pipelines)]
    (when-let [old-ctx (:context pipeline)]
      ((get-in old-ctx [:config :shutdown-sequence]) old-ctx))))

(mount/defstate init
                :start (git/init-ssh!))

(mount/defstate pipelines
                :start {:sb (lambdacd/assemble-pipeline pipeline/sb {:home-dir "builds/sb"
                                                                     :name "Foo"
                                                                     :max-builds 3
                                                                     :ui-config {:expand-active-default true
                                                                                 :expand-failures-default true}})}
                :stop (stop-pipelines pipelines))

(mount/defstate routes
                :start (compojure/routes
                         (compojure/context "/sb" [] (ui/ui-for (:sb pipelines)))))

(mount/defstate pipelines-run
                :start (start-pipelines pipelines))

(defn start []
  (mount/start))

(defn -main [& args]
  (start)
  (httpkit/run-server routes {:port 8080}))

In that way mount care about reload things themselves. No needs to make special separate code only for tests. mount call :stop and :start on every state in namespace which was reloaded. So in that case after mount/start once it always care about reload when namespace code is load. Works with lein ring server.

project.clj

(defproject sb-pipeline "0.1.0-SNAPSHOT"
...
  :dependencies [
                 ...
                 [mount "0.1.11"]]
  :plugins [[lein-ring "0.9.7"]]
  :ring {:handler sb-pipeline.core/routes
         :init sb-pipeline.core/start})

kwladyka avatar Jan 23 '17 19:01 kwladyka

Interesting, I like it! Could you publish the full project somewhere as an example for others? Either as its own repository or as a PR in the existing example project?

I'm sure lots of people are interested in having a clean solution for this.

flosell avatar Jan 24 '17 07:01 flosell

I will be glad to do it. After put project on production (this week?) i will share what i did. But maybe i can share solution for only lein ring server sooner. Or maybe more extended full real example ready to use.

kwladyka avatar Jan 24 '17 10:01 kwladyka

The above example with mount is very very clear thanks! I was checking https://github.com/flosell/lambdacd-auto-reload-example but I am more familiar with mount myself ;)

arichiardi avatar May 29 '17 19:05 arichiardi

Thanks for the feedback! I updated the README in lambdacd-auto-reload-example to make the link to the mount example a bit more visible

flosell avatar May 31 '17 05:05 flosell

not active

kwladyka avatar Nov 01 '22 21:11 kwladyka