Adds Podman Container Runtime #1750
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
schnie
force-pushed
the
podman-container-runtime
branch
2 times, most recently
from
120d170 to
8fda48e
Compare
schnie
force-pushed
the
podman-container-runtime
branch
from
8fda48e to
24eb943
Compare
schnie
commented
Dec 3, 2024
schnie
force-pushed
the
podman-container-runtime
branch
from
b2c1aa7 to
2cec150
Compare
schnie
force-pushed
the
podman-container-runtime
branch
2 times, most recently
from
9d8943c to
8c353ec
Compare
schnie
commented
Dec 6, 2024
Comment on lines
+35
to
+36
| Configure() error | ||
| ConfigureOrKill() error | ||
| Kill() error |
There was a problem hiding this comment.
Adding 3 new lifecycle methods here. The airflow hooks call these methods depending on the situation.
schnie
force-pushed
the
podman-container-runtime
branch
from
bea8e70 to
c86a6da
Compare
schnie
commented
Dec 6, 2024
schnie
commented
Dec 6, 2024
| // is running on Apple macOS. | ||
| func isMac() bool { | ||
| return runtime.GOOS == "darwin" | ||
| } |
schnie
force-pushed
the
podman-container-runtime
branch
3 times, most recently
from
20cc12b to
8e95465
Compare
schnie
force-pushed
the
podman-container-runtime
branch
from
349b073 to
0c585f5
Compare
schnie
requested review from
neel-astro,
jeremybeard and
kushalmalani
as code owners
schnie
commented
Dec 6, 2024
cmd/airflow_hooks.go
Outdated
Comment on lines
41
to
50
| // Check if the OS is Windows and create the plugins project directory if it doesn't exist. | ||
| // In Windows, the compose project will fail if the plugins directory doesn't exist, due | ||
| // to the volume mounts we specify. | ||
| osChecker := new(runtimes.DefaultOSChecker) | ||
| if osChecker.IsWindows() { | ||
| pluginsDir := filepath.Join(config.WorkingPath, "plugins") | ||
| if err := os.MkdirAll(pluginsDir, os.ModePerm); err != nil && !os.IsExist(err) { | ||
| return fmt.Errorf(failedToCreatePluginsDir, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
@pritt20 I attempted to simplify the nesting here a little. If you have your VM running would you mind testing this again on windows? And is the comment I added here accurate?
There was a problem hiding this comment.
Thanks @schnie !
I tested this scenario on windows machine and it is working as expected.
neel-astro
reviewed
Dec 9, 2024
Co-authored-by: Neel Dalsania <neel.dalsania@astronomer.io>
schnie
force-pushed
the
podman-container-runtime
branch
from
f0b5a09 to
92b929c
Compare
neel-astro
added a commit
that referenced
this pull request
Dec 13, 2024
Co-authored-by: Pritesh Arora <pritt@Priteshs-MacBook-Pro.local> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Neel Dalsania <neel.dalsania@astronomer.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds changes specific to podman container runtime and podman machine management. These changes will allow users to create astro local project without needing to manually manage podman machines and also removes some prerequisites required to setup astro cli therefore reducing friction during installation of CLI.
Notable code changes:
dockerandpodmancommands. We don't actually run the commands during the unit tests. We should test the concrete implementations with some e2e tests in the future though. "Runtimes" contain the high-level logic around managing the specific container runtimes. "Engines" are responsible for actually executing commands and returning the output.runtimespackage have been replaced withmockery-generated mocks for unit tests.🎟 Issue(s)
Related to https://github.com/astronomer/astro/issues/24344
🧪 Functional Testing
Tested Podman specific changes by spinning up local projects on Mac and Windows:
Mac:
Windows:
📸 Screenshots
📋 Checklist
make testbefore taking out of draftmake lintbefore taking out of draft