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

Create common python script for gathering images #1038

Merged
merged 6 commits into from
Sep 6, 2024

Conversation

kimwnasptd
Copy link
Contributor

@kimwnasptd kimwnasptd commented Aug 23, 2024

Resolves #1036
Resolves #960

This PR:

  1. Creates a new scripts/airgapped/get-all-images.py script
  2. Removes the old scripts/airgapped/get-all-images.sh script
  3. Updates all the READMEs and airgap scripts to use the new python script
  4. 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 kimwnasptd requested a review from a team as a code owner August 23, 2024 15:07
@kimwnasptd kimwnasptd force-pushed the KF-6146-common-image-gather-script branch from c47928b to 0784a7a Compare August 23, 2024 15:11
@kimwnasptd kimwnasptd force-pushed the KF-6146-common-image-gather-script branch from 0784a7a to 3d404c0 Compare August 23, 2024 15:16
@kimwnasptd
Copy link
Contributor Author

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

Copy link
Contributor

@DnPlas DnPlas left a comment

Choose a reason for hiding this comment

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

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 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
Copy link
Contributor

@NohaIhab NohaIhab left a comment

Choose a reason for hiding this comment

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

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
@NohaIhab
Copy link
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.

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
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
Copy link
Contributor

@NohaIhab NohaIhab left a comment

Choose a reason for hiding this comment

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

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
scripts/get-all-images.py Outdated Show resolved Hide resolved
scripts/get-all-images.py Outdated Show resolved Hide resolved
@NohaIhab
Copy link
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
Copy link
Contributor Author

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

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

@kimwnasptd 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
Copy link

@rgildein rgildein left a comment

Choose a reason for hiding this comment

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

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
rgildein
rgildein previously approved these changes Sep 5, 2024
Copy link

@rgildein rgildein left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

scripts/README.md Outdated Show resolved Hide resolved
@kimwnasptd kimwnasptd force-pushed the KF-6146-common-image-gather-script branch from 7ff0ea9 to 7007984 Compare September 6, 2024 10:23
Copy link
Contributor

@NohaIhab NohaIhab left a comment

Choose a reason for hiding this comment

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

thanks @kimwnasptd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants