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.

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

Using Alpine extended image #7520

Merged
merged 3 commits into from
Apr 29, 2022
Merged

Using Alpine extended image #7520

merged 3 commits into from
Apr 29, 2022

Conversation

AlonaKaplan
Copy link
Member

@AlonaKaplan AlonaKaplan commented Apr 5, 2022 •
edited

What this PR does / why we need it:
The PR is using alpine extended image that contains cloud-init and ga.

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:

Release note:

NONE

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/XL sig/network labels Apr 5, 2022
@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.22-sig-network

@AlonaKaplan AlonaKaplan marked this pull request as draft April 5, 2022 16:32
@kubevirt-bot kubevirt-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 5, 2022
@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.22-sig-network

@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.22-sig-network

@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.22-sig-network

@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network

1 similar comment
@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network

This was referenced Apr 10, 2022
Merged
Closed
@AlonaKaplan
Copy link
Member Author

AlonaKaplan commented Apr 10, 2022 •
edited

Summary
IPv6 single stack cluster-

  1. Using Cirros with long timout - ~2h (Support single stack IPv6 #7158)
  2. Using old Alpine image and configuring the networking via the console- ~45m (Replace Cirros with Alpine #7435)
  3. Using extended Alpine with cloud-init - ~1h

Dual stack cluster-

  1. Using Cirros - ~1:25 (Support single stack IPv6 #7158)
  2. Using old Alpine image and configuring the networking via the console- ~1:15m (Replace Cirros with Alpine #7435)
  3. Using extended Alpine with cloud-init - ~1:25h

@qinqon
Copy link
Contributor

qinqon commented Apr 12, 2022 •
edited

Summary IPv6 single stack cluster-

  1. Using Cirros with long timout - ~2h (Support single stack IPv6 #7158)
  2. Using old Alpine image and configuring the networking via the console- ~45m (Replace Cirros with Alpine #7435)
  3. Using extended Alpine with cloud-init - ~1h

Dual stack cluster-

  1. Using Cirros - ~1:25 (Support single stack IPv6 #7158)
  2. Using old Alpine image and configuring the networking via the console- ~1:15m (Replace Cirros with Alpine #7435)
  3. Using extended Alpine with cloud-init - ~1:25h

Do we know where time goes at alpine console vs cloud-init ? alpine cloud-init takes around 15m more and cloud-init is suppose to execute sooner than console, maybe is the image size ? cloud-init has a lot of dependencies since it's a python package.

Can we do the same test of alpine console but using the alpine expended image ? so we verify if the image size is the issue ?

@qinqon qinqon mentioned this pull request Apr 12, 2022
Merged
@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 12, 2022
@kubevirt-bot kubevirt-bot added size/L and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL labels Apr 12, 2022
@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.22-sig-network

@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.22-sig-network

@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 27, 2022
@AlonaKaplan
tests: Add AlpineExtended image to the tests
1d75424 
The new image includes cloud-init.
The next commits will use this image and configure the Alpine networking
using cloud-init network-data instead of configuring it via the console.

This commit includes the following -
- Adding the new image to the local registry.
- Introduce libvmi factory - "NewAlpineExtended"
- Introduce libvmi network Options - "WithMasqueradeNetworking"

Signed-off-by: Alona Kaplan <alkaplan@redhat.com>
@AlonaKaplan
tests, LoginToAlpine: if vmi.Spec.Hostname exists expect it in the pr…
c2d9c81 
…ompt

Signed-off-by: Alona Kaplan <alkaplan@redhat.com>
@AlonaKaplan
tests: use AlpineExtended image instead of configuring Alpine via con…
0f448ca 
…sole

Signed-off-by: Alona Kaplan <alkaplan@redhat.com>
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 27, 2022
@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-arm64

@AlonaKaplan
Copy link
Member Author

/hold the PR has to be updated with the kubevirtci version of the images

@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 Apr 27, 2022
@AlonaKaplan
Copy link
Member Author

/test pull-kubevirt-e2e-arm64

@AlonaKaplan
Copy link
Member Author

@AlonaKaplan Since the new image doesn't seem to have an aarch64 version, shouldn't we SkipIfARM64() the tests that use it?

@jean-edouard thanks a lot for the comment! Do you know when our test are run on ARM64? Since we want to move most of our tests to use this alpine image, skipping it means that most of the tests won't run on ARM64.

Yeah, let's build the image for arm64 and not skip anything!

@jean-edouard just realized the arm lanes are running only test with the "arm64" label. Non of the tests that are using the new "AlpineWithTestTooling" label having this label. Therefore, we can continue with this PR without introducing an "arm64" image.

Is there anything else blocking the PR?

/hold cancel

@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 Apr 27, 2022
@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 27, 2022
@jean-edouard
Copy link
Contributor

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jean-edouard, phoracek

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 27, 2022
@AlonaKaplan
Copy link
Member Author

/retest

4 similar comments
@qinqon
Copy link
Contributor

qinqon commented Apr 28, 2022

/retest

@AlonaKaplan
Copy link
Member Author

/retest

@AlonaKaplan
Copy link
Member Author

/retest

@EdDev
Copy link
Member

EdDev commented Apr 28, 2022

/retest

@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-bot
Copy link
Contributor

kubevirt-bot commented Apr 29, 2022 •
edited

@AlonaKaplan: 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
pull-kubevirt-e2e-arm64 d5269d9 link false /test pull-kubevirt-e2e-arm64

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/test-infra repository. I understand the commands that are listed here.

@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-bot kubevirt-bot merged commit 1ce6b3c into kubevirt:main Apr 29, 2022
@victortoso victortoso mentioned this pull request Feb 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qinqon qinqon qinqon approved these changes

@phoracek phoracek phoracek approved these changes

@EdDev EdDev EdDev approved these changes

@alonSadan alonSadan Awaiting requested review from alonSadan

@ILpinto ILpinto Awaiting requested review from ILpinto

@jean-edouard jean-edouard Awaiting requested review from jean-edouard

@rnetser rnetser Awaiting requested review from rnetser

Assignees

@qinqon qinqon

@phoracek phoracek

@EdDev EdDev

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/network size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@AlonaKaplan @qinqon @phoracek @jean-edouard @xpivarc @EdDev @kubevirt-bot @kubevirt-commenter-bot