Mousius commented on code in PR #11329:
URL: https://github.com/apache/tvm/pull/11329#discussion_r876839254


##########
tests/scripts/git_utils.py:
##########
@@ -29,6 +30,18 @@ def compress_query(query: str) -> str:
     return query
 
 
+def get(url: str, headers: Optional[Dict[str, str]] = None) -> Dict[str, Any]:
+    logging.info(f"Requesting GET to {url}")
+    if headers is None:
+        headers = {}
+    req = request.Request(url, headers=headers)
+    with request.urlopen(req) as response:
+        response_headers = {k: v for k, v in response.getheaders()}
+        response = json.loads(response.read())
+
+    return response, response_headers

Review Comment:
   Can we move this to another file? Maybe `http_utils.py` or similar? The 
first usage I saw was for the call to the Docker endpoint, unrelated to git.



##########
tests/python/ci/test_ci.py:
##########
@@ -731,5 +735,81 @@ def run(type, data, check):
     )
 
 
+def test_should_rebuild_docker(tmpdir_factory):

Review Comment:
   Let's split these tests into separate test files? Can put this test in a new 
file now and split the rest in another PR.



##########
tests/scripts/cmd_utils.py:
##########
@@ -44,18 +44,23 @@ def init_log():
 
 
 class Sh:
-    def __init__(self, env=None):
+    def __init__(self, env=None, cwd=None):
         self.env = os.environ.copy()
         if env is not None:
             self.env.update(env)
+        self.cwd = cwd
 
     def run(self, cmd: str, **kwargs):
         logging.info(f"+ {cmd}")
-        if "check" not in kwargs:
-            kwargs["check"] = True
-        if "shell" not in kwargs:
-            kwargs["shell"] = True
-        if "env" not in kwargs:
-            kwargs["env"] = self.env
-
-        subprocess.run(cmd, **kwargs)
+        defaults = {
+            "check": True,
+            "shell": True,
+            "env": self.env,
+            "encoding": "utf-8",
+            "cwd": self.cwd,
+        }
+        for k, v in defaults.items():
+            if k not in kwargs:
+                kwargs[k] = v

Review Comment:
   Minor minor nit, would be nicer not to mutate `kwargs` and instead produce a 
new dictionary here?
   
   ```
   subprocess_args = {
     k: v if k not in kwargs else kwargs[k]
     for k, v in defaults.items()
   }
   ```



##########
tests/python/ci/test_ci.py:
##########
@@ -731,5 +735,81 @@ def run(type, data, check):
     )
 
 
+def test_should_rebuild_docker(tmpdir_factory):
+    tag_script = REPO_ROOT / "tests" / "scripts" / "should_rebuild_docker.py"
+
+    def run(changed_files, name, check, expected_code):
+        git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
+        git.run("init")
+        git.run("checkout", "-b", "main")
+        git.run("remote", "add", "origin", "https://github.com/apache/tvm.git";)
+
+        git_path = Path(git.cwd)
+        for i, commits in enumerate(changed_files):
+            for filename in commits:
+                path = git_path / filename
+                path.parent.mkdir(exist_ok=True, parents=True)
+                path.touch()
+                git.run("add", filename)
+
+            git.run("commit", "-m", f"message {i}")
+
+        if name is None:
+            ref = "HEAD"
+            if len(changed_files) > 1:
+                ref = f"HEAD~{len(changed_files) - 1}"
+            proc = git.run("rev-parse", ref, stdout=subprocess.PIPE)
+            last_hash = proc.stdout.strip()
+            name = f"123-123-{last_hash}"
+
+        docker_data = {
+            "repositories/tlcpack": {
+                "results": [
+                    {
+                        "name": "ci-something",
+                    }
+                ],
+            },
+            "repositories/tlcpack/ci-something/tags": {
+                "results": [{"name": name}],
+            },
+        }
+
+        proc = subprocess.run(
+            [
+                str(tag_script),
+                "--testing-docker-data",
+                json.dumps(docker_data),
+            ],
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT,
+            encoding="utf-8",
+            cwd=git.cwd,
+        )
+
+        assert_in(check, proc.stdout)
+        assert proc.returncode == expected_code
+
+    run(
+        changed_files=[],
+        name="abc",
+        check="Image abc is not using new naming scheme",
+        expected_code=1,
+    )
+    run(changed_files=[], name="123-123-abc", check="No extant hash found", 
expected_code=1)
+    run(
+        changed_files=[["test.txt"]],
+        name=None,
+        check="Did not find changes, no rebuild necessary",
+        expected_code=0,
+    )
+    run(
+        changed_files=[["test.txt"], ["docker/test.txt"]],
+        name=None,
+        check="Found docker changes",
+        expected_code=2,
+    )

