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.

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

commit-image-ref does not respect target-imagerepo #92

Closed
adelton opened this issue Sep 21, 2023 · 5 comments · Fixed by #137
Closed

commit-image-ref does not respect target-imagerepo #92

adelton opened this issue Sep 21, 2023 · 5 comments · Fixed by #137
Assignees
Labels
kind/bug Something isn't working kind/documentation Improvements or additions to documentation priority/high Important issue that needs to be resolved asap. Releases should not have too many o

Comments

@adelton
Copy link
Contributor

adelton commented Sep 21, 2023

When checking what commit-image-ref of Deploy the GitOps pipeline created in pipeline_<UUID> branch in my Git repository, I see

diff --git a/acm/odh-edge/apps/tensorflow-housing-app/kustomization.yaml b/acm/odh-edge/apps/tensorflow-housing-app/kustomization.yaml
index c3ed7b7..77c7090 100644
--- a/acm/odh-edge/apps/tensorflow-housing-app/kustomization.yaml
+++ b/acm/odh-edge/apps/tensorflow-housing-app/kustomization.yaml
@@ -59,4 +59,4 @@ replacements:
 images:
 - name: edge-model-template-image
   newName: quay.io/rhoai-edge/tensorflow-housing
-  digest: sha256:b5f11c2f380e76bb4dc5b66e40fff46ee277cd9a7477a6519b95dc8017f67852
+  digest: sha256:19e7312d8cbfcbf21dcab083f9ef99af2f807f156b1216a423267010b7fdd7c0

Since the skopeo-copy task from the previous step copied the image to my Quay namespace configured by target-imagerepo, I would expect this commit-image-ref to apply that same value to newName. The image sha256:19e7312d8cbfcbf21dcab083f9ef99af2f807f156b1216a423267010b7fdd7c0 does not exist in quay.io/rhoai-edge/tensorflow-housing if it was pushed to a different namespace.

@adelton
Copy link
Contributor Author

adelton commented Sep 21, 2023

Hmmm, so sha256:19e7312d8cbfcbf21dcab083f9ef99af2f807f156b1216a423267010b7fdd7c0 actually does exist in quay.io/rhoai-edge/tensorflow-housing. But it looks like the gitops-update-pipeline merely copies information from pipelines/tekton/gitops-update-pipeline/example-pipelineruns/gitops-update-pipelinerun-tensorflow-housing.yaml to acm/odh-edge/apps/tensorflow-housing-app/kustomization.yaml.

But wasn't the the goal of the exercise to actually push to Quay and record in the PR the image that was built in azureml-container's build-mlflow-container task?

@adelton
Copy link
Contributor Author

adelton commented Oct 4, 2023

@piotrpdev @grdryn I try to investigate the intended behaviour here, and I got to a more general question -- why are the test-mlflow-image-pipeline and gitops-update-pipeline Pipelines separate? The test-mlflow-image-pipeline knows what it pushed and where, and if the test of the model in the image passed and image got pushed to Quay, shouldn't the filing of the pull request be done right away?

If it was the same Pipeline, we could just use the target-imagerepo value in commit-image-ref Task.

(One might also add, the build pipeline azureml-container-pipeline could also be part of that same Pipeline.)

@adelton
Copy link
Contributor Author

adelton commented Oct 4, 2023

Another problem is that currently the GitOps pipeline merely copies a static SHA-256 from pipelines/tekton/gitops-update-pipeline/example-pipelineruns/gitops-update-pipelinerun-*.yaml to the pull request which does not make much sense. It should most likely use skopeo or something to get the SHA-256 of the image that was built and pushed ... and for that, merging it into the test-mlflow-image-pipeline would again help because all the parameters are there, so the values would be easier to obtain.

@adelton
Copy link
Contributor Author

adelton commented Oct 4, 2023

Ah, so merging those two pipelines was already proposed in #68 (comment).

@piotrpdev piotrpdev added the kind/bug Something isn't working label Oct 4, 2023
@piotrpdev piotrpdev added priority/high Important issue that needs to be resolved asap. Releases should not have too many o kind/documentation Improvements or additions to documentation labels Oct 5, 2023
@adelton adelton self-assigned this Oct 10, 2023
@adelton
Copy link
Contributor Author

adelton commented Oct 10, 2023

Similar or same as #125 -- working on it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working kind/documentation Improvements or additions to documentation priority/high Important issue that needs to be resolved asap. Releases should not have too many o
Projects
Status: Done
2 participants