Skip to content

Commit

Permalink
Fix Seg fault error when no deployments exists on deploy (#1302)
Browse files Browse the repository at this point in the history
  • Loading branch information
kushalmalani authored Jul 10, 2023
1 parent 503e0c2 commit b76e610
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 29 deletions.
9 changes: 5 additions & 4 deletions cloud/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/astronomer/astro-cli/airflow/types"
airflowversions "github.com/astronomer/astro-cli/airflow_versions"
astro "github.com/astronomer/astro-cli/astro-client"
astrocore "github.com/astronomer/astro-cli/astro-client-core"
"github.com/astronomer/astro-cli/cloud/deployment"
"github.com/astronomer/astro-cli/config"
"github.com/astronomer/astro-cli/docker"
Expand Down Expand Up @@ -196,7 +197,7 @@ func deployDags(path, dagsPath, runtimeID string, client astro.Client) (string,
}

// Deploy pushes a new docker image
func Deploy(deployInput InputDeploy, client astro.Client) error { //nolint
func Deploy(deployInput InputDeploy, client astro.Client, coreClient astrocore.CoreClient) error { //nolint
// Get cloud domain
c, err := config.GetCurrentContext()
if err != nil {
Expand All @@ -217,7 +218,7 @@ func Deploy(deployInput InputDeploy, client astro.Client) error { //nolint

dagFiles := fileutil.GetFilesWithSpecificExtension(dagsPath, ".py")

deployInfo, err := getDeploymentInfo(deployInput.RuntimeID, deployInput.WsID, deployInput.DeploymentName, deployInput.Prompt, domain, client)
deployInfo, err := getDeploymentInfo(deployInput.RuntimeID, deployInput.WsID, deployInput.DeploymentName, deployInput.Prompt, domain, client, coreClient)
if err != nil {
return err
}
Expand Down Expand Up @@ -351,7 +352,7 @@ func Deploy(deployInput InputDeploy, client astro.Client) error { //nolint
return nil
}

func getDeploymentInfo(deploymentID, wsID, deploymentName string, prompt bool, cloudDomain string, client astro.Client) (deploymentInfo, error) {
func getDeploymentInfo(deploymentID, wsID, deploymentName string, prompt bool, cloudDomain string, client astro.Client, coreClient astrocore.CoreClient) (deploymentInfo, error) {
// Use config deployment if provided
if deploymentID == "" {
deploymentID = config.CFG.ProjectDeployment.GetProjectString()
Expand All @@ -366,7 +367,7 @@ func getDeploymentInfo(deploymentID, wsID, deploymentName string, prompt bool, c

// check if deploymentID or if force prompt was requested was given by user
if deploymentID == "" || prompt {
currentDeployment, err := deployment.GetDeployment(wsID, deploymentID, deploymentName, false, client, nil)
currentDeployment, err := deployment.GetDeployment(wsID, deploymentID, deploymentName, false, client, coreClient)
if err != nil {
return deploymentInfo{}, err
}
Expand Down
48 changes: 25 additions & 23 deletions cloud/deploy/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/astronomer/astro-cli/airflow/mocks"
airflowversions "github.com/astronomer/astro-cli/airflow_versions"
"github.com/astronomer/astro-cli/astro-client"
astrocore_mocks "github.com/astronomer/astro-cli/astro-client-core/mocks"
astro_mocks "github.com/astronomer/astro-cli/astro-client/mocks"
"github.com/astronomer/astro-cli/config"
"github.com/astronomer/astro-cli/pkg/fileutil"
Expand All @@ -28,6 +29,7 @@ var (
initiatedDagDeploymentID = "test-dag-deployment-id"
runtimeID = "test-id"
dagURL = "http://fake-url.windows.core.net"
mockCoreClient = new(astrocore_mocks.ClientWithResponsesInterface)
)

func TestDeployWithoutDagsDeploySuccess(t *testing.T) {
Expand Down Expand Up @@ -102,27 +104,27 @@ func TestDeployWithoutDagsDeploySuccess(t *testing.T) {
os.Stdin = r

defer testUtil.MockUserInput(t, "y")()
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

defer testUtil.MockUserInput(t, "y")()
deployInput.RuntimeID = "test-id"
deployInput.Pytest = "pytest"
deployInput.Prompt = false
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

// test custom image
defer testUtil.MockUserInput(t, "y")()
deployInput.ImageName = "custom-image"
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

config.CFG.ProjectDeployment.SetProjectString("test-id")
// test both deploymentID and name used
defer testUtil.MockUserInput(t, "y")()
deployInput.DeploymentName = "test-name"
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

mockClient.AssertExpectations(t)
Expand Down Expand Up @@ -221,37 +223,37 @@ func TestDeployWithDagsDeploySuccess(t *testing.T) {
os.Stdin = r

defer testUtil.MockUserInput(t, "y")()
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

defer testUtil.MockUserInput(t, "y")()
deployInput.RuntimeID = "test-id"
deployInput.Pytest = "pytest"
deployInput.Prompt = false
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

config.CFG.ProjectDeployment.SetProjectString("test-id")
// test both deploymentID and name used
defer testUtil.MockUserInput(t, "y")()
deployInput.DeploymentName = "test-name"
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

defer testUtil.MockUserInput(t, "y")()
deployInput.Pytest = "parse"
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

defer testUtil.MockUserInput(t, "y")()
deployInput.Pytest = parseAndPytest
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

// test custom image with dag deploy enabled
defer testUtil.MockUserInput(t, "y")()
deployInput.ImageName = "custom-image"
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

os.Mkdir("./testfiles1/", os.ModePerm)
Expand All @@ -271,7 +273,7 @@ func TestDeployWithDagsDeploySuccess(t *testing.T) {
Dags: false,
}
defer testUtil.MockUserInput(t, "y")()
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

defer os.RemoveAll("./testfiles1/")
Expand Down Expand Up @@ -343,7 +345,7 @@ func TestDagsDeploySuccess(t *testing.T) {
mockClient.On("ReportDagDeploymentStatus", reportDagDeploymentStatusInput).Return(astro.DagDeploymentStatus{}, nil).Times(4)

defer testUtil.MockUserInput(t, "y")()
err := Deploy(deployInput, mockClient)
err := Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

// Test pytest with dags deploy
Expand All @@ -365,17 +367,17 @@ func TestDagsDeploySuccess(t *testing.T) {

defer testUtil.MockUserInput(t, "y")()
deployInput.Pytest = "parse"
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

defer testUtil.MockUserInput(t, "y")()
deployInput.Pytest = allTests
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

defer testUtil.MockUserInput(t, "y")()
deployInput.Pytest = parseAndPytest
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

defer os.RemoveAll("./testfiles/dags/")
Expand Down Expand Up @@ -432,7 +434,7 @@ func TestNoDagsDeploy(t *testing.T) {
Prompt: true,
Dags: true,
}
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

mockClient.AssertExpectations(t)
Expand Down Expand Up @@ -483,7 +485,7 @@ func TestDagsDeployFailed(t *testing.T) {
mockClient.On("GetDeploymentConfig").Return(astro.DeploymentConfig{RuntimeReleases: []astro.RuntimeRelease{{Version: "4.2.5"}}}, nil).Times(2)

defer testUtil.MockUserInput(t, "y")()
err := Deploy(deployInput, mockClient)
err := Deploy(deployInput, mockClient, mockCoreClient)
assert.Equal(t, err.Error(), "DAG-only deploys are not enabled for this Deployment. Run 'astro deployment update test-id --dag-deploy enable' to enable DAG-only deploys.")

mockImageHandler := new(mocks.ImageHandler)
Expand All @@ -502,12 +504,12 @@ func TestDagsDeployFailed(t *testing.T) {

defer testUtil.MockUserInput(t, "y")()
deployInput.Pytest = "parse"
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.Error(t, err)

defer testUtil.MockUserInput(t, "y")()
deployInput.Pytest = allTests
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.Error(t, err)

mockClient.AssertExpectations(t)
Expand Down Expand Up @@ -538,7 +540,7 @@ func TestDeployFailure(t *testing.T) {
}

defer testUtil.MockUserInput(t, "y")()
err = Deploy(deployInput, nil)
err = Deploy(deployInput, nil, mockCoreClient)
assert.EqualError(t, err, "no context set, have you authenticated to Astro or Astronomer Software? Run astro login and try again")

// airflow parse failure
Expand Down Expand Up @@ -589,20 +591,20 @@ func TestDeployFailure(t *testing.T) {

defer testUtil.MockUserInput(t, "y")()
deployInput.RuntimeID = ""
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.ErrorIs(t, err, errDagsParseFailed)

mockClient.On("ListDeployments", org, "invalid-workspace").Return(mockDeplyResp, nil).Once()
defer testUtil.MockUserInput(t, "y")()
deployInput.RuntimeID = "test-id"
deployInput.WsID = "invalid-workspace"
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.NoError(t, err)

defer testUtil.MockUserInput(t, "y")()
deployInput.WsID = ws
deployInput.EnvFile = "invalid-path"
err = Deploy(deployInput, mockClient)
err = Deploy(deployInput, mockClient, mockCoreClient)
assert.ErrorIs(t, err, envFileMissing)

mockClient.AssertExpectations(t)
Expand Down
2 changes: 1 addition & 1 deletion cmd/cloud/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,5 +139,5 @@ func deploy(cmd *cobra.Command, args []string) error {
DagsPath: dagsPath,
}

return DeployImage(deployInput, astroClient)
return DeployImage(deployInput, astroClient, astroCoreClient)
}
3 changes: 2 additions & 1 deletion cmd/cloud/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

astro "github.com/astronomer/astro-cli/astro-client"
astrocore "github.com/astronomer/astro-cli/astro-client-core"
cloud "github.com/astronomer/astro-cli/cloud/deploy"
testUtil "github.com/astronomer/astro-cli/pkg/testing"
"github.com/spf13/cobra"
Expand All @@ -25,7 +26,7 @@ func TestDeployImage(t *testing.T) {
return nil
}

DeployImage = func(deployInput cloud.InputDeploy, client astro.Client) error {
DeployImage = func(deployInput cloud.InputDeploy, client astro.Client, coreClient astrocore.CoreClient) error {
return nil
}

Expand Down

0 comments on commit b76e610

Please sign in to comment.