From d38ff6c8e5a674c03a12939ab0d8e8b8bb698874 Mon Sep 17 00:00:00 2001 From: Timo Kramer Date: Sun, 2 Feb 2020 18:36:38 +0100 Subject: [PATCH] pull image function refactored --- src/bob/execution/internals.clj | 79 +++++++++++++++------------ test/bob/execution/internals_test.clj | 40 ++++++++++---- 2 files changed, 73 insertions(+), 46 deletions(-) diff --git a/src/bob/execution/internals.clj b/src/bob/execution/internals.clj index 464709c5..45ae1b4f 100644 --- a/src/bob/execution/internals.clj +++ b/src/bob/execution/internals.clj @@ -24,21 +24,18 @@ "Checks if an image is present locally. Returns the name or the error if any." [name] - (let [images (docker/client {:category :images - :conn states/docker-conn}) - result (f/try* (filter #(= (:RepoTags %) [name]) - (docker/invoke images {:op :ImageList})))] - (if (or (f/failed? result) (zero? (count result))) + (let [result (filter #(= (:RepoTags %) [name]) + (docker/invoke states/images {:op :ImageList}))] + (if (zero? (count result)) (f/fail "Failed to find %s" name) - name))) + (do (log/debugf "Found image locally: %s" name) + name)))) (defn kill-container "Kills a running container using SIGKILL. Returns the name or the error if any." [name] - (let [containers (docker/client {:category :containers - :conn states/docker-conn}) - result (docker/invoke containers {:op :ContainerKill :params {:id name}})] + (let [result (docker/invoke states/containers {:op :ContainerKill :params {:id name}})] (if (clojure.string/blank? result) name (do (log/errorf "Error killing container %s" name) @@ -48,13 +45,28 @@ "Pulls in an image if it's not present locally. Returns the name or the error if any." [name] - (if (and (f/failed? (has-image name)) - (f/failed? (f/try* (do (log/debugf "Pulling image: %s" name) - (docker/pull states/docker-conn name) - (log/debugf "Pulled image: %s" name))))) - (do (log/errorf "Could not pull image: %s" name) - (f/fail "Cannot pull %s" name)) - name)) + (let [validate-name (fn [name] + (let [split-name (str/split name #":")] + (if (= 2 (count split-name)) + split-name + (do (log/errorf "Please provide a repository and a tag as image name: %s" split-name) + (f/fail "Please provide a repository and a tag as image name: %s" split-name))))) + pull-invoke (fn [split-name] + (let [repo (first split-name) + tag (second split-name) + _ (log/debugf "Pulling image: %s" name) + result (docker/invoke states/images {:op :ImageCreate + :params {:fromImage repo + :tag tag}})] + (if (contains? result :message) + (f/fail "Could not pull image %s:%s" repo tag) + result)))] + (if (and (f/failed? (has-image name)) + (f/failed? (pull-invoke (validate-name name)))) + (do (log/errorf "Could not pull image: %s" name) + (f/fail "Could not pull image: %s" name)) + (do (log/debugf "Successfully pulled image: %s" name) + name)))) (defn build "Builds a container. @@ -104,26 +116,25 @@ status (-> (docker/inspect states/docker-conn id) :State :ExitCode)] - (if (zero? status) - (u/format-id id) - (do (log/debugf "Container %s exited with non-zero status: %s" - id - status) - (f/fail "Abnormal exit."))) - (f/when-failed [err] - (log/errorf "Error in running container %s: %s" - id - (f/message err)) - err))) + (if (zero? status) + (u/format-id id) + (do (log/debugf "Container %s exited with non-zero status: %s" + id + status) + (f/fail "Abnormal exit."))) + (f/when-failed [err] + (log/errorf "Error in running container %s: %s" + id + (f/message err)) + err))) (comment - (def images (docker/client {:category :images - :conn states/docker-conn})) - (filter #(= (:RepoTags %) ["nginx:latest"]) (docker/invoke images {:op :ImageList})) - (filter #(= (:RepoTags %) ["img"]) [{:RepoTags ["img"]}]) - - - + (docker/ops states/images) + (docker/doc states/images :ImageCreate) + (docker/invoke states/images {:op :ImageCreate :params {:fromImage "clojure" :tag "latest"}}) + (docker/invoke states/images {:op :ImageDelete :params {:name "clojure:latest"}}) + (pull "foo") + (pull "clojure:latest") (build "busybox:musl" {:needs_resource "source" :cmd "ls"} diff --git a/test/bob/execution/internals_test.clj b/test/bob/execution/internals_test.clj index 614901f6..033fed65 100644 --- a/test/bob/execution/internals_test.clj +++ b/test/bob/execution/internals_test.clj @@ -22,14 +22,21 @@ (deftest docker-image-presence (testing "image present" - (with-redefs-fn {#'docker/invoke (constantly [{:RepoTags ["img"]}])} - #(is (= "img" (has-image "img"))))) + (with-redefs-fn {#'docker/invoke (constantly [{:RepoTags ["clojure:latest"]}])} + #(is (= "clojure:latest" (has-image "clojure:latest"))))) + + (testing "image present no tag provided" + (with-redefs-fn {#'docker/invoke (constantly [{:RepoTags ["clojure:latest"]}])} + #(let [result (has-image "clojure")] + (is (and (f/failed? result) + (= "Failed to find clojure" + (f/message result))))))) (testing "image absent" - (with-redefs-fn {#'docker/invoke (constantly [{:RepoTags ["foo"]}])} - #(let [result (has-image "img")] + (with-redefs-fn {#'docker/invoke (constantly [{:RepoTags ["clojure:foo"]}])} + #(let [result (has-image "clojure:latest")] (is (and (f/failed? result) - (= "Failed to find img" + (= "Failed to find clojure:latest" (f/message result)))))))) (deftest container-kill @@ -49,18 +56,27 @@ (deftest image-pull (testing "successful pull" (with-redefs-fn {#'has-image (constantly true) - #'docker/pull (fn [_ name] - (tu/check-and-fail - #(= "img" name)))} + #'docker/invoke (fn [_ name] + (tu/check-and-fail + #(= "img" name)))} #(is (= "img" (pull "img"))))) (testing "unsuccessful pull" (with-redefs-fn {#'has-image (constantly (f/fail "Failed")) - #'docker/pull (fn [_ _] (throw (Exception. "Failed")))} - #(let [result (pull "img")] + #'docker/invoke (fn [_ _] {:message "failed"})} + #(let [result (pull "clojure:foo")] (is (and (f/failed? result) - (= "Cannot pull img" - (f/message result)))))))) + (= "Could not pull image: clojure:foo" + (f/message result))))))) + + (testing "missing tag in image name" + (with-redefs-fn {#'docker/invoke (constantly {:message "failed"})} + #(let [result (pull "clojure")] + (is (and (f/failed? result)))))) + + (testing "image already locally available" + (with-redefs-fn {#'has-image (constantly true)} + #(is (= "img" (pull "img")))))) (deftest container-status (testing "successful status fetch"