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

Create common python script for gathering images #1038

Merged

kimwnasptd merged 6 commits into main from KF-6146-common-image-gather-script

Sep 6, 2024

Contributor

kimwnasptd commented Aug 23, 2024 •

edited

Loading

Resolves #1036
Resolves #960

This PR:

Creates a new scripts/airgapped/get-all-images.py script
Removes the old scripts/airgapped/get-all-images.sh script
Updates all the READMEs and airgap scripts to use the new python script
Update the GH Action of scanning to use this script, instead of the bash one in kubeflow-ci

More information and technical decisions are exposed in #1036

kimwnasptd requested a review from a team as a code owner

August 23, 2024 15:07

kimwnasptd force-pushed the KF-6146-common-image-gather-script branch from c47928b to 0784a7a Compare

August 23, 2024 15:11

kimwnasptd mentioned this pull request

Fix the scanning for 1.9 #1029

Closed

kimwnasptd force-pushed the KF-6146-common-image-gather-script branch from 0784a7a to 3d404c0 Compare

August 23, 2024 15:16

Contributor Author

kimwnasptd commented Aug 23, 2024

You can also find a scanning workflow run with the above code, that is failing because of canonical/knative-operators#219
https://github.com/canonical/bundle-kubeflow/actions/runs/10528194560/job/29173123429

kimwnasptd force-pushed the KF-6146-common-image-gather-script branch from ebc9b6b to ca11768 Compare

August 23, 2024 15:38

kimwnasptd mentioned this pull request

Add GitHub metadata for repo/branches in 1.9 #1039

Merged

DnPlas reviewed

View reviewed changes

Contributor

DnPlas left a comment

Thanks @kimwnasptd, left some comments.

.github/workflows/scan-images.yaml Outdated Show resolved Hide resolved

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

kimwnasptd force-pushed the KF-6146-common-image-gather-script branch 2 times, most recently from 000abdc to 359d1ad Compare

August 31, 2024 09:24

NohaIhab reviewed

View reviewed changes

Contributor

NohaIhab left a comment

thanks @kimwnasptd , left some comments.

scripts/airgapped/requirements.txt Outdated Show resolved Hide resolved

scripts/get-all-images.py Outdated Show resolved Hide resolved

scripts/get-all-images.py Show resolved Hide resolved

scripts/get-all-images.py Show resolved Hide resolved

scripts/get-all-images.py Outdated Show resolved Hide resolved

tests/airgapped/README.md Outdated Show resolved Hide resolved

Contributor

NohaIhab commented Sep 3, 2024

Can you link in your PR description #960 since it addresses the issue.
You can comment there the approach that you are taking.

DnPlas reviewed

View reviewed changes

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

DnPlas reviewed

View reviewed changes

scripts/airgapped/get-all-images.py Outdated Show resolved Hide resolved

DnPlas reviewed

View reviewed changes

scripts/get-all-images.py Outdated Show resolved Hide resolved

DnPlas reviewed

View reviewed changes

scripts/get-all-images.py Outdated Show resolved Hide resolved

DnPlas reviewed

View reviewed changes

scripts/get-all-images.py Outdated Show resolved Hide resolved

DnPlas reviewed

View reviewed changes

scripts/get-all-images.py Show resolved Hide resolved

rgildein reviewed

View reviewed changes

scripts/get-all-images.py Outdated Show resolved Hide resolved

scripts/get-all-images.py Outdated Show resolved Hide resolved

scripts/get-all-images.py Outdated Show resolved Hide resolved

scripts/get-all-images.py Outdated Show resolved Hide resolved

NohaIhab reviewed

View reviewed changes

Contributor

NohaIhab left a comment •

edited

Loading

small comments on the readme and file structure

tests/airgapped/README.md Outdated Show resolved Hide resolved

tests/airgapped/README.md Outdated Show resolved Hide resolved

scripts/get-all-images.py Outdated Show resolved Hide resolved

tests/airgapped/README.md Outdated Show resolved Hide resolved

NohaIhab reviewed

View reviewed changes

scripts/airgapped/README.md Show resolved Hide resolved

NohaIhab reviewed

View reviewed changes

scripts/get-all-images.py Outdated Show resolved Hide resolved

NohaIhab reviewed

View reviewed changes

scripts/get-all-images.py Outdated Show resolved Hide resolved

Contributor

NohaIhab commented Sep 4, 2024

Hey @kimwnasptd, could you now link a successful run of the workflow? now that canonical/knative-operators#219 is closed

kimwnasptd mentioned this pull request

Update images used for testing an airgapped CKF 1.9 deployment #1051

Closed

Contributor Author

kimwnasptd commented Sep 4, 2024

@NohaIhab sure! Here's a latest run of the current code.

https://github.com/canonical/bundle-kubeflow/actions/runs/10708875310

kimwnasptd force-pushed the KF-6146-common-image-gather-script branch 3 times, most recently from a8ee551 to 5ebfad2 Compare

September 5, 2024 08:42

rgildein suggested changes

View reviewed changes

rgildein left a comment

We need to update README with latest approach, other than that only not blocking comments.
I tested it locally and it works fine.

scripts/requirements.txt Show resolved Hide resolved

scripts/get-all-images.py Show resolved Hide resolved

scripts/get-all-images.py Show resolved Hide resolved

tests/airgapped/README.md Outdated Show resolved Hide resolved

kimwnasptd force-pushed the KF-6146-common-image-gather-script branch from f1aaa83 to b3fc058 Compare

September 5, 2024 12:22

kimwnasptd mentioned this pull request

Airgap tests files structure doesn't currently support multiple versions #1054

Closed

rgildein previously approved these changes

View reviewed changes

rgildein left a comment

LGTM, thanks.

kimwnasptd dismissed rgildein’s stale review via

29d3710

September 5, 2024 13:03

kimwnasptd force-pushed the KF-6146-common-image-gather-script branch from 52c998d to a31ae34 Compare

September 5, 2024 16:05

kimwnasptd mentioned this pull request

Repo's /script folder structure is not intuitive #1058

Open

NohaIhab reviewed

View reviewed changes

scripts/README.md Outdated Show resolved Hide resolved

NohaIhab mentioned this pull request

Update images used in testing an airgapped CKF 1.9 deployment #1053

Merged

This was referenced Sep 6, 2024

Investigate switching to jsonschema for validating the bundle in image-gather script #1059

Open

Investigate having an --output parameter in our CLI scripts #1060

Open

kimwnasptd added 6 commits

September 6, 2024 13:19


          Create get-all-images.py

00135df


          requirements.txt

aa1e154


          Use the get-all-images.py in airgap tests

163df40


          Use get-all-images.py from scanning action

fc94d7e

Signed-off-by: Kimonas Sotirchos <[email protected]>


          remove get-all-images.sh

bb72b21

Signed-off-by: Kimonas Sotirchos <[email protected]>


          Address review comments

kimwnasptd force-pushed the KF-6146-common-image-gather-script branch from 7ff0ea9 to 7007984 Compare

September 6, 2024 10:23

NohaIhab approved these changes

View reviewed changes

Contributor

NohaIhab left a comment

thanks @kimwnasptd

kimwnasptd merged commit ea6126f into main

kimwnasptd deleted the KF-6146-common-image-gather-script branch

September 6, 2024 10:27

kimwnasptd mentioned this pull request

Improve airgapped setup and testing scripts #933

Closed

kimwnasptd mentioned this pull request

Scan images job is failing with different error messages #1080

Closed

NohaIhab mentioned this pull request

Refactor airgapped scripts to work for any bundle version #959

Open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet