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

Removes dead code for conflict-test #1797

Open
wants to merge 8 commits 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
15 changes: 0 additions & 15 deletions airflow/airflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ var (
//go:embed include/dockerfile
Dockerfile string

//go:embed include/test-conflicts.dockerfile
testConflictsDockerfile string

//go:embed include/dockerignore
Dockerignore string

Expand Down Expand Up @@ -139,18 +136,6 @@ func Init(path, airflowImageName, airflowImageTag, template string) error {
return nil
}

func initConflictTest(path, airflowImageName, airflowImageTag string) error {
// Map of files to create
files := map[string]string{
"conflict-check.Dockerfile": fmt.Sprintf(testConflictsDockerfile, airflowImageName, airflowImageTag),
}
// Initialize files
if err := initFiles(path, files); err != nil {
return errors.Wrap(err, "failed to create upgrade check files")
}
return nil
}

// repositoryName creates an airflow repository name
func repositoryName(name string) string {
return fmt.Sprintf("%s/%s", name, componentName)
Expand Down
18 changes: 0 additions & 18 deletions airflow/airflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,21 +117,3 @@ func (s *Suite) TestTemplateInitFail() {
err = Init(tmpDir, "astro-runtime", "test", "etl")
s.EqualError(err, "failed to set up template-based astro project: error extracting files")
}

func (s *Suite) TestInitConflictTest() {
tmpDir, err := os.MkdirTemp("", "temp")
s.Require().NoError(err)
defer os.RemoveAll(tmpDir)

err = initConflictTest(tmpDir, "astro-runtime", "test")
s.NoError(err)

expectedFiles := []string{
"conflict-check.Dockerfile",
}
for _, file := range expectedFiles {
exist, err := fileutil.Exists(filepath.Join(tmpDir, file), nil)
s.NoError(err)
s.True(exist)
}
}
3 changes: 1 addition & 2 deletions airflow/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type ContainerHandler interface {
ComposeExport(settingsFile, composeFile string) error
Pytest(pytestFile, customImageName, deployImageName, pytestArgsString, buildSecretString string) (string, error)
Parse(customImageName, deployImageName, buildSecretString string) error
UpgradeTest(runtimeVersion, deploymentID, newImageName, customImageName, buildSecretString string, dependencyTest, versionTest, dagTest bool, astroPlatformCore astroplatformcore.ClientWithResponsesInterface) error
UpgradeTest(runtimeVersion, deploymentID, newImageName, customImageName, buildSecretString string, versionTest, dagTest bool, astroPlatformCore astroplatformcore.ClientWithResponsesInterface) error
}

// RegistryHandler defines methods require to handle all operations with registry
Expand All @@ -54,7 +54,6 @@ type ImageHandler interface {
TagLocalImage(localImage string) error
Run(dagID, envFile, settingsFile, containerName, dagFile, executionDate string, taskLogs bool) error
Pytest(pytestFile, airflowHome, envFile, testHomeDirectory string, pytestArgs []string, htmlReport bool, config types.ImageBuildConfig) (string, error)
ConflictTest(workingDirectory, testHomeDirectory string, buildConfig types.ImageBuildConfig) (string, error)
CreatePipFreeze(altImageName, pipFreezeFile string) error
GetImageRepoSHA(registry string) (string, error)
}
Expand Down
37 changes: 1 addition & 36 deletions airflow/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ func (d *DockerCompose) Pytest(pytestFile, customImageName, deployImageName, pyt
return exitCode, errors.New("something went wrong while Pytesting your DAGs")
}

func (d *DockerCompose) UpgradeTest(newAirflowVersion, deploymentID, newImageName, customImage, buildSecretString string, conflictTest, versionTest, dagTest bool, astroPlatformCore astroplatformcore.CoreClient) error {
func (d *DockerCompose) UpgradeTest(newAirflowVersion, deploymentID, newImageName, customImage, buildSecretString string, versionTest, dagTest bool, astroPlatformCore astroplatformcore.CoreClient) error {
// figure out which tests to run
if !versionTest && !dagTest {
versionTest = true
Expand Down Expand Up @@ -598,13 +598,6 @@ func (d *DockerCompose) UpgradeTest(newAirflowVersion, deploymentID, newImageNam
}
newDockerFile := destFolder + "/Dockerfile"

// check for dependency conflicts
if conflictTest {
err = d.conflictTest(testHomeDirectory, newImageName, newAirflowVersion)
if err != nil {
return err
}
}
Comment on lines -602 to -607
Copy link
Member Author

Choose a reason for hiding this comment

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

Nowhere in our code base do we seem to set conflictTest to true. There should be no way for this branch to ever run. This conflictTest used to exist as a flag on the CLI commands, but seems to have been removed.

if versionTest {
err := d.versionTest(testHomeDirectory, currentAirflowVersion, deploymentImage, newDockerFile, newAirflowVersion, customImage, buildSecretString)
if err != nil {
Expand All @@ -620,9 +613,6 @@ func (d *DockerCompose) UpgradeTest(newAirflowVersion, deploymentID, newImageNam
}
fmt.Println("\nTest Summary:")
fmt.Printf("\tUpgrade Test Results Directory: %s\n", testHomeDirectory)
if conflictTest {
fmt.Printf("\tDependency Conflict Test Results file: %s\n", "conflict-test-results.txt")
}
if versionTest {
fmt.Printf("\tDependency Version Comparison Results file: %s\n", "dependency_compare.txt")
}
Expand Down Expand Up @@ -656,31 +646,6 @@ func (d *DockerCompose) pullImageFromDeployment(deploymentID string, platformCor
return nil
}

func (d *DockerCompose) conflictTest(testHomeDirectory, newImageName, newAirflowVersion string) error {
fmt.Println("\nChecking your 'requirements.txt' for dependency conflicts with the new version of Airflow")
fmt.Println("\nThis may take a few minutes...")

// create files needed for conflict test
err := initConflictTest(config.WorkingPath, newImageName, newAirflowVersion)
defer os.Remove("conflict-check.Dockerfile")
if err != nil {
return err
}

exitCode, conflictErr := d.imageHandler.ConflictTest(d.airflowHome, testHomeDirectory, airflowTypes.ImageBuildConfig{Path: d.airflowHome})
if conflictErr != nil {
return conflictErr
}
if strings.Contains(exitCode, "0") || exitCode == "" { // if the error code is 0 the pytests passed
fmt.Println("There were no dependency conflicts found")
} else {
fmt.Println("\nSomething went wrong while compiling your dependencies check the logs above for conflicts")
fmt.Println("If there are conflicts remove them from your 'requirments.txt' and rerun this test\nYou will see the best candidate in the 'conflict-test-results.txt' file")
return err
}
return nil
}

func (d *DockerCompose) versionTest(testHomeDirectory, currentAirflowVersion, deploymentImage, newDockerFile, newAirflowVersion, customImage, buildSecretString string) error {
fmt.Println("\nComparing dependency versions between current and upgraded environment")
// pip freeze old Airflow image
Expand Down
78 changes: 0 additions & 78 deletions airflow/docker_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"io"
"os"
"os/exec"
"regexp"
"strings"

"github.com/astronomer/astro-cli/airflow/runtimes"
Expand All @@ -30,7 +29,6 @@ import (
)

const (
EchoCmd = "echo"
Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated, but also unused.

pushingImagePrompt = "Pushing image to Astronomer registry"
astroRunContainer = "astro-run"
pullingImagePrompt = "Pulling image from Astronomer registry"
Expand Down Expand Up @@ -264,82 +262,6 @@ func (d *DockerImage) Pytest(pytestFile, airflowHome, envFile, testHomeDirectory
return outb.String(), err
}

func (d *DockerImage) ConflictTest(workingDirectory, testHomeDirectory string, buildConfig airflowTypes.ImageBuildConfig) (string, error) {
containerRuntime, err := runtimes.GetContainerRuntimeBinary()
if err != nil {
return "", err
}
// delete container
err = cmdExec(containerRuntime, nil, nil, "rm", "astro-temp-container")
if err != nil {
logger.Debug(err)
}
// Change to location of Dockerfile
err = os.Chdir(buildConfig.Path)
if err != nil {
return "", err
}
args := []string{
"build",
"-t",
"conflict-check:latest",
"-f",
"conflict-check.Dockerfile",
".",
}

// Create a buffer to capture the command output
var stdout, stderr bytes.Buffer
multiStdout := io.MultiWriter(&stdout, os.Stdout)
multiStderr := io.MultiWriter(&stderr, os.Stdout)

// Start the command execution
err = cmdExec(containerRuntime, multiStdout, multiStderr, args...)
if err != nil {
return "", err
}
// Get the exit code
exitCode := ""
if _, ok := err.(*exec.ExitError); ok {
// The command exited with a non-zero status
exitCode = parseExitCode(stderr.String())
} else if err != nil {
// An error occurred while running the command
return "", err
}
// Run a temporary container to copy the file from the image
err = cmdExec(containerRuntime, nil, nil, "create", "--name", "astro-temp-container", "conflict-check:latest")
if err != nil {
return exitCode, err
}
// Copy the result.txt file from the container to the destination folder
err1 := cmdExec(containerRuntime, nil, nil, "cp", "astro-temp-container:/usr/local/airflow/conflict-test-results.txt", "./"+testHomeDirectory)
if err1 != nil {
// Remove the temporary container
err = cmdExec(containerRuntime, nil, nil, "rm", "astro-temp-container")
if err != nil {
return exitCode, err
}
return exitCode, err1
}

// Remove the temporary container
err = cmdExec(containerRuntime, nil, nil, "rm", "astro-temp-container")
if err != nil {
return exitCode, err
}
return exitCode, nil
}

func parseExitCode(logs string) string {
re := regexp.MustCompile(`exit code: (\d+)`)
match := re.FindStringSubmatch(logs)
if len(match) > 1 {
return match[1]
}
return ""
}

func (d *DockerImage) CreatePipFreeze(altImageName, pipFreezeFile string) error {
containerRuntime, err := runtimes.GetContainerRuntimeBinary()
if err != nil {
Expand Down
97 changes: 0 additions & 97 deletions airflow/docker_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,103 +223,6 @@ func (s *Suite) TestDockerImagePytest() {
})
}

func (s *Suite) TestDockerImageConflictTest() {
handler := DockerImage{
imageName: "testing",
}

cwd, err := os.Getwd()
s.NoError(err)

dockerIgnoreFile := cwd + "/.dockerignore"
fileutil.WriteStringToFile(dockerIgnoreFile, "")
defer afero.NewOsFs().Remove(dockerIgnoreFile)

options := airflowTypes.ImageBuildConfig{
Path: cwd,
TargetPlatforms: []string{"linux/amd64"},
NoCache: false,
}

s.Run("conflict test success", func() {
cmdExec = func(cmd string, stdout, stderr io.Writer, args ...string) error {
return nil
}
_, err = handler.ConflictTest("", "", options)
s.NoError(err)
})

s.Run("conflict test create error", func() {
options = airflowTypes.ImageBuildConfig{
Path: cwd,
TargetPlatforms: []string{"linux/amd64"},
NoCache: false,
}

cmdExec = func(cmd string, stdout, stderr io.Writer, args ...string) error {
return errMock
}
_, err = handler.ConflictTest("", "", options)
s.Error(err)
})

s.Run("conflict test cp error", func() {
options = airflowTypes.ImageBuildConfig{
Path: cwd,
TargetPlatforms: []string{"linux/amd64"},
NoCache: false,
}

cmdExec = func(cmd string, stdout, stderr io.Writer, args ...string) error {
switch {
case args[0] == "cp":
return errMock
default:
return nil
}
}
_, err = handler.ConflictTest("", "", options)
s.Error(err)
})

s.Run("conflict test rm error", func() {
options = airflowTypes.ImageBuildConfig{
Path: cwd,
TargetPlatforms: []string{"linux/amd64"},
NoCache: false,
}

cmdExec = func(cmd string, stdout, stderr io.Writer, args ...string) error {
switch {
case args[0] == "rm":
return errMock
default:
return nil
}
}
_, err = handler.ConflictTest("", "", options)
s.Error(err)
})
s.Run("unable to read file error", func() {
options := airflowTypes.ImageBuildConfig{
Path: "incorrect-path",
TargetPlatforms: []string{"linux/amd64"},
NoCache: false,
}

_, err = handler.ConflictTest("", "", options)
s.Error(err)
})
}

func (s *Suite) TestParseExitCode() {
output := "exit code: 1"
s.Run("success", func() {
_ = parseExitCode(output)
_ = parseExitCode("")
})
}

func (s *Suite) TestDockerCreatePipFreeze() {
handler := DockerImage{
imageName: "testing",
Expand Down
Loading