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

[GHA] digest updater workflow fixes and updates #566

Merged

Conversation

jstourac
Copy link
Member

@jstourac jstourac commented Jun 15, 2024

This contains changes for the digest update workflow for the update of notebook images and commit shas. There are couple of fixes and a small refactoring done, more details in particular commits:

  • [GHA] fix the regexp used for the latest image tag selection
  • [GHA] fix the missing habana image update record for the digest updater
  • [GHA] Minor refactoring

Truth is that I don't fully understand, how is possible that in this case #565 the rstudio images were selected properly 🤔


https://issues.redhat.com/browse/RHOAIENG-8813

How I tested, check my comment below. As stated there, my plan was also to deduplicate the code, but it's WIP on my machine right now, so if this is needed, we can merge this and I will raise another followup PR after my PTO.

How Has This Been Tested?

Right now, it hasn't been tested, will do later.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Copy link
Contributor

openshift-ci bot commented Jun 15, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jstourac jstourac self-assigned this Jun 15, 2024
@jstourac jstourac changed the title [CI] fix the regexp used for the latest image tag selection [GHA] fix the regexp used for the latest image tag selection Jun 19, 2024
In case that there is an image tag subpart of another tag, there could
be wrong tag chosen and as a result wrong SHA used for the manifest
update. E.g. these two:

* "rstudio-c9s-python-3.9-2024a-20240315-02193dd",
* "cuda-rstudio-c9s-python-3.9-2024a-20240315-02193dd",

Example of wrong update opendatahub-io#541 (3df148c).

Together with this, I also updated the quay security analysis script
where is the exact same issue.
@jstourac jstourac force-pushed the fixDigestUpdaterRegexp branch from 58de6d1 to 29c7538 Compare June 19, 2024 13:01
@jstourac
Copy link
Member Author

jstourac commented Jun 19, 2024

Current implementation working example:
* main...jstourac:notebooks:digest-updater-9584749053
* https://github.com/jstourac/notebooks/actions/runs/9584749053

Looks good except that I don't understand why it skipped the habana image update 🤔

Execution time was 33minutes vs 45minutes in upstream.


Okay, mystery solved - reason was that I ran it from the main branch instead of the my branch with the fixes 🤦 so ignore the above.

Example of the wrong update is opendatahub-io#565, where it had to be updated
manually, see the be022e2.
@jstourac jstourac force-pushed the fixDigestUpdaterRegexp branch from ed2f2bb to d2e8887 Compare June 19, 2024 16:51
@jstourac
Copy link
Member Author

Ah, unfortunately GH uses Ubuntu 22.04 LTS, which has Skopeo in old version and doesn't contain the --no-tags option: https://manpages.ubuntu.com/manpages/jammy/man1/skopeo-inspect.1.html. So we have to live without it until a newer OS is introduced (a little bit slower for now).

* Get rid of shell check warnings
* Make the code faster by caching skopeo output
* Make skopeo to retry if it fails (3 times retry)
@jstourac jstourac force-pushed the fixDigestUpdaterRegexp branch from d2e8887 to 0c6aecc Compare June 19, 2024 17:08
@jstourac
Copy link
Member Author

jstourac commented Jun 19, 2024

Anyway, this looks good:

So, unless our check job is faulty, the changes in this PR should be fine.

Didn't check the security analysis, though - but we plan to review and update it anyway, so probably not a big deal right now.

Also, code deduplication using GH job matrix is WIP on my machine locally right now. But looks like current content is ready for review and comments - so if it is required, I can open a followup PR for the code deduplication later.

@jstourac jstourac changed the title [GHA] fix the regexp used for the latest image tag selection [GHA] digest updater workflow fixes and updates Jun 19, 2024
@jstourac jstourac marked this pull request as ready for review June 19, 2024 17:50
@openshift-ci openshift-ci bot requested review from atheo89 and dibryant June 19, 2024 17:50
@jstourac jstourac requested review from jiridanek and harshad16 June 19, 2024 17:50
Copy link
Member

@jiridanek jiridanek left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Contributor

openshift-ci bot commented Jun 21, 2024

@jstourac: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/notebooks-e2e-tests 0c6aecc link true /test notebooks-e2e-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@jiridanek
Copy link
Member

/test notebooks-ubi9-e2e-tests

@atheo89
Copy link
Member

atheo89 commented Jun 26, 2024

Thanks Jan for the update! It looks great now!

I tested this locally and works as expected.
image

On downstream, we will need to open a separate PR to apply the same fixes. In short, on downstream we fetch images from different registries for almost every notebook, also the name of the workflow is different from the one on upstream.

/lgtm
/approve
/override ci/prow/intel-notebooks-e2e-tests

Copy link
Contributor

openshift-ci bot commented Jun 26, 2024

@atheo89: Overrode contexts on behalf of atheo89: ci/prow/intel-notebooks-e2e-tests

In response to this:

Thanks Jan for the update! It looks great now!

I tested this locally and works as expected.
image

On downstream, we will need to open a separate PR to apply the same fixes. In short, on downstream we fetch images from different registries for almost every notebook, also the name of the workflow is different from the one on upstream.

/lgtm
/approve
/override ci/prow/intel-notebooks-e2e-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Contributor

openshift-ci bot commented Jun 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atheo89, jiridanek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jiridanek
Copy link
Member

/cherrypick 2024a
/cherrypick 2023b
/cherrypick 2023a

@openshift-cherrypick-robot

@jiridanek: once the present PR merges, I will cherry-pick it on top of 2024a in a new PR and assign it to you.

In response to this:

/cherrypick 2024a
/cherrypick 2023b
/cherrypick 2023a

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@jiridanek
Copy link
Member

/cherrypick 2023b

@openshift-cherrypick-robot

@jiridanek: once the present PR merges, I will cherry-pick it on top of 2023b in a new PR and assign it to you.

In response to this:

/cherrypick 2023b

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@jiridanek
Copy link
Member

/cherrypick 2023a

@openshift-cherrypick-robot

@jiridanek: once the present PR merges, I will cherry-pick it on top of 2023a in a new PR and assign it to you.

In response to this:

/cherrypick 2023a

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit ea7f467 into opendatahub-io:main Jun 26, 2024
41 checks passed
@openshift-cherrypick-robot

@jiridanek: new pull request created: #592

In response to this:

/cherrypick 2024a
/cherrypick 2023b
/cherrypick 2023a

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot

@jiridanek: #566 failed to apply on top of branch "2023b":

Applying: fix the regexp used for the latest image tag selection
Using index info to reconstruct a base tree...
M	.github/workflows/notebooks-digest-updater-upstream.yaml
A	.github/workflows/runtimes-digest-updater-upstream.yaml
A	ci/security-scan/quay_security_analysis.py
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): ci/security-scan/quay_security_analysis.py deleted in HEAD and modified in fix the regexp used for the latest image tag selection. Version fix the regexp used for the latest image tag selection of ci/security-scan/quay_security_analysis.py left in tree.
CONFLICT (modify/delete): .github/workflows/runtimes-digest-updater-upstream.yaml deleted in HEAD and modified in fix the regexp used for the latest image tag selection. Version fix the regexp used for the latest image tag selection of .github/workflows/runtimes-digest-updater-upstream.yaml left in tree.
Auto-merging .github/workflows/notebooks-digest-updater-upstream.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix the regexp used for the latest image tag selection
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick 2023b

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot

@jiridanek: #566 failed to apply on top of branch "2023a":

Applying: fix the regexp used for the latest image tag selection
Using index info to reconstruct a base tree...
A	.github/workflows/notebooks-digest-updater-upstream.yaml
A	.github/workflows/runtimes-digest-updater-upstream.yaml
A	ci/security-scan/quay_security_analysis.py
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): ci/security-scan/quay_security_analysis.py deleted in HEAD and modified in fix the regexp used for the latest image tag selection. Version fix the regexp used for the latest image tag selection of ci/security-scan/quay_security_analysis.py left in tree.
CONFLICT (modify/delete): .github/workflows/runtimes-digest-updater-upstream.yaml deleted in HEAD and modified in fix the regexp used for the latest image tag selection. Version fix the regexp used for the latest image tag selection of .github/workflows/runtimes-digest-updater-upstream.yaml left in tree.
CONFLICT (modify/delete): .github/workflows/notebooks-digest-updater-upstream.yaml deleted in HEAD and modified in fix the regexp used for the latest image tag selection. Version fix the regexp used for the latest image tag selection of .github/workflows/notebooks-digest-updater-upstream.yaml left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix the regexp used for the latest image tag selection
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick 2023a

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@harshad16
Copy link
Member

@jiridanek , please don't cherrypick pr in 2024a , as we would sync them with teh new automation.
thanks

@jstourac jstourac deleted the fixDigestUpdaterRegexp branch July 4, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants