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

Feature/podman build #1916

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Feature/podman build #1916

merged 2 commits into from
Aug 21, 2024

Conversation

fedinskiy
Copy link
Contributor

Summary

  • Add coverage for podman-build extension in a new module
  • Move the module and docker-build to the same folder

Please select the relevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • Dependency update
  • Refactoring
  • Backport
  • New scenario (non-breaking change which adds functionality)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

@fedinskiy fedinskiy self-assigned this Aug 7, 2024
@fedinskiy fedinskiy force-pushed the feature/podman-build branch 5 times, most recently from bdb5bb2 to 35ccd40 Compare August 8, 2024 13:29
@fedinskiy fedinskiy requested a review from michalvavrik August 9, 2024 14:12
michalvavrik
michalvavrik reviewed Aug 14, 2024
View reviewed changes
build/podman/pom.xml Outdated Show resolved Hide resolved
build/podman/src/test/java/io/quarkus/ts/docker/PodmanBuildIT.java
* since otherwise we won't be able to distinguish between the extension being broken
* and a situation, when podman is not installed.
*
* Unfortunately, that means, that the test is disabled on Windows and Mac
Copy link
Member

Choose a reason for hiding this comment

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

This feels clumsy, why don't you try podman cli command instead, or are there no Podman specific environment variables or something else? We have Windows tests with Podman, so this must be tested there, or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we use DockerUtils, which use DOCKER_HOST, so it is the most direct way to check, that the test will work.

I am not able to start a podman machine on aws, so I can not verify, which parameter is used there.

Copy link
Member

Choose a reason for hiding this comment

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

oki

build/podman/src/main/resources/application.properties Outdated Show resolved Hide resolved
build/podman/src/main/resources/application.properties Outdated Show resolved Hide resolved
build/podman/pom.xml Show resolved Hide resolved
@fedinskiy fedinskiy force-pushed the feature/podman-build branch from 35ccd40 to 91d246d Compare August 19, 2024 10:56
michalvavrik
michalvavrik previously approved these changes Aug 19, 2024
View reviewed changes
@fedinskiy fedinskiy force-pushed the feature/podman-build branch from 91d246d to eb47ea8 Compare August 21, 2024 08:48
@michalvavrik michalvavrik dismissed their stale review August 21, 2024 10:15

I will have to look considering there has been new commits.

@github-actions github-actions bot added the triage/flaky-test Signal that flaky tests were detected during CI run label Aug 21, 2024
@github-actions GitHub Actions
Copy link

Following jobs contain at least one flaky test: 'PR - Linux - JVM build - Latest Version'

@fedinskiy
Copy link
Contributor Author

@michalvavrik I rebased the changed on the newer main to make CI happy. Do you still want to take another look?

@michalvavrik
Copy link
Member

@michalvavrik I rebased the changed on the newer main to make CI happy. Do you still want to take another look?

ah, sorry, ok

@michalvavrik michalvavrik merged commit 9723ea1 into quarkus-qe:main Aug 21, 2024
14 checks passed
@michalvavrik michalvavrik removed the triage/flaky-test Signal that flaky tests were detected during CI run label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalvavrik michalvavrik michalvavrik left review comments

Assignees

@fedinskiy fedinskiy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fedinskiy @michalvavrik