Skip to content

Commit

Permalink
Update to script-exec 0.4.2
Browse files Browse the repository at this point in the history 
Solves issues with caching of SSH connections.

Fixes #292
  • Loading branch information
@hugoduncan
hugoduncan committed Apr 27, 2016
1 parent e8f7d7d commit 42c1d6a
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 48 deletions.
2 changes: 1 addition & 1 deletion project.clj
View file
Expand Up @@ -22,7 +22,7 @@ unprecedented level of customization."
[com.palletops/pallet-common "0.4.0"]
[com.palletops/pallet-repl "0.8.0-beta.2"
:exclusions [com.palletops/pallet]]
[com.palletops/script-exec "0.4.1"]
[com.palletops/script-exec "0.4.2"]
[com.palletops/stevedore "0.8.0-beta.7"]
[clj-ssh "0.5.7"]
[enlive "1.0.1"
Expand Down
61 changes: 33 additions & 28 deletions src/pallet/ssh/execute.clj
View file
Expand Up @@ -69,15 +69,23 @@
:err (:err result)
:out (:out result)})))))

(def ssh-connection-options {:max-tries 3})

(defn get-connection [session]
(transport/open
ssh-connection (endpoint session) (authentication session)
{:max-tries 3}))
ssh-connection-options))

(defn release-connection [session]
(defn release-connection [session connection]
(assert connection)
(transport/release
ssh-connection connection (endpoint session) (authentication session)
ssh-connection-options))

(defn expire-connection [session]
(transport/expire
ssh-connection (endpoint session) (authentication session)
{:max-tries 3}))
ssh-connection-options))

(defn ^{:internal true} with-connection-exception-handler
[e]
Expand Down Expand Up @@ -108,53 +116,52 @@
(defn ^{:internal true} with-connection*
"Execute a function with a connection to the current target node,"
[session f]
(loop [retries 1]
(let [connection (get-connection session)
r (f connection)]
(loop [retries 5]
(let [r (f)]
(if (map? r)
(cond
(and (::retriable r) (pos? retries))
(do
(release-connection session)
(logging/warnf "Retrying SSH commands due to: %s" (::exception r))
(on-retry)
(recur (dec retries)))

(::retriable r) (throw (::exception r))
(::retriable r)
(throw (::exception r))

:else (do
(try
(release-connection session)
(catch Exception _))
r))
(do
(try
(release-connection session)
(catch Exception _))
r)))))
:else r)
r))))

