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

Test: Run e2e test on Packit #638

Merged
merged 13 commits into from
Jul 3, 2024
Merged

Test: Run e2e test on Packit #638

merged 13 commits into from
Jul 3, 2024

Conversation

henrywang
Copy link
Contributor

@henrywang henrywang commented Jun 25, 2024

This PR includes:

  1. rename old integration test to e2e test
  2. drop aws test, use libvirt test (for bootc install to-existing-root and bootc switch test) instead
  3. drop rhel test
  4. replace quay.io registry by local registry
  5. use Packit to run e2e test
  6. build bootc PRM package on copr. Support x86_64, aarch64, and ppc64le arch and RHEL 9, CS9, Fedora-40/rawhide distros.
  7. the integration workflow removed.

tests/e2e/bootc-install.sh Outdated Show resolved Hide resolved
Comment on lines +39 to +48
/to-existing-root:
summary: Run bootc install to-existing-root and bootc switch test locally (nested)
environment+:
TEST_CASE: to-existing-root
discover+:
test:
- /to-existing-root
adjust+:
- when: arch == ppc64le
enabled: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

This syntax is YAML sorcery to me...is there any description of what the / and + do in particular?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When configure tmt plan or test, it supports regular expression, starting with / to avoid regular expression, but just a string match. The / comes from plan or test name, for example:

➜ tmt test ls
/tests/e2e/e2e/to-disk
/tests/e2e/e2e/to-existing-root
/tests-integration/build-image

The + means adding addition configuration based on top level configuration. In this case, The top level has adjust configured already.

tests/e2e/playbooks/templates/user-data.j2 Show resolved Hide resolved
@henrywang henrywang force-pushed the new_e2e branch 24 times, most recently from 3f74106 to d649683 Compare June 27, 2024 15:05
@henrywang henrywang requested a review from cgwalters June 27, 2024 16:08
Copy link
Contributor Author

@henrywang henrywang Jun 27, 2024

Choose a reason for hiding this comment

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

Hi @cgwalters, I made some changes here to support update bootc.spec to work with copr build requirement. Any comment on this code change? I'm just a beginner for Rust. :-) Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Code overall looks fine 😄

@henrywang henrywang marked this pull request as ready for review June 27, 2024 16:12
@henrywang
Copy link
Contributor Author

There is outage in Packit testing farm runner for x86_64 runner. I'll re-run the x86_64 tests after outage fixed. Please don't merge this PR until all tests passed. Thanks.

@@ -246,6 +247,41 @@ fn package(sh: &Shell) -> Result<()> {
Ok(())
}

fn update_spec(sh: &Shell) -> Result<Utf8PathBuf> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm...but this overlaps with package-srpm right? I think in theory we could share code here...or even extract the spec from the srpm?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This also relates to #651 and the copr makefile...we've grown way too many copies of this logic.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Code overall looks fine 😄

@henrywang henrywang force-pushed the new_e2e branch 2 times, most recently from 8619cd8 to 01a3ff3 Compare July 3, 2024 00:18
@henrywang
Copy link
Contributor Author

henrywang commented Jul 3, 2024

Hi @cgwalters! We can merge this PR now.
Sometimes, the testing farm might report Guest couldn't be provisioned: Artemis resource ended in 'error' state. Ticket add more bare metal instance types to public ranch to track this issue.

Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Needs a rebase, but otherwise looks sane to me!

1. rename old integration test to e2e tet
2. drop aws test, use libvirt test (for bootc install to-existing-root
   and bootc switch test) instead
3. drop rhel test
4. replace quay.io registry by local registry

Signed-off-by: Xiaofeng Wang <[email protected]>
delete workflow file because e2e runs by Packit
delete mockbuild because build bootc rpm on copr

Signed-off-by: Xiaofeng Wang <[email protected]>
deploy-libvirt playbook

Signed-off-by: Xiaofeng Wang <[email protected]>
} else {
writeln!(o, "{}", line)?;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very minor and we can fix in a followup, but just glancing at this again I notice we're missing a call to into_inner here which will ensure the buffer gets flushed.

While Rust means never having to close a socket it does mean you can forget to flush a buffered writer and then have the ugly semantic where if that fails the file is silently only partially written.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Learning and learned! Thanks.

@cgwalters cgwalters enabled auto-merge July 3, 2024 14:30
@cgwalters cgwalters merged commit 9459a66 into containers:main Jul 3, 2024
9 of 36 checks passed
@henrywang henrywang deleted the new_e2e branch July 3, 2024 14:46
cgwalters added a commit to cgwalters/bootc that referenced this pull request Nov 6, 2024
Update to use external `ocidir` crate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants