From 8676726556cb455d266c7e393acc5efa22b491b2 Mon Sep 17 00:00:00 2001 From: Greg Neiheisel <1036482+schnie@users.noreply.github.com> Date: Tue, 29 Oct 2024 21:00:45 -0400 Subject: [PATCH] Move new code to new files --- airflow/container_runtime.go | 113 ++++++++++++++++++++++++++++++ airflow/container_runtime_test.go | 75 ++++++++++++++++++++ airflow/docker.go | 71 ------------------- airflow/docker_test.go | 72 ------------------- 4 files changed, 188 insertions(+), 143 deletions(-) create mode 100644 airflow/container_runtime.go create mode 100644 airflow/container_runtime_test.go diff --git a/airflow/container_runtime.go b/airflow/container_runtime.go new file mode 100644 index 000000000..29db0b664 --- /dev/null +++ b/airflow/container_runtime.go @@ -0,0 +1,113 @@ +package airflow + +import ( + "os" + "path/filepath" + "strings" + "sync" + + "github.com/astronomer/astro-cli/pkg/util" + + "github.com/astronomer/astro-cli/config" + "github.com/astronomer/astro-cli/pkg/fileutil" + "github.com/pkg/errors" +) + +// FileChecker interface defines a method to check if a file exists. +// This is here mostly for testing purposes. This allows us to mock +// around actually checking for binaries on a live system as that +// would create inconsistencies across developer machines when +// working with the unit tests. +type FileChecker interface { + Exists(path string) bool +} + +// OSFileChecker is a concrete implementation of FileChecker. +type OSFileChecker struct{} + +// Exists checks if the file exists in the file system. +func (f OSFileChecker) Exists(path string) bool { + exists, _ := fileutil.Exists(path, nil) + return exists +} + +// FindBinary searches for the specified binary name in the provided PATH directories, +// 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) bool { + // Split the $PATH variable into it's individual path, + // 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 isWindows() { + binaryName += ".exe" + } + + var wg sync.WaitGroup + found := make(chan string, 1) + + // Search each individual path concurrently. + for _, dir := range paths { + wg.Add(1) + go func(dir string) { + defer wg.Done() + binaryPath := filepath.Join(dir, binaryName) + if exists := checker.Exists(binaryPath); exists { + found <- binaryPath + } + }(dir) + } + + // Wait for the concurrent checks to finish and close the channel. + wg.Wait() + close(found) + + // If we found the binary in one of the paths, return true. + if _, ok := <-found; ok { + return true + } + + // Otherwise the binary was not found, return false. + return false +} + +// GetContainerRuntimeBinary will return the manually configured container runtime, +// or search the $PATH for an acceptable runtime binary to use. This allows users +// to use alternative container runtimes without needing to explicitly configure it. +// Manual configuration should only be needed when both runtimes are installed and +// need to override to use one or the other and not use the auto-detection. +func GetContainerRuntimeBinary() (string, error) { + // Supported container runtime binaries + binaries := []string{dockerCmd, podmanCmd} + + // If the binary is manually configured to an acceptable runtime, return it directly. + // If a manual configuration exists, but it's not an appropriate runtime, we'll still + // search the $PATH for an acceptable one before completely bailing out. + configuredBinary := config.CFG.DockerCommand.GetString() + if util.Contains(binaries, configuredBinary) { + return configuredBinary, nil + } + + // Get the PATH environment variable. + pathEnv := os.Getenv("PATH") + for _, binary := range binaries { + if found := FindBinary(pathEnv, binary, OSFileChecker{}); found { + return binary, nil + } + } + + // If we made it here, no runtime was found, so we show a helpful error message + // and halt the command execution. + return "", errors.New("Failed to find a container runtime. " + + "See the Astro CLI prerequisites for more information. " + + "https://www.astronomer.io/docs/astro/cli/install-cli") +} + +// isWindows is a utility function to determine if the CLI host machine +// is running on Microsoft Windows OS. +func isWindows() bool { + return strings.Contains(strings.ToLower(os.Getenv("OS")), "windows") +} diff --git a/airflow/container_runtime_test.go b/airflow/container_runtime_test.go new file mode 100644 index 000000000..f9413cae5 --- /dev/null +++ b/airflow/container_runtime_test.go @@ -0,0 +1,75 @@ +package airflow + +// MockFileChecker is a mock implementation of FileChecker for tests. +type MockFileChecker struct { + existingFiles map[string]bool +} + +// Exists 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) Exists(path string) bool { + return m.existingFiles[path] +} + +// TestGetContainerRuntimeBinary runs a suite of tests against GetContainerRuntimeBinary, +// using the MockFileChecker defined above. +func (s *Suite) TestGetContainerRuntimeBinary() { + tests := []struct { + name string + pathEnv string + binary string + mockFiles map[string]bool + expected bool + }{ + { + name: "Find docker", + pathEnv: "/usr/local/bin:/usr/bin:/bin", + binary: "docker", + mockFiles: map[string]bool{ + "/usr/local/bin/docker": true, + }, + expected: true, + }, + { + name: "Find docker - doesn't exist", + pathEnv: "/usr/local/bin:/usr/bin:/bin", + binary: "docker", + mockFiles: map[string]bool{}, + expected: false, + }, + { + name: "Find podman", + pathEnv: "/usr/local/bin:/usr/bin:/bin", + binary: "podman", + mockFiles: map[string]bool{ + "/usr/local/bin/podman": true, + }, + expected: true, + }, + { + name: "Find podman - doesn't exist", + pathEnv: "/usr/local/bin:/usr/bin:/bin", + binary: "podman", + mockFiles: map[string]bool{}, + expected: false, + }, + { + name: "Binary not found", + pathEnv: "/usr/local/bin:/usr/bin:/bin", + binary: "notarealbinary", + mockFiles: map[string]bool{ + "/usr/local/bin/docker": true, + "/usr/local/bin/podman": true, + }, + expected: false, + }, + } + + for _, tt := range tests { + s.Run(tt.name, func() { + mockChecker := MockFileChecker{existingFiles: tt.mockFiles} + result := FindBinary(tt.pathEnv, tt.binary, mockChecker) + s.Equal(tt.expected, result) + }) + } +} diff --git a/airflow/docker.go b/airflow/docker.go index a9d82b6f9..ed88776f9 100644 --- a/airflow/docker.go +++ b/airflow/docker.go @@ -12,7 +12,6 @@ import ( "runtime" "sort" "strings" - "sync" "text/tabwriter" "time" @@ -1552,73 +1551,3 @@ func waitForDocker() error { } } } - -// FileChecker interface defines a method to check if a file exists. -type FileChecker interface { - Exists(path string) bool -} - -// OSFileChecker is a concrete implementation of FileChecker. -type OSFileChecker struct{} - -// Exists checks if the file exists in the file system. -func (f OSFileChecker) Exists(path string) bool { - exists, _ := fileutil.Exists(path, nil) - return exists -} - -// FindBinary searches for the specified binary in the provided PATH directories. -func FindBinary(pathEnv string, binaryName string, checker FileChecker) bool { - paths := strings.Split(pathEnv, string(os.PathListSeparator)) - - if isWindows() { - binaryName += ".exe" - } - - var wg sync.WaitGroup - found := make(chan string, 1) - - for _, dir := range paths { - wg.Add(1) - go func(dir string) { - defer wg.Done() - binaryPath := filepath.Join(dir, binaryName) - if exists := checker.Exists(binaryPath); exists { - found <- binaryPath - } - }(dir) - } - - wg.Wait() - close(found) - - if _, ok := <-found; ok { - return true - } - - return false -} - -func GetContainerRuntimeBinary() (string, error) { - // Supported container runtime binaries - binaries := []string{dockerCmd, podmanCmd} - - // If the binary is hard configured, return it - configuredBinary := config.CFG.DockerCommand.GetString() - if configuredBinary != "" { - return configuredBinary, nil - } - - // Get the PATH environment variable - pathEnv := os.Getenv("PATH") - for _, binary := range binaries { - if found := FindBinary(pathEnv, binary, OSFileChecker{}); found { - return binary, nil - } - } - return "", errors.New("Failed to find a container runtime. See the Astro CLI prerequisites for more information. https://www.astronomer.io/docs/astro/cli/install-cli") -} - -func isWindows() bool { - return strings.Contains(strings.ToLower(os.Getenv("OS")), "windows") -} diff --git a/airflow/docker_test.go b/airflow/docker_test.go index 19015f372..7ff1f306f 100644 --- a/airflow/docker_test.go +++ b/airflow/docker_test.go @@ -2075,75 +2075,3 @@ func (s *Suite) TestUpgradeDockerfile() { s.Contains(err.Error(), "no such file or directory") }) } - -// MockFileChecker is a mock implementation of FileChecker for tests. -type MockFileChecker struct { - existingFiles map[string]bool -} - -// Exists 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) Exists(path string) bool { - return m.existingFiles[path] -} - -func (s *Suite) TestGetContainerRuntimeBinary() { - tests := []struct { - name string - pathEnv string - binary string - mockFiles map[string]bool - expected bool - }{ - { - name: "Find docker", - pathEnv: "/usr/local/bin:/usr/bin:/bin", - binary: "docker", - mockFiles: map[string]bool{ - "/usr/local/bin/docker": true, - }, - expected: true, - }, - { - name: "Find docker - doesn't exist", - pathEnv: "/usr/local/bin:/usr/bin:/bin", - binary: "docker", - mockFiles: map[string]bool{}, - expected: false, - }, - { - name: "Find podman", - pathEnv: "/usr/local/bin:/usr/bin:/bin", - binary: "podman", - mockFiles: map[string]bool{ - "/usr/local/bin/podman": true, - }, - expected: true, - }, - { - name: "Find podman - doesn't exist", - pathEnv: "/usr/local/bin:/usr/bin:/bin", - binary: "podman", - mockFiles: map[string]bool{}, - expected: false, - }, - { - name: "Binary not found", - pathEnv: "/usr/local/bin:/usr/bin:/bin", - binary: "notarealbinary", - mockFiles: map[string]bool{ - "/usr/local/bin/docker": true, - "/usr/local/bin/podman": true, - }, - expected: false, - }, - } - - for _, tt := range tests { - s.Run(tt.name, func() { - mockChecker := MockFileChecker{existingFiles: tt.mockFiles} - result := FindBinary(tt.pathEnv, tt.binary, mockChecker) - s.Equal(tt.expected, result) - }) - } -}