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

fix,pre-pull-images: use pre-generated image lists for pre-pull #1374

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dhiller
Copy link
Contributor

@dhiller dhiller commented Feb 7, 2025

What this PR does / why we need it:

Detection of missing pre-pulled images broke when the provisioning was moved into gocli. Therefore we change from calling fetch-images.sh at runtime to a generated pre-pull-images file.

Changes in this PR:

  • generation: script update-pre-pull-images.sh generates pre-pull-images, calling fetch-images.sh, which is moved from the version folders into the main provision/k8s folder
  • simplification: replace the logic of pre-pulling based on fetch-images.sh and extra-... with simply concatting pre-pull-images and extra-pre-pull-images text file
  • safety-net: pre-pull-images needs to get updated whenever the manifests inside either the k8s version folder or the manifests inside the gocli change - add a call to update-pre-pull-images.sh to check idempotency (no changes in repo) in check-cluster-up.sh
  • install: set some more flags to install cdi and ceph, remove the install-everything script

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

@kubevirt-bot
Copy link
Contributor

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

@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Feb 7, 2025
@kubevirt-bot kubevirt-bot added size/L needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 7, 2025
@dhiller
Copy link
Contributor Author

dhiller commented Feb 10, 2025

/test
check-provision-k8s-1.32

@kubevirt-bot
Copy link
Contributor

@dhiller: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

  • /test check-gocli
  • /test check-provision-centos-base
  • /test check-provision-k8s-1.30
  • /test check-provision-k8s-1.30-s390x
  • /test check-provision-k8s-1.31
  • /test check-provision-k8s-1.32
  • /test check-provision-k8s-1.32-s390x
  • /test check-provision-manager
  • /test check-up-kind-1.30-vgpu
  • /test check-up-kind-ovn
  • /test check-up-kind-sriov

The following commands are available to trigger optional jobs:

  • /test check-provision-alpine-with-test-tooling
  • /test check-provision-k8s-1.31-s390x
  • /test check-up-kind-1.28
  • /test check-up-kind-1.31

Use /test all to run the following jobs that were automatically triggered:

  • check-gocli
  • check-provision-alpine-with-test-tooling
  • check-provision-k8s-1.30
  • check-provision-k8s-1.31
  • check-provision-k8s-1.31-s390x
  • check-provision-k8s-1.32
  • check-provision-manager
  • check-up-kind-1.30-vgpu
  • check-up-kind-sriov

In response to this:

/test
check-provision-k8s-1.32

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.

@dhiller
Copy link
Contributor Author

dhiller commented Feb 10, 2025

/test check-provision-k8s-1.32

@dhiller dhiller force-pushed the fix-pre-pull-images branch from 483f345 to 38115f0 Compare February 10, 2025 10:44
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2025
@dhiller
Copy link
Contributor Author

dhiller commented Feb 10, 2025

/test check-provision-k8s-1.32

@dhiller dhiller force-pushed the fix-pre-pull-images branch from 38115f0 to 1019d09 Compare February 10, 2025 15:01
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from dhiller. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@dhiller dhiller force-pushed the fix-pre-pull-images branch from 1019d09 to 3e7c641 Compare February 10, 2025 15:01
@dhiller dhiller changed the title WIP pre-pull-images: move fetch-images.sh to manual fix,pre-pull-images: move fetch-images.sh to manual Feb 10, 2025
@dhiller dhiller changed the title fix,pre-pull-images: move fetch-images.sh to manual fix,pre-pull-images: use pre-generated image lists for pre-pull Feb 10, 2025
@dhiller
Copy link
Contributor Author

dhiller commented Feb 10, 2025

/test check-provision-k8s-1.32

@dhiller
Copy link
Contributor Author

dhiller commented Feb 11, 2025

/hold

waiting for kubevirt/project-infra#3948

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 11, 2025
@dhiller
Copy link
Contributor Author

dhiller commented Feb 11, 2025

updating images through kubevirt/project-infra#3949

@dhiller
Copy link
Contributor Author

dhiller commented Feb 12, 2025

/unhold

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 12, 2025
@dhiller
Copy link
Contributor Author

dhiller commented Feb 12, 2025

/test check-provision-k8s-1.32

since the manifests are not inside the cluster deploy directory any more
they are not being copied into the directory any more, thus the
fetch-images.sh will not find any images.

Therefore we move the fetch-images.sh into the main provision/k8s folder
and replace the per version with a pre-pull-images text file.

This file needs to get updated whenever the manifests inside either the
k8s version folder or the manifests inside the gocli change.

Thus we also create a script update-pre-pull-images.sh and call that
whenever the cluster is checked at first.

Signed-off-by: Daniel Hiller <[email protected]>
@dhiller dhiller force-pushed the fix-pre-pull-images branch from 3e7c641 to d80ea5a Compare February 13, 2025 09:01
@dhiller
Copy link
Contributor Author

dhiller commented Feb 13, 2025

/test check-provision-k8s-1.32

@dhiller dhiller marked this pull request as ready for review February 13, 2025 09:38
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 13, 2025
@kubevirt-bot kubevirt-bot requested a review from qinqon February 13, 2025 09:38
@dhiller
Copy link
Contributor Author

dhiller commented Feb 13, 2025

@brianmcarey I think I have it working now :)

@dhiller
Copy link
Contributor Author

dhiller commented Feb 17, 2025

Hm, strange, on all the check-provision the same test fails: [sig-network] [rfe_id:694][crit:medium][vendor:[email protected]][level:component]Networking VirtualMachineInstance with masquerade binding mechanism when performing migration [Conformance] should preserve connectivity - IPv6

Any ideas?

@dhiller
Copy link
Contributor Author

dhiller commented Feb 17, 2025

/test check-provision-1.30
/test check-provision-1.31
/test check-provision-1.32

@kubevirt-bot
Copy link
Contributor

@dhiller: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test check-gocli
/test check-provision-centos-base
/test check-provision-k8s-1.30
/test check-provision-k8s-1.30-s390x
/test check-provision-k8s-1.31
/test check-provision-k8s-1.32
/test check-provision-k8s-1.32-s390x
/test check-provision-manager
/test check-up-kind-1.30-vgpu
/test check-up-kind-ovn
/test check-up-kind-sriov

The following commands are available to trigger optional jobs:

/test check-provision-alpine-with-test-tooling
/test check-provision-k8s-1.31-s390x
/test check-up-kind-1.28
/test check-up-kind-1.31

Use /test all to run the following jobs that were automatically triggered:

check-gocli
check-provision-alpine-with-test-tooling
check-provision-k8s-1.30
check-provision-k8s-1.31
check-provision-k8s-1.31-s390x
check-provision-k8s-1.32
check-provision-manager
check-up-kind-1.30-vgpu
check-up-kind-sriov

In response to this:

/test check-provision-1.30
/test check-provision-1.31
/test check-provision-1.32

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.

@dhiller
Copy link
Contributor Author

dhiller commented Feb 17, 2025

/test check-provision-k8s-1.30
/test check-provision-k8s-1.31
/test check-provision-k8s-1.32

@kubevirt-bot
Copy link
Contributor

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

Test name Commit Details Required Rerun command
check-provision-k8s-1.31-s390x d80ea5a link false /test check-provision-k8s-1.31-s390x
check-provision-k8s-1.31 d80ea5a link true /test check-provision-k8s-1.31
check-provision-k8s-1.30 d80ea5a link true /test check-provision-k8s-1.30
check-provision-k8s-1.32 d80ea5a link true /test check-provision-k8s-1.32

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants