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

Adds Orbstack Container Runtime Engine #1761

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .mockery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ packages:
outpkg: mocks
interfaces:
OSChecker:
EnvChecker:
FileChecker:
HostInspector:
ContainerRuntime:
PodmanEngine:
DockerEngine:
17 changes: 11 additions & 6 deletions airflow/runtimes/container_runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ var spinnerCharSet = spinner.CharSets[14]
const (
docker = "docker"
podman = "podman"
orbstack = "orbstack"
orbctl = "orbctl"
containerRuntimeNotFoundErrMsg = "Failed to find a container runtime. " +
"See the Astro CLI prerequisites for more information. " +
"https://www.astronomer.io/docs/astro/cli/install-cli"
Expand Down Expand Up @@ -48,9 +50,9 @@ func GetContainerRuntime() (ContainerRuntime, error) {
// Return the appropriate container runtime based on the binary discovered.
switch containerRuntime {
case docker:
return CreateDockerRuntime(new(dockerEngine), new(osChecker)), nil
return CreateDockerRuntimeWithDefaults(), nil
case podman:
return CreatePodmanRuntime(new(podmanEngine), new(osChecker)), nil
return CreatePodmanRuntimeWithDefaults(), nil
Comment on lines -51 to +55
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of specifying these concrete implementations out here, we've added new constructor functions to construct with the default concrete implementations. This lets us encapsulate the new logic to determine the specific Engine to plug in.

default:
return nil, errors.New(containerRuntimeNotFoundErrMsg)
}
Expand All @@ -60,14 +62,14 @@ func GetContainerRuntime() (ContainerRuntime, error) {
// using the provided FileChecker. It searches each specific path within the systems
// $PATH environment variable for the binary concurrently and returns a boolean result
// indicating if the binary was found or not.
func FindBinary(pathEnv, binaryName string, checker FileChecker, osChecker OSChecker) bool {
func FindBinary(pathEnv, binaryName string, inspector HostInterrogator) bool {
// Split the $PATH variable into it's individual paths,
// using the OS specific path separator character.
paths := strings.Split(pathEnv, string(os.PathListSeparator))

// Although programs can be called without the .exe extension,
// we need to append it here when searching the file system.
if osChecker.IsWindows() {
if inspector.IsWindows() {
binaryName += ".exe"
}

Expand All @@ -82,7 +84,7 @@ func FindBinary(pathEnv, binaryName string, checker FileChecker, osChecker OSChe
go func(dir string) {
defer wg.Done()
binaryPath := filepath.Join(dir, binaryName)
if exists := checker.Exists(binaryPath); exists {
if exists := inspector.FileExists(binaryPath); exists {
select {
// If the channel is open, send the path in, indicating a found binary.
case found <- binaryPath:
Expand Down Expand Up @@ -128,10 +130,13 @@ var GetContainerRuntimeBinary = func() (string, error) {
return configuredBinary, nil
}

// Create a host interrogator with default implementations.
interrogator := CreateHostInspectorWithDefaults()

// Get the $PATH environment variable.
pathEnv := os.Getenv("PATH")
for _, binary := range binaries {
if found := FindBinary(pathEnv, binary, CreateFileChecker(), CreateOSChecker()); found {
if found := FindBinary(pathEnv, binary, interrogator); found {
return binary, nil
}
}
Expand Down
16 changes: 14 additions & 2 deletions airflow/runtimes/container_runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ import (
"github.com/stretchr/testify/suite"
)

// mockFileChecker is a mock implementation of FileChecker for tests in this file.
type mockFileChecker struct {
ExistingFiles map[string]bool
}

// FileExists is just a mock for os.Stat(). In our test implementation, we just check
// if the file exists in the list of mocked files for a given test.
func (m mockFileChecker) FileExists(path string) bool {
return m.ExistingFiles[path]
}

Comment on lines +14 to +24
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this special mock here to live with its specific tests. I've also configured mockery to now generate a more standard mock under mocks for the other tests.

type ContainerRuntimeSuite struct {
suite.Suite
}
Expand Down Expand Up @@ -142,8 +153,9 @@ func (s *ContainerRuntimeSuite) TestGetContainerRuntimeBinary() {

for _, tt := range tests {
s.Run(tt.name, func() {
mockChecker := mocks.FileChecker{ExistingFiles: tt.mockFiles}
result := FindBinary(tt.pathEnv, tt.binary, mockChecker, new(osChecker))
mockChecker := mockFileChecker{ExistingFiles: tt.mockFiles}
inspector := CreateHostInspector(CreateOSChecker(), mockChecker, CreateEnvChecker())
result := FindBinary(tt.pathEnv, tt.binary, inspector)
s.Equal(tt.expected, result)
})
}
Expand Down
43 changes: 43 additions & 0 deletions airflow/runtimes/docker_runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,53 @@ type DockerRuntime struct {
OSChecker OSChecker
}

// CreateDockerRuntime creates a new DockerRuntime with the provided DockerEngine and OSChecker.
func CreateDockerRuntime(engine DockerEngine, osChecker OSChecker) DockerRuntime {
return DockerRuntime{Engine: engine, OSChecker: osChecker}
}

// CreateDockerRuntimeWithDefaults creates a new DockerRuntime with the default DockerEngine and OSChecker.
func CreateDockerRuntimeWithDefaults() DockerRuntime {
return DockerRuntime{Engine: GetDockerEngine(CreateHostInspectorWithDefaults()), OSChecker: CreateOSChecker()}
}

// GetDockerEngine returns the appropriate DockerEngine based on the binary discovered.
func GetDockerEngine(interrogator HostInterrogator) DockerEngine {
engine, err := GetDockerEngineBinary(interrogator)
if err != nil {
return new(dockerEngine)
}

// Return the appropriate container runtime based on the binary discovered.
switch engine {
case orbctl:
return new(orbstackEngine)
default:
return new(dockerEngine)
}
}

// GetDockerEngineBinary returns the first Docker binary found in the $PATH environment variable.
// This is used to determine which Engine to use. Eg: orbctl or docker.
func GetDockerEngineBinary(inspector HostInterrogator) (string, error) {
// If the orbctl binary is found, it means orbstack is installed. The docker binary would also exist
// in this case, but we use this as a signal to use the orbstack engine. We'll still end up
// shelling our docker commands out using the docker binary.
binaries := []string{orbctl, docker}

// Get the $PATH environment variable.
pathEnv := inspector.GetEnvVar("PATH")
for _, binary := range binaries {
if found := FindBinary(pathEnv, binary, inspector); found {
return binary, nil
}
}

// If no binary is found, we just return our default docker binary.
// The higher level check for the container runtime binary will handle the user-facing error.
return docker, nil
}

// Initialize initializes the Docker runtime.
// We only attempt to initialize Docker on Mac today.
func (rt DockerRuntime) Initialize() error {
Expand Down
129 changes: 127 additions & 2 deletions airflow/runtimes/docker_runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package runtimes

import (
"fmt"
"strings"
"testing"

"github.com/astronomer/astro-cli/airflow/runtimes/mocks"
Expand All @@ -11,8 +12,11 @@ import (
)

var (
mockDockerEngine *mocks.DockerEngine
mockDockerOSChecker *mocks.OSChecker
mockDockerEngine *mocks.DockerEngine
mockDockerOSChecker *mocks.OSChecker
mockDockerFileChecker *mocks.FileChecker
mockDockerEnvChecker *mocks.EnvChecker
mockDockerHostInspector HostInterrogator
)

type DockerRuntimeSuite struct {
Expand All @@ -27,6 +31,127 @@ func (s *DockerRuntimeSuite) SetupTest() {
// Reset some variables to defaults.
mockDockerEngine = new(mocks.DockerEngine)
mockDockerOSChecker = new(mocks.OSChecker)
mockDockerFileChecker = new(mocks.FileChecker)
mockDockerEnvChecker = new(mocks.EnvChecker)
mockDockerHostInspector = CreateHostInspector(mockDockerOSChecker, mockDockerFileChecker, mockDockerEnvChecker)
}

func (s *DockerRuntimeSuite) TestGetDockerEngineReturnsOrbstackEngine() {
s.Run("Get Docker Engine returns Orbstack Engine for orbctl", func() {
paths := []string{"/usr/local/bin", "/usr/bin", "/bin"}
mockDockerEnvChecker.On("GetEnvVar", "PATH").Return(strings.Join(paths, ":"), nil)
mockDockerOSChecker.On("IsWindows").Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+orbctl).Return(true)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+orbctl).Return(false)

engine := GetDockerEngine(mockDockerHostInspector)
assert.IsType(s.T(), new(orbstackEngine), engine, "Expected orbstack engine to be returned")
mockDockerEnvChecker.AssertExpectations(s.T())
mockDockerOSChecker.AssertExpectations(s.T())
mockDockerFileChecker.AssertExpectations(s.T())
})
}

func (s *DockerRuntimeSuite) TestGetDockerEngineReturnsDockerEngine() {
s.Run("Get Docker Engine returns Docker Engine for docker", func() {
paths := []string{"/usr/local/bin", "/usr/bin", "/bin"}
mockDockerEnvChecker.On("GetEnvVar", "PATH").Return(strings.Join(paths, ":"), nil)
mockDockerOSChecker.On("IsWindows").Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+docker).Return(true)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+docker).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+docker).Return(false)

engine := GetDockerEngine(mockDockerHostInspector)
assert.IsType(s.T(), new(dockerEngine), engine, "Expected docker engine to be returned")
mockDockerEnvChecker.AssertExpectations(s.T())
mockDockerOSChecker.AssertExpectations(s.T())
mockDockerFileChecker.AssertExpectations(s.T())
})
}

func (s *DockerRuntimeSuite) TestGetDockerEngineFallbackToDocker() {
s.Run("Get Docker Engine Binary finds no binaries in $PATH", func() {
paths := []string{"/usr/local/bin", "/usr/bin", "/bin"}
mockDockerEnvChecker.On("GetEnvVar", "PATH").Return(strings.Join(paths, ":"), nil)
mockDockerOSChecker.On("IsWindows").Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+docker).Return(false)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+docker).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+docker).Return(false)

binary, err := GetDockerEngineBinary(mockDockerHostInspector)
assert.Nil(s.T(), err, "Expected no error when docker binary is found")
assert.Equal(s.T(), docker, binary, "Expected docker binary to returned by default")
mockDockerEnvChecker.AssertExpectations(s.T())
mockDockerOSChecker.AssertExpectations(s.T())
mockDockerFileChecker.AssertExpectations(s.T())
})
}

func (s *DockerRuntimeSuite) TestGetDockerEngineFindsOrbstack() {
s.Run("Get Docker Engine Binary finds Orbstack in $PATH", func() {
paths := []string{"/usr/local/bin", "/usr/bin", "/bin"}
mockDockerEnvChecker.On("GetEnvVar", "PATH").Return(strings.Join(paths, ":"), nil)
mockDockerOSChecker.On("IsWindows").Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+orbctl).Return(true)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+orbctl).Return(false)

binary, err := GetDockerEngineBinary(mockDockerHostInspector)
assert.Nil(s.T(), err, "Expected no error when orbctl binary is found")
assert.Equal(s.T(), orbctl, binary, "Expected orbctl binary to be found")
mockDockerEnvChecker.AssertExpectations(s.T())
mockDockerOSChecker.AssertExpectations(s.T())
mockDockerFileChecker.AssertExpectations(s.T())
})
}

func (s *DockerRuntimeSuite) TestGetDockerEngineBinaryFindsDocker() {
s.Run("Get Docker Engine Binary finds Docker in $PATH", func() {
paths := []string{"/usr/local/bin", "/usr/bin", "/bin"}
mockDockerEnvChecker.On("GetEnvVar", "PATH").Return(strings.Join(paths, ":"), nil)
mockDockerOSChecker.On("IsWindows").Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+docker).Return(true)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+docker).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+docker).Return(false)

binary, err := GetDockerEngineBinary(mockDockerHostInspector)
assert.Nil(s.T(), err, "Expected no error when docker binary is found")
assert.Equal(s.T(), docker, binary, "Expected docker binary to be found")
mockDockerEnvChecker.AssertExpectations(s.T())
mockDockerOSChecker.AssertExpectations(s.T())
mockDockerFileChecker.AssertExpectations(s.T())
})
}

func (s *DockerRuntimeSuite) TestGetDockerEngineBinaryFindsNoBinaryFallbackToDocker() {
s.Run("Get Docker Engine Binary finds Docker in $PATH", func() {
paths := []string{"/usr/local/bin", "/usr/bin", "/bin"}
mockDockerEnvChecker.On("GetEnvVar", "PATH").Return(strings.Join(paths, ":"), nil)
mockDockerOSChecker.On("IsWindows").Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+orbctl).Return(false)
mockDockerFileChecker.On("FileExists", paths[0]+"/"+docker).Return(false)
mockDockerFileChecker.On("FileExists", paths[1]+"/"+docker).Return(false)
mockDockerFileChecker.On("FileExists", paths[2]+"/"+docker).Return(false)

binary, err := GetDockerEngineBinary(mockDockerHostInspector)
assert.Nil(s.T(), err, "Expected no error when docker binary is found")
assert.Equal(s.T(), docker, binary, "Expected docker binary to be found")
mockDockerEnvChecker.AssertExpectations(s.T())
mockDockerOSChecker.AssertExpectations(s.T())
mockDockerFileChecker.AssertExpectations(s.T())
})
}

func (s *DockerRuntimeSuite) TestStartDocker() {
Expand Down
20 changes: 20 additions & 0 deletions airflow/runtimes/env_checker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package runtimes

import (
"os"
)

type EnvChecker interface {
GetEnvVar(string) string
}

type envChecker struct{}

func CreateEnvChecker() EnvChecker {
return new(envChecker)
}

// GetEnvVar returns the value of the specified environment variable.
func (o envChecker) GetEnvVar(string) string {
return os.Getenv("PATH")
}
6 changes: 3 additions & 3 deletions airflow/runtimes/file_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import "github.com/astronomer/astro-cli/pkg/fileutil"
// would create inconsistencies across developer machines when
// working with the unit tests.
type FileChecker interface {
Exists(path string) bool
FileExists(path string) bool
}

// fileChecker is a concrete implementation of FileChecker.
Expand All @@ -18,8 +18,8 @@ func CreateFileChecker() FileChecker {
return new(fileChecker)
}

// Exists checks if the file exists in the file system.
func (f fileChecker) Exists(path string) bool {
// FileExists checks if the file exists in the file system.
func (f fileChecker) FileExists(path string) bool {
exists, _ := fileutil.Exists(path, nil)
return exists
}
30 changes: 30 additions & 0 deletions airflow/runtimes/host_interrogator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package runtimes

type HostInterrogator interface {
IsMac() bool
IsWindows() bool
FileExists(string) bool
GetEnvVar(string) string
}

type hostInterrogator struct {
OSChecker
FileChecker
EnvChecker
}

func CreateHostInspector(osChecker OSChecker, fileChecker FileChecker, envChecker EnvChecker) HostInterrogator {
return hostInterrogator{
osChecker,
fileChecker,
envChecker,
}
}

func CreateHostInspectorWithDefaults() HostInterrogator {
return hostInterrogator{
CreateOSChecker(),
CreateFileChecker(),
CreateEnvChecker(),
}
}
Comment on lines +1 to +30
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of passing all 3 of these implementations around, I've consolidated them into a HostInterrogator object that can answer all the questions about the host. In our tests we mock out anything that interacts with the underlying host.

Loading