Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

File the pull request with information about the latest built and pushed image for the model #137

Merged
merged 4 commits into from
Oct 13, 2023
Merged

File the pull request with information about the latest built and pushed image for the model #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a3c5908
The tekton.dev/pipeline label gets automatically added by Tekton,
adelton Oct 10, 2023
a4dff89
Record and retrieve image SHA from the latest test-mlflow-image Pipel…
adelton Oct 10, 2023
805d169
Use the retrieved image SHA (to minimize race conditions).
adelton Oct 12, 2023
de4ed5b
Record and retrieve information about registry to which the image got…
adelton Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pipelines/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ in your Git server.

### Deploy and run the GitOps pipeline

The `gitops-update-pipeline` will fetch information about the last successfuly built and tested container image for the given model
from the PipelineRun of the above Pipelines, and record information about that image in your git repo.

```bash
oc apply -k tekton/gitops-update-pipeline/
oc apply -f tekton/gitops-update-pipeline/example-pipelineruns/example-git-credentials-secret.yaml
Expand Down
4 changes: 2 additions & 2 deletions ...n/gitops-update-pipeline/example-pipelineruns/gitops-update-pipelinerun-bike-rentals.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ metadata:
tekton.dev/pipeline: gitops-update-pipeline
spec:
params:
- name: model-name
value: bike-rentals-auto-ml
- name: gitServer
value: https://github.com
- name: gitApiServer
Expand All @@ -16,8 +18,6 @@ spec:
value: ai-edge-gitops
- name: gitRepoBranchBase
value: main
- name: imageDigest
value: sha256:22590bbf246cd231abebc13d7b20761b7f70e614dca9e430a1069510bdba2820
- name: imageReferenceFilePath
value: acm/odh-edge/apps/bike-rental-app/kustomization.yaml
- name: gitTokenSecretName
Expand Down
4 changes: 2 additions & 2 deletions ...ps-update-pipeline/example-pipelineruns/gitops-update-pipelinerun-tensorflow-housing.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ metadata:
tekton.dev/pipeline: gitops-update-pipeline
spec:
params:
- name: model-name
value: tensorflow-housing
- name: gitServer
value: https://github.com
- name: gitApiServer
Expand All @@ -16,8 +18,6 @@ spec:
value: ai-edge-gitops
- name: gitRepoBranchBase
value: main
- name: imageDigest
value: sha256:19e7312d8cbfcbf21dcab083f9ef99af2f807f156b1216a423267010b7fdd7c0
- name: imageReferenceFilePath
value: acm/odh-edge/apps/tensorflow-housing-app/kustomization.yaml
- name: gitTokenSecretName
Expand Down
20 changes: 16 additions & 4 deletions pipelines/tekton/gitops-update-pipeline/gitops-update-pipeline.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ metadata:
name: gitops-update-pipeline
spec:
params:
- name: model-name
type: string
- name: gitServer
type: string
- name: gitApiServer
Expand All @@ -15,8 +17,6 @@ spec:
- name: gitRepoBranchBase
default: main
type: string
- name: imageDigest
type: string
- name: imageReferenceFilePath
type: string
- name: gitApiPrefix
Expand All @@ -28,6 +28,15 @@ spec:
default: token
type: string
tasks:
- name: retrieve-image-info
taskRef:
kind: Task
name: retrieve-image-info
params:
- name: namespace
value: $(context.pipelineRun.namespace)
- name: model-name
value: $(params.model-name)
- name: git-clone
params:
- name: url
Expand Down Expand Up @@ -63,7 +72,8 @@ spec:
PR_BRANCH=pipeline_$(context.pipelineRun.uid)
git checkout -b $PR_BRANCH

sed -i -e "s/digest: .*/digest: $(params.imageDigest)/" $(params.imageReferenceFilePath)
sed -i -e "s%newName: .*%newName: $(tasks.retrieve-image-info.results.target-registry-url)%" $(params.imageReferenceFilePath)
sed -i -e "s/digest: .*/digest: $(tasks.retrieve-image-info.results.image-sha)/" $(params.imageReferenceFilePath)

if [ -z "$(git status --porcelain)" ]; then
echo "Update did not cause a modification"
Expand All @@ -77,6 +87,7 @@ spec:
- name: VERBOSE
value: "true"
runAfter:
- retrieve-image-info
- git-clone
taskRef:
kind: ClusterTask
Expand Down Expand Up @@ -114,7 +125,8 @@ spec:
| PipelineRun Name | $(context.pipelineRun.name) |
| PipelinRun UID | `$(context.pipelineRun.uid)` |
| File Path | `$(params.imageReferenceFilePath)` |
| New Digest | `$(params.imageDigest)` |
| Image registry | `$(tasks.retrieve-image-info.results.target-registry-url)` |
| New Digest | `$(tasks.retrieve-image-info.results.image-sha)` |
- name: TITLE
value: "Auto: update image ref"
runAfter:
Expand Down
1 change: 1 addition & 0 deletions pipelines/tekton/gitops-update-pipeline/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1

resources:
- https://raw.githubusercontent.com/tektoncd/catalog/main/task/github-open-pr/0.2/github-open-pr.yaml
- test-mlflow-retrieve-image-info-task.yaml
- gitops-update-pipeline.yaml
22 changes: 22 additions & 0 deletions pipelines/tekton/gitops-update-pipeline/test-mlflow-retrieve-image-info-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
name: retrieve-image-info
spec:
description: Retrieve location and SHA of the built image from the previous PipelineRun
params:
- name: namespace
type: string
- name: model-name
type: string
steps:
- name: get-image-sha
image: image-registry.openshift-image-registry.svc:5000/openshift/cli:latest
script: |
oc get -n $(params.namespace) pipelinerun --selector tekton.dev/pipeline=test-mlflow-image --sort-by=.status.completionTime -o jsonpath='{range .items[?(@.spec.params[0].name == "model-name")]}{.spec.params[?(@.name == "model-name")].value} {.status.results[?(@.name == "image-sha")].value} {.status.results[?(@.name == "target-registry-url")].value}{"\n"}{end}' | awk -v model=$(params.model-name) '$1 == model && NF == 3 { print $2, $3 }' | tail -1 | tee /dev/stderr | while read sha registry ; do echo -n "$sha" > $(results.image-sha.path) ; echo -n "$registry" > $(results.target-registry-url.path) ; done ;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since a Task is supposed to be a reusable self-contained action, I think this script should stay embedded in the Pipeline since it is hardcoded to test-mlflow-image.

Instead of querying a PipelineRun, can we simplify this workflow to rely on a known incluster ImageTag OR a ConfigMap that holds the current ImagestreamTag to reference for this Pipeline step

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is, with a task defined merely as an entry in tasks in Pipeline, we are not able to have results there, IIRC. The only place I found results definition working is when I created it as a separate Task.

Yes, the task hardcodes test-mlflow-image -- but we have to hardcode something, to be able to reliably pick up the values from the previous PipelineRuns. I can turn it into a default of a parameter, if that's what you are suggesting.

The problem of using an ImageTag -- we want to be able to record in the pull request the SHA-256 of the exact image that was tested by the test-mlflow-image Pipeline. And the tag can change between the testing PipelineRun running the application and this PipelineRun finding the tag. That's why the whole logic of providing that value through results in the PipelineRuns.

Or do you suggest for the test-mlflow-image to set some very-well-known tag like :testing-passed any time the tests pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, my ideal approach would be to do it all in one Pipeline, test and file the pull request. But @piotrpdev was not fond of that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or do you suggest for the test-mlflow-image to set some very-well-known tag like :testing-passed any time the tests pass?

Your intent is sound but in the event we have to pass some config between PipelineRuns then we should use a configmap or annotation on the target object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged the changes after approval by @piotrpdev and after having #146 for the followup work. Let's consider this an interim quick-fix for the #125 problem and a baseline for further work.

test -s $(results.image-sha.path) && test -s $(results.target-registry-url.path)
results:
- name: target-registry-url
description: The target-registry-url where the image will be stored
- name: image-sha
description: The image checksum
1 change: 1 addition & 0 deletions pipelines/tekton/test-mlflow-image-pipeline/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ resources:
- bike-rentals-test-data-cm.yaml
- tensorflow-housing-test-data-cm.yaml
- test-mlflow-rest-svc-task.yaml
- test-mlflow-record-image-task.yaml
- test-mlflow-image-pipeline.yaml
26 changes: 23 additions & 3 deletions pipelines/tekton/test-mlflow-image-pipeline/test-mlflow-image-pipeline.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ spec:
- name: upon-end
type: string
default: delete
results:
- name: target-registry-url
value: $(tasks.record-image.results.target-registry-url)
- name: image-sha
value: $(tasks.record-image.results.image-sha)
tasks:
- name: deploy-container
params:
Expand Down Expand Up @@ -124,18 +129,33 @@ spec:
taskRef:
kind: ClusterTask
name: openshift-client
- name: record-image
taskRef:
kind: Task
name: record-image
params:
- name: model-name
value: $(params.model-name)
- name: model-version
value: $(params.model-version)
- name: namespace
value: $(params.target-namespace)
- name: target-registry-url
value: quay.io/$(params.target-imagerepo)/$(params.model-name)
runAfter:
- test-mlflow-rest-svc
- name: skopeo-copy
params:
- name: srcImageURL
value: docker://image-registry.openshift-image-registry.svc:5000/$(context.pipelineRun.namespace)/$(params.model-name):$(params.model-version)
value: docker://image-registry.openshift-image-registry.svc:5000/$(context.pipelineRun.namespace)/$(params.model-name)@$(tasks.record-image.results.image-sha)
- name: destImageURL
value: docker://quay.io/$(params.target-imagerepo)/$(params.model-name):$(params.model-version)-$(context.pipelineRun.uid)
value: docker://$(tasks.record-image.results.target-registry-url):$(params.model-version)-$(context.pipelineRun.uid)
- name: srcTLSverify
value: "true"
- name: destTLSverify
value: "true"
runAfter:
- test-mlflow-rest-svc
- record-image
taskRef:
kind: ClusterTask
name: skopeo-copy
Expand Down
2 changes: 0 additions & 2 deletions pipelines/tekton/test-mlflow-image-pipeline/test-mlflow-image-pipelinerun-bike-rental.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
labels:
tekton.dev/pipeline: test-mlflow-image
generateName: test-mlflow-image-bike-rentals-
spec:
params:
Expand Down
2 changes: 0 additions & 2 deletions ...s/tekton/test-mlflow-image-pipeline/test-mlflow-image-pipelinerun-tensorflow-housing.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
labels:
tekton.dev/pipeline: test-mlflow-image
generateName: test-mlflow-image-tensorflow-housing-
spec:
params:
Expand Down
27 changes: 27 additions & 0 deletions pipelines/tekton/test-mlflow-image-pipeline/test-mlflow-record-image-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
name: record-image
spec:
description: Record location and SHA of the built image
params:
- name: namespace
type: string
- name: model-name
type: string
- name: model-version
type: string
- name: target-registry-url
type: string
steps:
- name: get-image-sha
image: image-registry.openshift-image-registry.svc:5000/openshift/cli:latest
script: |
echo -n "$(params.target-registry-url)" | tee $(results.target-registry-url.path) ;
echo ;
oc get -n $(params.namespace) -o jsonpath='{.status.tags[?(@.tag == "$(params.model-version)")].items[0].image}' imagestream/$(params.model-name) | tee $(results.image-sha.path)
results:
- name: target-registry-url
description: The target-registry-url where the image will be stored
- name: image-sha
description: The image checksum