Allow run_e2e_workflow to select which version of ks to use (kubeflow…

…#228) * Fix for ks 0.12.0 * Fix test and lint * Fix pylint * Check apiVersion in app.yaml * Fix test * Fix test
Linchin · Oct 3, 2018 · fae93d3 · fae93d3
1 parent 5446929
commit fae93d3
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 21 deletions.
diff --git a/images/Dockerfile b/images/Dockerfile
@@ -89,14 +89,22 @@ RUN cd /tmp && \
     tar -xvf glide-v0.13.0-linux-amd64.tar.gz && \
     mv ./linux-amd64/glide /usr/local/bin/
 
-# Install ksonnet
+# Install ksonnet. We install both 0.11.0 and 0.12.0 due to compatibility issues
+# (see https://github.com/kubeflow/testing/issues/220).
 RUN cd /tmp && \
     wget -O ks.tar.gz \
-    https://github.com/ksonnet/ksonnet/releases/download/v0.12.0/ks_0.12.0_linux_amd64.tar.gz && \
+    https://github.com/ksonnet/ksonnet/releases/download/v0.11.0/ks_0.11.0_linux_amd64.tar.gz && \
     tar -xvf ks.tar.gz && \
-    mv ks_0.12.0_linux_amd64/ks /usr/local/bin && \
+    mv ks_0.11.0_linux_amd64/ks /usr/local/bin && \
     chmod a+x /usr/local/bin/ks
 
+RUN cd /tmp && \
+    wget -O ks-12.tar.gz \
+    https://github.com/ksonnet/ksonnet/releases/download/v0.12.0/ks_0.12.0_linux_amd64.tar.gz && \
+    tar -xvf ks-12.tar.gz && \
+    mv ks_0.12.0_linux_amd64/ks /usr/local/bin/ks-12 && \
+    chmod a+x /usr/local/bin/ks-12
+
 RUN cd /tmp && \
     wget https://github.com/google/jsonnet/archive/v0.11.2.tar.gz && \
     tar -xvf v0.11.2.tar.gz && \

diff --git a/py/kubeflow/testing/run_e2e_workflow.py b/py/kubeflow/testing/run_e2e_workflow.py
@@ -110,9 +110,22 @@ def generate_env_from_head(args):
       continue
     os.environ[k] = env_var.get(k)
 
+# Get the ksonnet cmd name based on apiVersion in app.yaml.
+def get_ksonnet_cmd(workflow):
+  app_yaml_file = workflow.app_dir + "/app.yaml"
+  with open(app_yaml_file) as app_yaml:
+    results = yaml.load(app_yaml)
+
+  if results["apiVersion"] == "0.1.0":
+    return "ks"
+
+  if results["apiVersion"] == "0.2.0":
+    return "ks-12"
+
+  # For compatibility reasons we'll keep the default cmd as "ks".
+  return "ks"
+
 def run(args, file_handler): # pylint: disable=too-many-statements,too-many-branches
-  # Print ksonnet version
-  util.run(["ks", "version"])
   job_type = os.getenv("JOB_TYPE")
   repo_owner = os.getenv("REPO_OWNER")
   repo_name = os.getenv("REPO_NAME")
@@ -155,6 +168,10 @@ def run(args, file_handler): # pylint: disable=too-many-statements,too-many-bran
     # Workflow name should not be more than 63 characters because its used
     # as a label on the pods.
     workflow_name = os.getenv("JOB_NAME") + "-" + w.name
+    ks_cmd = get_ksonnet_cmd(w)
+
+    # Print ksonnet version
+    util.run([ks_cmd, "version"])
 
     # Skip this workflow if it is scoped to a different job type.
     if w.job_types and not job_type in w.job_types:
@@ -202,9 +219,9 @@ def run(args, file_handler): # pylint: disable=too-many-statements,too-many-bran
     # Create a new environment for this run
     env = workflow_name
 
-    util.run(["ks", "env", "add", env], cwd=w.app_dir)
+    util.run([ks_cmd, "env", "add", env], cwd=w.app_dir)
 
-    util.run(["ks", "param", "set", "--env=" + env, w.component,
+    util.run([ks_cmd, "param", "set", "--env=" + env, w.component,
               "name", workflow_name],
              cwd=w.app_dir)
 
@@ -220,27 +237,27 @@ def run(args, file_handler): # pylint: disable=too-many-statements,too-many-bran
         continue
       prow_env.append("{0}={1}".format(v, os.getenv(v)))
 
-    util.run(["ks", "param", "set", "--env=" + env, w.component, "prow_env", ",".join(prow_env)],
-             cwd=w.app_dir)
-    util.run(["ks", "param", "set", "--env=" + env, w.component, "namespace", get_namespace(args)],
-             cwd=w.app_dir)
-    util.run(["ks", "param", "set", "--env=" + env, w.component, "bucket", args.bucket],
-             cwd=w.app_dir)
+    util.run([ks_cmd, "param", "set", "--env=" + env, w.component, "prow_env",
+             ",".join(prow_env)], cwd=w.app_dir)
+    util.run([ks_cmd, "param", "set", "--env=" + env, w.component, "namespace",
+             get_namespace(args)], cwd=w.app_dir)
+    util.run([ks_cmd, "param", "set", "--env=" + env, w.component, "bucket",
+             args.bucket], cwd=w.app_dir)
     if args.release:
-      util.run(["ks", "param", "set", "--env=" + env, w.component, "versionTag",
+      util.run([ks_cmd, "param", "set", "--env=" + env, w.component, "versionTag",
                 os.getenv("VERSION_TAG")], cwd=w.app_dir)
 
     # Set any extra params. We do this in alphabetical order to make it easier to verify in
     # the unittest.
     param_names = w.params.keys()
     param_names.sort()
     for k in param_names:
-      util.run(["ks", "param", "set", "--env=" + env, w.component, k, "{0}".format(w.params[k])],
-               cwd=w.app_dir)
+      util.run([ks_cmd, "param", "set", "--env=" + env, w.component, k,
+               "{0}".format(w.params[k])], cwd=w.app_dir)
 
     # For debugging print out the manifest
-    util.run(["ks", "show", env, "-c", w.component], cwd=w.app_dir)
-    util.run(["ks", "apply", env, "-c", w.component], cwd=w.app_dir)
+    util.run([ks_cmd, "show", env, "-c", w.component], cwd=w.app_dir)
+    util.run([ks_cmd, "apply", env, "-c", w.component], cwd=w.app_dir)
 
     ui_url = ("http://testing-argo.kubeflow.org/workflows/kubeflow-test-infra/{0}"
               "?tab=workflow".format(workflow_name))

diff --git a/py/kubeflow/tests/run_e2e_workflow_test.py b/py/kubeflow/tests/run_e2e_workflow_test.py
@@ -60,18 +60,24 @@ def testWithConfig(self, mock_run, mock_configure, *unused_mocks):  # pylint: di
     os.environ["BUILD_NUMBER"] = "1234"
     os.environ["BUILD_ID"] = "11"
 
+    cwd = os.getcwd()
+    # Current directory is in the format:
+    # "/mnt/test-data-volume/kubeflow-testing-12345/src/kubeflow/testing"
+    # We need to parse the actual repo root here.
+    repo_dir = cwd[:cwd.index("/kubeflow/testing")]
+
     args = ["--project=some-project", "--cluster=some-cluster",
             "--zone=us-east1-d", "--bucket=some-bucket",
             "--config_file=" + name,
-            "--repos_dir=/src"]
+            "--repos_dir=" + repo_dir]
     run_e2e_workflow.main(args)
 
     mock_configure.assert_called_once_with("some-project", "us-east1-d",
                                            "some-cluster",)
 
     expected_calls = [
-      ["ks", "version"],
       ["git", "diff", "--name-only", "master"],
+      ["ks", "version"],
       ["ks", "env", "add", "kubeflow-presubmit-wf-77-123abc-1234-.*"],
       ["ks", "param", "set", "--env=.*", "workflows", "name",
            "kubeflow-presubmit-wf-77-123abc-1234-[0-9a-z]{4}"],
@@ -104,7 +110,7 @@ def testWithConfig(self, mock_run, mock_configure, *unused_mocks):  # pylint: di
         mock_run.call_args_list[i][0][0])
       if i > 1:
         self.assertEqual(
-           "/src/kubeflow/testing/workflows",
+           repo_dir + "/kubeflow/testing/workflows",
            mock_run.call_args_list[i][1]["cwd"])
 
 if __name__ == "__main__":