Review Comment:
   `pytest.mark.parametrize` please 😸 



##########
jenkins/Jenkinsfile.j2:
##########
@@ -190,190 +190,187 @@ if 
(currentBuild.getBuildCauses().toString().contains('BranchIndexingCause')) {
 
 cancel_previous_build()
 
-def lint() {
-stage('Lint') {
-  parallel(
-    {% call m.sharded_lint_step(name='Lint', num_shards=2, node='CPU-SMALL', 
ws='tvm/lint') %}
-      {% for image in images %}
-      {{ image.name }} = params.{{ image.name }}_param ?: {{ image.name }}
-      {% endfor %}
+def run_lint() {
+  stage('Lint') {
+    parallel(
+      {% call m.sharded_lint_step(
+        name='Lint',
+        num_shards=2,
+        node='CPU-SMALL',
+        ws='tvm/lint',
+        docker_image="ci_lint")
+      %}
+        sh (
+          script: "${docker_run} ${ci_lint} ./tests/scripts/task_lint.sh",
+          label: 'Run lint',
+        )
+      {% endcall %}
+    )
+  }
+}
 
-      sh (script: """
-        echo "Docker images being used in this build:"
+def lint() {

Review Comment:
   This doesn't actually do any `lint`ing anymore does it? it just triggers the 
`run_lint`, maybe `prepare_build` or something? 



##########
tests/python/ci/test_ci.py:
##########
@@ -731,5 +735,81 @@ def run(type, data, check):
     )
 
 
+def test_should_rebuild_docker(tmpdir_factory):
+    tag_script = REPO_ROOT / "tests" / "scripts" / "should_rebuild_docker.py"
+
+    def run(changed_files, name, check, expected_code):
+        git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
+        git.run("init")
+        git.run("checkout", "-b", "main")
+        git.run("remote", "add", "origin", "https://github.com/apache/tvm.git";)
+
+        git_path = Path(git.cwd)
+        for i, commits in enumerate(changed_files):
+            for filename in commits:
+                path = git_path / filename
+                path.parent.mkdir(exist_ok=True, parents=True)
+                path.touch()
+                git.run("add", filename)
+
+            git.run("commit", "-m", f"message {i}")
+
+        if name is None:
+            ref = "HEAD"
+            if len(changed_files) > 1:
+                ref = f"HEAD~{len(changed_files) - 1}"
+            proc = git.run("rev-parse", ref, stdout=subprocess.PIPE)
+            last_hash = proc.stdout.strip()
+            name = f"123-123-{last_hash}"
+
+        docker_data = {

Review Comment:
   Can we get a test case with more than one result? To test the filtering 
logic.



##########
jenkins/Jenkinsfile.j2:
##########
@@ -190,190 +190,187 @@ if 
(currentBuild.getBuildCauses().toString().contains('BranchIndexingCause')) {
 
 cancel_previous_build()
 
-def lint() {
-stage('Lint') {
-  parallel(
-    {% call m.sharded_lint_step(name='Lint', num_shards=2, node='CPU-SMALL', 
ws='tvm/lint') %}
-      {% for image in images %}
-      {{ image.name }} = params.{{ image.name }}_param ?: {{ image.name }}
-      {% endfor %}
+def run_lint() {
+  stage('Lint') {
+    parallel(
+      {% call m.sharded_lint_step(
+        name='Lint',
+        num_shards=2,
+        node='CPU-SMALL',
+        ws='tvm/lint',
+        docker_image="ci_lint")
+      %}
+        sh (
+          script: "${docker_run} ${ci_lint} ./tests/scripts/task_lint.sh",
+          label: 'Run lint',
+        )
+      {% endcall %}
+    )
+  }
+}
 
-      sh (script: """
-        echo "Docker images being used in this build:"
+def lint() {
+  stage('Prepare') {
+    node('CPU-SMALL') {
+      ws("workspace/exec_${env.EXECUTOR_NUMBER}/tvm/prepare") {
+        init_git()
         {% for image in images %}
-        echo " {{ image.name }} = ${ {{- image.name -}} }"
+        {{ image.name }} = params.{{ image.name }}_param ?: {{ image.name }}
         {% endfor %}
-      """, label: 'Docker image names')
 
-      is_docs_only_build = sh (
-        returnStatus: true,
-        script: './tests/scripts/git_change_docs.sh',
-        label: 'Check for docs only changes',
-      )
-      skip_ci = should_skip_ci(env.CHANGE_ID)
-      skip_slow_tests = should_skip_slow_tests(env.CHANGE_ID)
-      rebuild_docker_images = sh (
-        returnStatus: true,
-        script: './tests/scripts/git_change_docker.sh',
-        label: 'Check for any docker changes',
-      )
-      if (skip_ci) {
-        // Don't rebuild when skipping CI
-        rebuild_docker_images = false
-      }
-      if (rebuild_docker_images) {
-        // Exit before linting so we can use the newly created Docker images
-        // to run the lint
-        return
+        sh (script: """
+          echo "Docker images being used in this build:"
+          {% for image in images %}
+          echo " {{ image.name }} = ${ {{- image.name -}} }"
+          {% endfor %}
+        """, label: 'Docker image names')
+
+        is_docs_only_build = sh (
+          returnStatus: true,
+          script: './tests/scripts/git_change_docs.sh',
+          label: 'Check for docs only changes',
+        )
+        skip_ci = should_skip_ci(env.CHANGE_ID)
+        skip_slow_tests = should_skip_slow_tests(env.CHANGE_ID)
+        rebuild_docker_images = sh (
+          returnStatus: true,
+          script: './tests/scripts/should_rebuild_docker.py',
+          label: 'Check for any docker changes',
+        )
+        if (skip_ci) {
+          // Don't rebuild when skipping CI
+          rebuild_docker_images = false
+        }
       }
-      sh (
-        script: "${docker_run} ${ci_lint} ./tests/scripts/task_lint.sh",
-        label: 'Run lint',
-      )
-    {% endcall %}
-  )
-}
+    }
+  }
+
+  if (!rebuild_docker_images) {
+    run_lint()
+  }

Review Comment:
   I think we can remove this from here in favour of placing it just after the 
conditional image rebuild?



##########
jenkins/Jenkinsfile.j2:
##########
@@ -190,190 +190,187 @@ if 
(currentBuild.getBuildCauses().toString().contains('BranchIndexingCause')) {
 
 cancel_previous_build()
 
-def lint() {
-stage('Lint') {
-  parallel(
-    {% call m.sharded_lint_step(name='Lint', num_shards=2, node='CPU-SMALL', 
ws='tvm/lint') %}
-      {% for image in images %}
-      {{ image.name }} = params.{{ image.name }}_param ?: {{ image.name }}
-      {% endfor %}
+def run_lint() {
+  stage('Lint') {
+    parallel(
+      {% call m.sharded_lint_step(
+        name='Lint',
+        num_shards=2,
+        node='CPU-SMALL',
+        ws='tvm/lint',
+        docker_image="ci_lint")
+      %}
+        sh (
+          script: "${docker_run} ${ci_lint} ./tests/scripts/task_lint.sh",
+          label: 'Run lint',
+        )
+      {% endcall %}
+    )
+  }
+}
 
-      sh (script: """
-        echo "Docker images being used in this build:"
+def lint() {
+  stage('Prepare') {
+    node('CPU-SMALL') {
+      ws("workspace/exec_${env.EXECUTOR_NUMBER}/tvm/prepare") {
+        init_git()
         {% for image in images %}
-        echo " {{ image.name }} = ${ {{- image.name -}} }"
+        {{ image.name }} = params.{{ image.name }}_param ?: {{ image.name }}
         {% endfor %}
-      """, label: 'Docker image names')
 
-      is_docs_only_build = sh (
-        returnStatus: true,
-        script: './tests/scripts/git_change_docs.sh',
-        label: 'Check for docs only changes',
-      )
-      skip_ci = should_skip_ci(env.CHANGE_ID)
-      skip_slow_tests = should_skip_slow_tests(env.CHANGE_ID)
-      rebuild_docker_images = sh (
-        returnStatus: true,
-        script: './tests/scripts/git_change_docker.sh',
-        label: 'Check for any docker changes',
-      )
-      if (skip_ci) {
-        // Don't rebuild when skipping CI
-        rebuild_docker_images = false
-      }
-      if (rebuild_docker_images) {
-        // Exit before linting so we can use the newly created Docker images
-        // to run the lint
-        return
+        sh (script: """
+          echo "Docker images being used in this build:"
+          {% for image in images %}
+          echo " {{ image.name }} = ${ {{- image.name -}} }"
+          {% endfor %}
+        """, label: 'Docker image names')
+
+        is_docs_only_build = sh (
+          returnStatus: true,
+          script: './tests/scripts/git_change_docs.sh',
+          label: 'Check for docs only changes',
+        )
+        skip_ci = should_skip_ci(env.CHANGE_ID)
+        skip_slow_tests = should_skip_slow_tests(env.CHANGE_ID)
+        rebuild_docker_images = sh (
+          returnStatus: true,
+          script: './tests/scripts/should_rebuild_docker.py',
+          label: 'Check for any docker changes',
+        )
+        if (skip_ci) {
+          // Don't rebuild when skipping CI
+          rebuild_docker_images = false
+        }
       }
-      sh (
-        script: "${docker_run} ${ci_lint} ./tests/scripts/task_lint.sh",
-        label: 'Run lint',
-      )
-    {% endcall %}
-  )
-}
+    }
+  }
+
+  if (!rebuild_docker_images) {
+    run_lint()
+  }
 }
 
 // [note: method size]
 // This has to be extracted into a method due to JVM limitations on the size of
 // a method (so the code can't all be inlined)
 lint()
 
-def build_image(image_name) {
-  hash = sh(
+def ecr_push(full_name) {
+  aws_account_id = sh(
     returnStdout: true,
-    script: 'git log -1 --format=\'%h\''
+    script: 'aws sts get-caller-identity | grep Account | cut -f4 -d\\"',
+    label: 'Get AWS ID'
   ).trim()
-  def full_name = 
"${image_name}:${env.BRANCH_NAME}-${hash}-${env.BUILD_NUMBER}"
-  sh(
-    script: "${docker_build} ${image_name} --spec ${full_name}",
-    label: 'Build docker image'
-  )
+
+  def ecr_name = 
"${aws_account_id}.dkr.ecr.us-west-2.amazonaws.com/${full_name}"
+  try {
+    withEnv([
+      "AWS_ACCOUNT_ID=${aws_account_id}",
+      'AWS_DEFAULT_REGION=us-west-2',
+      "AWS_ECR_REPO=${aws_account_id}.dkr.ecr.us-west-2.amazonaws.com"]) {
+      sh(
+        script: '''
+          set -eux
+          aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker 
login --username AWS --password-stdin $AWS_ECR_REPO
+        ''',
+        label: 'Log in to ECR'
+      )
+      sh(
+        script: """
+          set -x
+          docker tag ${full_name} \$AWS_ECR_REPO/${full_name}
+          docker push \$AWS_ECR_REPO/${full_name}
+        """,
+        label: 'Upload image to ECR'
+      )
+    }
+  } finally {
+    withEnv([
+      "AWS_ACCOUNT_ID=${aws_account_id}",
+      'AWS_DEFAULT_REGION=us-west-2',
+      "AWS_ECR_REPO=${aws_account_id}.dkr.ecr.us-west-2.amazonaws.com"]) {
+      sh(
+        script: 'docker logout $AWS_ECR_REPO',
+        label: 'Clean up login credentials'
+      )
+    }
+  }
+  return ecr_name
+}
+
+def ecr_pull(full_name) {
   aws_account_id = sh(
     returnStdout: true,
     script: 'aws sts get-caller-identity | grep Account | cut -f4 -d\\"',
     label: 'Get AWS ID'
   ).trim()
 
   try {
-    // Use a credential so Jenkins knows to scrub the AWS account ID which is 
nice
-    // (but so we don't have to rely it being hardcoded in Jenkins)
-    withCredentials([string(
-      credentialsId: 'aws-account-id',
-      variable: '_ACCOUNT_ID_DO_NOT_USE',
-      )]) {
-      withEnv([
-        "AWS_ACCOUNT_ID=${aws_account_id}",
-        'AWS_DEFAULT_REGION=us-west-2']) {
-        sh(
-          script: '''
-            set -x
-            aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker 
login --username AWS --password-stdin 
$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com
-          ''',
-          label: 'Log in to ECR'
-        )
-        sh(
-          script: """
-            set -x
-            docker tag ${full_name} 
\$AWS_ACCOUNT_ID.dkr.ecr.\$AWS_DEFAULT_REGION.amazonaws.com/${full_name}
-            docker push 
\$AWS_ACCOUNT_ID.dkr.ecr.\$AWS_DEFAULT_REGION.amazonaws.com/${full_name}
-          """,
-          label: 'Upload image to ECR'
-        )
-      }
+    withEnv([
+      "AWS_ACCOUNT_ID=${aws_account_id}",
+      'AWS_DEFAULT_REGION=us-west-2',
+      "AWS_ECR_REPO=${aws_account_id}.dkr.ecr.us-west-2.amazonaws.com"]) {
+      sh(
+        script: '''
+          set -eux
+          aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker 
login --username AWS --password-stdin $AWS_ECR_REPO
+        ''',
+        label: 'Log in to ECR'
+      )
+      sh(
+        script: """
+          set -eux
+          docker pull ${full_name}
+        """,
+        label: 'Pull image from ECR'
+      )
     }
   } finally {
-    sh(
-      script: 'rm -f ~/.docker/config.json',
-      label: 'Clean up login credentials'
-    )
+    withEnv([
+      "AWS_ACCOUNT_ID=${aws_account_id}",
+      'AWS_DEFAULT_REGION=us-west-2',
+      "AWS_ECR_REPO=${aws_account_id}.dkr.ecr.us-west-2.amazonaws.com"]) {
+      sh(
+        script: 'docker logout $AWS_ECR_REPO',
+        label: 'Clean up login credentials'
+      )
+    }
   }
+}
+
+
+def build_image(image_name) {
+  hash = sh(
+    returnStdout: true,
+    script: 'git log -1 --format=\'%h\''
+  ).trim()
+  def full_name = 
"${image_name}:${env.BRANCH_NAME}-${hash}-${env.BUILD_NUMBER}"
   sh(
-    script: "docker rmi ${full_name}",
-    label: 'Remove docker image'
+    script: "${docker_build} ${image_name} --spec ${full_name}",
+    label: 'Build docker image'
   )
+  return ecr_push(full_name)
 }
 
 if (rebuild_docker_images) {
   stage('Docker Image Build') {
     // TODO in a follow up PR: Find ecr tag and use in subsequent builds
-    parallel 'ci-lint': {
-      node('CPU') {
-        timeout(time: max_time, unit: 'MINUTES') {
-          init_git()
-          build_image('ci_lint')
-        }
-      }
-    }, 'ci-cpu': {
-      node('CPU') {
-        timeout(time: max_time, unit: 'MINUTES') {
-          init_git()
-          build_image('ci_cpu')
-        }
-      }
-    }, 'ci-gpu': {
-      node('GPU') {
-        timeout(time: max_time, unit: 'MINUTES') {
-          init_git()
-          build_image('ci_gpu')
-        }
-      }
-    }, 'ci-qemu': {
-      node('CPU') {
-        timeout(time: max_time, unit: 'MINUTES') {
-          init_git()
-          build_image('ci_qemu')
-        }
-      }
-    }, 'ci-i386': {
-      node('CPU') {
-        timeout(time: max_time, unit: 'MINUTES') {
-          init_git()
-          build_image('ci_i386')
-        }
-      }
-    }, 'ci-arm': {
-      node('ARM') {
-        timeout(time: max_time, unit: 'MINUTES') {
-          init_git()
-          build_image('ci_arm')
-        }
-      }
-    }, 'ci-wasm': {
-      node('CPU') {
-        timeout(time: max_time, unit: 'MINUTES') {
-          init_git()
-          build_image('ci_wasm')
-        }
-      }
-    }, 'ci-hexagon': {
-      node('CPU') {
-        timeout(time: max_time, unit: 'MINUTES') {
-          init_git()
-          build_image('ci_hexagon')
+    parallel(
+    {% for image in images %}
+      '{{ image.name }}': {
+        node('{{ image.platform }}') {
+          timeout(time: max_time, unit: 'MINUTES') {
+            init_git()
+            {{ image.name }} = build_image('{{ image.name }}')
+          }
         }
-      }
-    }
+      },
+    {% endfor %}
+    )
   }
-  // // TODO: Once we are able to use the built images, enable this step
-  // // If the docker images changed, we need to run the image build before 
the lint
-  // // can run since it requires a base docker image. Most of the time the 
images
-  // // aren't build though so it's faster to use the same node that checks for
-  // // docker changes to run the lint in the usual case.
-  // stage('Sanity Check (re-run)') {
-  //   timeout(time: max_time, unit: 'MINUTES') {
-  //     node('CPU') {
-  //       ws({{ m.per_exec_ws('tvm/sanity') }}) {
-  //         init_git()
-  //         sh (
-  //           script: "${docker_run} ${ci_lint}  
./tests/scripts/task_lint.sh",
-  //           label: 'Run lint',
-  //         )
-  //       }
-  //     }
-  //   }
-  // }
+
+  // Re-run the lint with the new Docker image before continuing to builds
+  run_lint()

Review Comment:
   As above, I think if we move this outside this block and remove the lines 
above we get the same effect whilst keeping the preparation function from 
triggering linting itself?



##########
jenkins/Jenkinsfile.j2:
##########
@@ -933,4 +984,36 @@ stage('Deploy') {
       }
     }
   }
+  // if (env.BRANCH_NAME == 'main' && rebuild_docker_images) {
+  if (env.BRANCH_NAME == 'PR-11329' && rebuild_docker_images && 
upstream_revision != null) {
+    node('CPU') {
+      ws({{ m.per_exec_ws('tvm/deploy-docker') }}) {
+        try {
+          withCredentials([string(
+            credentialsId: 'dockerhub-tlcpackstaging-key',
+            variable: 'DOCKERHUB_KEY',
+          )]) {
+            sh(
+              script: 'docker login -u tlcpackstaging -p ${DOCKERHUB_KEY}',
+              label: 'Log in to Docker Hub',
+            )
+          }
+          def date_Ymd_HMS = sh(
+            script: 'python -c \'import datetime; 
print(datetime.datetime.now().strftime("%Y%m%d-%H%M%S"))\'',
+            label: 'Determine date',
+            returnStdout: true,
+          ).trim()
+          def tag = "${date_Ymd_HMS}-${upstream_revision.substring(0, 8)}"
+          {% for image in images %}
+          update_docker({{ image.name }}, "tlcpackstaging/test_{{ image.name 
}}:${tag}")

Review Comment:
   I think this is literally for testing, but I'll leave a comment just in case 
😸 
   
   ```suggestion
             update_docker({{ image.name }}, "tlcpackstaging/{{ image.name 
}}:${tag}")
   ```



##########
jenkins/Jenkinsfile.j2:
##########
@@ -190,190 +190,187 @@ if 
(currentBuild.getBuildCauses().toString().contains('BranchIndexingCause')) {
 
 cancel_previous_build()
 
-def lint() {
-stage('Lint') {
-  parallel(
-    {% call m.sharded_lint_step(name='Lint', num_shards=2, node='CPU-SMALL', 
ws='tvm/lint') %}
-      {% for image in images %}
-      {{ image.name }} = params.{{ image.name }}_param ?: {{ image.name }}
-      {% endfor %}
+def run_lint() {
+  stage('Lint') {
+    parallel(
+      {% call m.sharded_lint_step(
+        name='Lint',
+        num_shards=2,
+        node='CPU-SMALL',
+        ws='tvm/lint',
+        docker_image="ci_lint")
+      %}
+        sh (
+          script: "${docker_run} ${ci_lint} ./tests/scripts/task_lint.sh",
+          label: 'Run lint',
+        )
+      {% endcall %}
+    )
+  }
+}
 
-      sh (script: """
-        echo "Docker images being used in this build:"
+def lint() {
+  stage('Prepare') {
+    node('CPU-SMALL') {
+      ws("workspace/exec_${env.EXECUTOR_NUMBER}/tvm/prepare") {
+        init_git()
         {% for image in images %}
-        echo " {{ image.name }} = ${ {{- image.name -}} }"
+        {{ image.name }} = params.{{ image.name }}_param ?: {{ image.name }}
         {% endfor %}
-      """, label: 'Docker image names')
 
-      is_docs_only_build = sh (
-        returnStatus: true,
-        script: './tests/scripts/git_change_docs.sh',
-        label: 'Check for docs only changes',
-      )
-      skip_ci = should_skip_ci(env.CHANGE_ID)
-      skip_slow_tests = should_skip_slow_tests(env.CHANGE_ID)
-      rebuild_docker_images = sh (
-        returnStatus: true,
-        script: './tests/scripts/git_change_docker.sh',
-        label: 'Check for any docker changes',
-      )
-      if (skip_ci) {
-        // Don't rebuild when skipping CI
-        rebuild_docker_images = false
-      }
-      if (rebuild_docker_images) {
-        // Exit before linting so we can use the newly created Docker images
-        // to run the lint
-        return
+        sh (script: """
+          echo "Docker images being used in this build:"
+          {% for image in images %}
+          echo " {{ image.name }} = ${ {{- image.name -}} }"
+          {% endfor %}
+        """, label: 'Docker image names')
+
+        is_docs_only_build = sh (
+          returnStatus: true,
+          script: './tests/scripts/git_change_docs.sh',
+          label: 'Check for docs only changes',
+        )
+        skip_ci = should_skip_ci(env.CHANGE_ID)
+        skip_slow_tests = should_skip_slow_tests(env.CHANGE_ID)
+        rebuild_docker_images = sh (
+          returnStatus: true,
+          script: './tests/scripts/should_rebuild_docker.py',
+          label: 'Check for any docker changes',
+        )
+        if (skip_ci) {
+          // Don't rebuild when skipping CI
+          rebuild_docker_images = false
+        }
       }
-      sh (
-        script: "${docker_run} ${ci_lint} ./tests/scripts/task_lint.sh",
-        label: 'Run lint',
-      )
-    {% endcall %}
-  )
-}
+    }
+  }
+
+  if (!rebuild_docker_images) {
+    run_lint()
+  }
 }
 
 // [note: method size]
 // This has to be extracted into a method due to JVM limitations on the size of
 // a method (so the code can't all be inlined)
 lint()
 
-def build_image(image_name) {
-  hash = sh(
+def ecr_push(full_name) {
+  aws_account_id = sh(
     returnStdout: true,
-    script: 'git log -1 --format=\'%h\''
+    script: 'aws sts get-caller-identity | grep Account | cut -f4 -d\\"',
+    label: 'Get AWS ID'
   ).trim()
-  def full_name = 
"${image_name}:${env.BRANCH_NAME}-${hash}-${env.BUILD_NUMBER}"
-  sh(
-    script: "${docker_build} ${image_name} --spec ${full_name}",
-    label: 'Build docker image'
-  )
+
+  def ecr_name = 
"${aws_account_id}.dkr.ecr.us-west-2.amazonaws.com/${full_name}"
+  try {
+    withEnv([
+      "AWS_ACCOUNT_ID=${aws_account_id}",
+      'AWS_DEFAULT_REGION=us-west-2',
+      "AWS_ECR_REPO=${aws_account_id}.dkr.ecr.us-west-2.amazonaws.com"]) {
+      sh(
+        script: '''
+          set -eux
+          aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker 
login --username AWS --password-stdin $AWS_ECR_REPO
+        ''',
+        label: 'Log in to ECR'
+      )
+      sh(
+        script: """
+          set -x
+          docker tag ${full_name} \$AWS_ECR_REPO/${full_name}
+          docker push \$AWS_ECR_REPO/${full_name}
+        """,
+        label: 'Upload image to ECR'
+      )
+    }
+  } finally {
+    withEnv([
+      "AWS_ACCOUNT_ID=${aws_account_id}",
+      'AWS_DEFAULT_REGION=us-west-2',
+      "AWS_ECR_REPO=${aws_account_id}.dkr.ecr.us-west-2.amazonaws.com"]) {

Review Comment:
   Can we de-dupe these by placing this into a variable or using a common 
method?



##########
jenkins/Jenkinsfile.j2:
##########
@@ -933,4 +984,36 @@ stage('Deploy') {
       }
     }
   }
+  // if (env.BRANCH_NAME == 'main' && rebuild_docker_images) {
+  if (env.BRANCH_NAME == 'PR-11329' && rebuild_docker_images && 
upstream_revision != null) {

Review Comment:
   😉 
   
   ```suggestion
     if (env.BRANCH_NAME == 'main' && rebuild_docker_images && 
upstream_revision != null) {
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to