(defmacro ^{:indent 2} with-connection
"Execute the body with a connection to the current target node,"
[session [connection] & body]
`(with-connection* ~session (fn [~connection]
(try
~@body
(catch Exception e#
(with-connection-exception-handler e#))))))
[session [connection & [no-cache]] & body]
`(with-connection* ~session
(fn []
(try
(let [~connection (get-connection ~session)
res# (do ~@body)]
(assert ~connection)
(when-not ~no-cache
(release-connection ~session ~connection))
res#)
(catch Exception e#
(expire-connection ~session)
(with-connection-exception-handler e#))))))

(defn ssh-script-on-target
"Execute a bash action on the target via ssh."
[session {:keys [context node-value-path] :as action} action-type
[options script]]
(logging/trace "ssh-script-on-target")
(logging/trace "action %s options %s" action options)
(logging/tracef "action %s options %s" action options)
(let [endpoint (endpoint session)]
(logutils/with-context [:target (:server endpoint)]
(logging/infof
"%s %s %s %s"
(:server endpoint) (:port endpoint)
(or (context-label action) "")
(or (:summary options) ""))
(with-connection session [connection]
(with-connection session [connection (:new-login-after-action action)]
(let [authentication (transport/authentication connection)
script (script-builder/build-script options script action)
sudo-user (or (:sudo-user action)
Expand Down Expand Up @@ -208,8 +215,6 @@
(logging/trace "ssh-script-on-target done")
(logging/debugf "%s <== ----------------------------------------"
(:server endpoint))
(when (:new-login-after-action action)
(transport/close connection))
[result session]))))))

(defn- ssh-upload
Expand Down
46 changes: 29 additions & 17 deletions test/pallet/ssh/execute_test.clj
View file
Expand Up @@ -12,10 +12,17 @@
:refer [get-connection ssh-script-on-target with-connection]]
[pallet.transport :as transport]))

(def release-connection #'pallet.ssh.execute/release-connection)

(use-fixtures :once (logging-threshold-fixture))

(def open-channel clj-ssh.ssh/open-channel)

(defn- get-cached-connection [session]
(let [connection (get-connection session)]
(release-connection session connection)
connection))

(deftest with-connection-test
(let [session {:server {:node (make-localhost-node)
:image {:os-family :ubuntu}}
Expand All @@ -25,12 +32,13 @@
[connection]
(is connection)))
(testing "caching"
(let [original-connection (get-connection session)]
(let [original-connection (get-cached-connection session)]
(with-connection session
[connection]
(is connection))
(is connection)
(is (= original-connection connection) "connection cached"))
(is (= original-connection (get-connection session))
"connection cached")
"connection still cached")
(is (transport/open? (get-connection session)))))
(testing "fail on general open-channel exception"
(let [log-out
Expand Down Expand Up @@ -58,20 +66,20 @@
(:type (ex-data e))))))))]
(is log-out)))
(testing "retry on session is not opened open-channel exception"
(let [a (atom nil)
(let [no-throw-after-first-call (atom nil)
seen (atom nil)
c (atom 0)
original-connection (get-connection session)
open-channel-count (atom 0)
original-connection (get-cached-connection session)
log-out
(with-log-to-string []
(with-redefs [clj-ssh.ssh/open-channel
(fn [session session-type]
(swap! c inc)
(swap! open-channel-count inc)
(debugf (Exception. "here") "Inc")
(if @a
(if @no-throw-after-first-call
(open-channel session session-type)
(do
(reset! a true)
(reset! no-throw-after-first-call true)
(throw
(ex-info
(format "clj-ssh open-channel failure")
Expand All @@ -83,29 +91,33 @@
[connection]
(transport/exec connection {:execv ["echo" "1"]} {})
(reset! seen true))
(is @seen)
(is (= 3 @c)) ; 1 failed + sftp +exec
(is @no-throw-after-first-call "called at least once")
(is @seen "called at least twice")
(is (= 3 @open-channel-count) "1 failed + sftp + exec")
(is (not= original-connection (get-connection session))
"new cached connection")))]
(is log-out "exception is logged")))
(testing "new session after :new-login-after-action"
(with-script-for-node (:server session) nil
(let [original-connection (get-connection session)]
(let [original-connection (get-cached-connection session)]
(ssh-script-on-target
session {:node-value-path (keyword (name (gensym "nv")))}
nil [{} "echo 1"])
(is (= original-connection (get-connection session)))
(is (= original-connection (get-cached-connection session)))
(ssh-script-on-target
session {:node-value-path (keyword (name (gensym "nv")))
:new-login-after-action true}
nil [{} "echo 1"])
(is (not= original-connection (get-connection session)))
(let [second-connection (get-connection session)]
(is (not= original-connection (get-cached-connection session))
"a new connection following :new-login-after-action")
(let [second-connection (get-cached-connection session)]
(ssh-script-on-target
session {:node-value-path (keyword (name (gensym "nv")))}
nil [{} "echo 1"])
(is (not= original-connection (get-connection session)))
(is (= second-connection (get-connection session)))))))
(is (not= original-connection (get-cached-connection session))
"a new connection following :new-login-after-action")
(is (= second-connection (get-cached-connection session))
"a new connection is stable following :new-login-after-action")))))
(testing "agent-forward"
(with-script-for-node (:server session) nil
(let [[r s] (ssh-script-on-target
Expand Down
6 changes: 4 additions & 2 deletions test/pallet/ssh/file_upload/sftp_upload_test.clj
View file
Expand Up @@ -47,7 +47,8 @@
(is (= content (slurp target-f)))
(.delete target-f)))
(finally
(transport/release ssh-connection endpoint auth {:max-tries 3})))))
(transport/release
ssh-connection connection endpoint auth {:max-tries 3})))))

(deftest sftp-ensure-dir-test
(let [endpoint {:server "127.0.0.1"}
Expand All @@ -61,4 +62,5 @@
(.isDirectory (.getParentFile f))
(.delete (.getParentFile f)))
(finally
(transport/release ssh-connection endpoint auth {:max-tries 3})))))
(transport/release
ssh-connection connection endpoint auth {:max-tries 3})))))

0 comments on commit 42c1d6a

Please sign in to comment.