Skip to content

Commit

Permalink
Add the ability to change the deployment type to dag_deploy from the …
Browse files Browse the repository at this point in the history 
…CLI (both for create and update) (#1516)

* Add the ability to change the deployment type to dag_deploy from the CLI

* Update version of golangci-lint to the latest version (#1522)

This is needed to support Go 1.21

- Depguard changed to v2 so the default mod changed
- Revive was just noisey, so has been removed
- I removed some repeated (sub)strings into consts
- I also moved a repeated code block (detected by contsnts) into a
  function

* Update to latest Go versions (#1521)

Also switch from using deprecated CircleCI images in favour of the new
ones

* Streamline getDeploymentTypeCmdMessage to show image, volume, git_sync, dag_only for all cases

* change function call to constant

---------

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
  • Loading branch information
@rujhan-arora-astronomer @ashb
rujhan-arora-astronomer and ashb committed Feb 2, 2024
1 parent 5667296 commit 0764455
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 16 deletions.
25 changes: 15 additions & 10 deletions cmd/software/deployment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const (
k8sExecutorArg = "k8s"

cliDeploymentHardDeletePrompt = "\nWarning: This action permanently deletes all data associated with this Deployment, including the database. You will not be able to recover it. Proceed with hard delete?"
deploymentTypeCmdMessage = "DAG Deployment mechanism: image, volume, git_sync, dag_only"
)

var (
Expand Down Expand Up @@ -120,17 +121,18 @@ func newDeploymentCreateCmd(out io.Writer) *cobra.Command {
},
}

var nfsMountDAGDeploymentEnabled, triggererEnabled, gitSyncDAGDeploymentEnabled, runtimeEnabled bool
var nfsMountDAGDeploymentEnabled, triggererEnabled, gitSyncDAGDeploymentEnabled, runtimeEnabled, dagOnlyDeployEnabled bool
if appConfig != nil {
nfsMountDAGDeploymentEnabled = appConfig.Flags.NfsMountDagDeployment
triggererEnabled = appConfig.Flags.TriggererEnabled
gitSyncDAGDeploymentEnabled = appConfig.Flags.GitSyncEnabled
runtimeEnabled = appConfig.Flags.AstroRuntimeEnabled
dagOnlyDeployEnabled = appConfig.Flags.DagOnlyDeployment
}

// let's hide under feature flag
if nfsMountDAGDeploymentEnabled || gitSyncDAGDeploymentEnabled {
cmd.Flags().StringVarP(&dagDeploymentType, "dag-deployment-type", "t", "", "DAG Deployment mechanism: image, volume, git_sync")
if nfsMountDAGDeploymentEnabled || gitSyncDAGDeploymentEnabled || dagOnlyDeployEnabled {
cmd.Flags().StringVarP(&dagDeploymentType, "dag-deployment-type", "t", "", deploymentTypeCmdMessage)
}

if nfsMountDAGDeploymentEnabled {
Expand Down Expand Up @@ -210,18 +212,19 @@ $ astro deployment update [deployment ID] --dag-deployment-type=volume --nfs-loc
},
}

var nfsMountDAGDeploymentEnabled, triggererEnabled, gitSyncDAGDeploymentEnabled bool
var nfsMountDAGDeploymentEnabled, triggererEnabled, gitSyncDAGDeploymentEnabled, dagOnlyDeployEnabled bool
if appConfig != nil {
nfsMountDAGDeploymentEnabled = appConfig.Flags.NfsMountDagDeployment
triggererEnabled = appConfig.Flags.TriggererEnabled
gitSyncDAGDeploymentEnabled = appConfig.Flags.GitSyncEnabled
dagOnlyDeployEnabled = appConfig.Flags.DagOnlyDeployment
}

cmd.Flags().StringVarP(&executorUpdate, "executor", "e", "", "Add executor parameter: local, celery, or kubernetes")

// let's hide under feature flag
if nfsMountDAGDeploymentEnabled || gitSyncDAGDeploymentEnabled {
cmd.Flags().StringVarP(&dagDeploymentType, "dag-deployment-type", "t", "", "DAG Deployment mechanism: image, volume, git_sync")
if nfsMountDAGDeploymentEnabled || gitSyncDAGDeploymentEnabled || dagOnlyDeployEnabled {
cmd.Flags().StringVarP(&dagDeploymentType, "dag-deployment-type", "t", "", deploymentTypeCmdMessage)
}

if nfsMountDAGDeploymentEnabled {
Expand Down Expand Up @@ -360,18 +363,19 @@ func deploymentCreate(cmd *cobra.Command, out io.Writer) error {
return err
}

var nfsMountDAGDeploymentEnabled, gitSyncDAGDeploymentEnabled bool
var nfsMountDAGDeploymentEnabled, gitSyncDAGDeploymentEnabled, dagOnlyDeployEnabled bool
if appConfig != nil {
nfsMountDAGDeploymentEnabled = appConfig.Flags.NfsMountDagDeployment
gitSyncDAGDeploymentEnabled = appConfig.Flags.GitSyncEnabled
dagOnlyDeployEnabled = appConfig.Flags.DagOnlyDeployment
}

if !cmd.Flags().Changed("triggerer-replicas") {
createTriggererReplicas = -1
}

// we should validate only in case when this feature has been enabled
if nfsMountDAGDeploymentEnabled || gitSyncDAGDeploymentEnabled {
if nfsMountDAGDeploymentEnabled || gitSyncDAGDeploymentEnabled || dagOnlyDeployEnabled {
err = validateDagDeploymentArgs(dagDeploymentType, nfsLocation, gitRepoURL, false)
if err != nil {
return err
Expand Down Expand Up @@ -442,14 +446,15 @@ func deploymentUpdate(cmd *cobra.Command, args []string, dagDeploymentType, nfsL
// Silence Usage as we have now validated command input
cmd.SilenceUsage = true

var nfsMountDAGDeploymentEnabled, gitSyncDAGDeploymentEnabled bool
var nfsMountDAGDeploymentEnabled, gitSyncDAGDeploymentEnabled, dagOnlyDeployEnabled bool
if appConfig != nil {
nfsMountDAGDeploymentEnabled = appConfig.Flags.NfsMountDagDeployment
gitSyncDAGDeploymentEnabled = appConfig.Flags.GitSyncEnabled
dagOnlyDeployEnabled = appConfig.Flags.DagOnlyDeployment
}

// we should validate only in case when this feature has been enabled
if nfsMountDAGDeploymentEnabled || gitSyncDAGDeploymentEnabled {
if nfsMountDAGDeploymentEnabled || gitSyncDAGDeploymentEnabled || dagOnlyDeployEnabled {
err := validateDagDeploymentArgs(dagDeploymentType, nfsLocation, gitRepoURL, true)
if err != nil {
return err
Expand Down
70 changes: 67 additions & 3 deletions cmd/software/deployment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func TestDeploymentCreateCommandNfsMountEnabled(t *testing.T) {
}{
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=volume", "--nfs-location=test:/test"}, expectedOutput: "Successfully created deployment with Celery executor. Deployment can be accessed at the following URLs", expectedError: ""},
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery"}, expectedOutput: "Successfully created deployment with Celery executor. Deployment can be accessed at the following URLs", expectedError: ""},
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=dummy"}, expectedOutput: "", expectedError: "please specify the correct DAG deployment type, one of the following: image, volume, git_sync"},
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=dummy"}, expectedOutput: "", expectedError: ErrInvalidDAGDeploymentType.Error()},
}
for _, tt := range myTests {
houstonClient = api
Expand Down Expand Up @@ -225,7 +225,39 @@ func TestDeploymentCreateCommandGitSyncEnabled(t *testing.T) {
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=git_sync", "--git-repository-url=git@github.com:bote795/private-ariflow-dags-test.git", "--dag-directory-path=dagscopy/", "--git-branch-name=main", "--ssh-key=./testfiles/ssh_key", "--known-hosts=./testfiles/known_hosts"}, expectedOutput: "Successfully created deployment with Celery executor. Deployment can be accessed at the following URLs", expectedError: ""},
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=git_sync", "--git-repository-url=git@github.com:neel-astro/private-airflow-dags-test.git", "--dag-directory-path=dagscopy/", "--git-branch-name=main", "--ssh-key=./testfiles/ssh_key", "--known-hosts=./testfiles/known_hosts"}, expectedOutput: "Successfully created deployment with Celery executor. Deployment can be accessed at the following URLs", expectedError: ""},
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=git_sync", "--git-repository-url=git@github.com:neel-astro/private-airflow-dags-test.git", "--ssh-key=./testfiles/ssh_key", "--known-hosts=./testfiles/known_hosts"}, expectedOutput: "Successfully created deployment with Celery executor. Deployment can be accessed at the following URLs", expectedError: ""},
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=dummy"}, expectedOutput: "", expectedError: "please specify the correct DAG deployment type, one of the following: image, volume, git_sync"},
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=dummy"}, expectedOutput: "", expectedError: ErrInvalidDAGDeploymentType.Error()},
}
for _, tt := range myTests {
houstonClient = api
output, err := execDeploymentCmd(tt.cmdArgs...)
if tt.expectedError != "" {
assert.EqualError(t, err, tt.expectedError)
} else {
assert.NoError(t, err)
}
assert.Contains(t, output, tt.expectedOutput)
}
}

func TestDeploymentCreateCommandDagOnlyDeployEnabled(t *testing.T) {
testUtil.InitTestConfig(testUtil.SoftwarePlatform)
appConfig = &houston.AppConfig{
Flags: houston.FeatureFlags{
DagOnlyDeployment: true,
},
}

api := new(mocks.ClientInterface)
api.On("GetAppConfig", nil).Return(appConfig, nil)
api.On("CreateDeployment", mock.Anything).Return(mockDeployment, nil).Times(5)

myTests := []struct {
cmdArgs []string
expectedOutput string
expectedError string
}{
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=dag_deploy"}, expectedOutput: "Successfully created deployment with Celery executor. Deployment can be accessed at the following URLs", expectedError: ""},
{cmdArgs: []string{"create", "--label=new-deployment-name", "--executor=celery", "--dag-deployment-type=dummy"}, expectedOutput: "", expectedError: ErrInvalidDAGDeploymentType.Error()},
}
for _, tt := range myTests {
houstonClient = api
Expand Down Expand Up @@ -325,7 +357,7 @@ func TestDeploymentUpdateCommand(t *testing.T) {
{cmdArgs: []string{"update", "cknrml96n02523xr97ygj95n5", "--label=test22222", "--dag-deployment-type=git_sync", "--git-repository-url=git@github.com:bote795/private-ariflow-dags-test.git", "--dag-directory-path=dagscopy/", "--git-branch-name=main", "--ssh-key=./testfiles/ssh_key", "--known-hosts=./testfiles/known_hosts"}, expectedOutput: "Successfully updated deployment", expectedError: ""},
{cmdArgs: []string{"update", "cknrml96n02523xr97ygj95n5", "--label=test22222", "--dag-deployment-type=git_sync", "--git-repository-url=git@github.com:neel-astro/private-airflow-dags-test.git", "--dag-directory-path=dagscopy/", "--git-branch-name=main", "--ssh-key=./testfiles/ssh_key", "--known-hosts=./testfiles/known_hosts"}, expectedOutput: "Successfully updated deployment", expectedError: ""},
{cmdArgs: []string{"update", "cknrml96n02523xr97ygj95n5", "--label=test22222", "--dag-deployment-type=git_sync", "--git-repository-url=git@github.com:neel-astro/private-airflow-dags-test.git", "--ssh-key=./testfiles/ssh_key", "--known-hosts=./testfiles/known_hosts"}, expectedOutput: "Successfully updated deployment", expectedError: ""},
{cmdArgs: []string{"update", "cknrml96n02523xr97ygj95n5", "--label=test22222", "--dag-deployment-type=wrong", "--nfs-location=test:/test"}, expectedOutput: "", expectedError: "please specify the correct DAG deployment type, one of the following: image, volume, git_sync"},
{cmdArgs: []string{"update", "cknrml96n02523xr97ygj95n5", "--label=test22222", "--dag-deployment-type=wrong", "--nfs-location=test:/test"}, expectedOutput: "", expectedError: ErrInvalidDAGDeploymentType.Error()},
{cmdArgs: []string{"update", "cknrml96n02523xr97ygj95n5", "--label=test22222", "--executor=local"}, expectedOutput: "Successfully updated deployment", expectedError: ""},
{cmdArgs: []string{"update", "cknrml96n02523xr97ygj95n5", "--cloud-role=arn:aws:iam::1234567890:role/test_role4c2301381e"}, expectedOutput: "Successfully updated deployment", expectedError: ""},
}
Expand Down Expand Up @@ -375,6 +407,38 @@ func TestDeploymentUpdateCommandGitSyncDisabled(t *testing.T) {
}
}

func TestDeploymentUpdateCommandDagOnlyDeployEnabled(t *testing.T) {
testUtil.InitTestConfig(testUtil.SoftwarePlatform)
appConfig = &houston.AppConfig{
Flags: houston.FeatureFlags{
DagOnlyDeployment: true,
},
}

api := new(mocks.ClientInterface)
api.On("GetAppConfig", nil).Return(appConfig, nil)
api.On("UpdateDeployment", mock.Anything).Return(mockDeployment, nil)

myTests := []struct {
cmdArgs []string
expectedOutput string
expectedError string
}{
{cmdArgs: []string{"update", "cknrml96n02523xr97ygj95n5", "--label=test22222", "--dag-deployment-type=dag_deploy"}, expectedOutput: "Successfully updated deployment", expectedError: ""},
{cmdArgs: []string{"update", "cknrml96n02523xr97ygj95n5", "--label=test22222", "--dag-deployment-type=invalid"}, expectedOutput: "", expectedError: ErrInvalidDAGDeploymentType.Error()},
}
for _, tt := range myTests {
houstonClient = api
output, err := execDeploymentCmd(tt.cmdArgs...)
if tt.expectedError != "" {
assert.EqualError(t, err, tt.expectedError)
} else {
assert.NoError(t, err)
}
assert.Contains(t, output, tt.expectedOutput)
}
}

func TestDeploymentAirflowUpgradeCommand(t *testing.T) {
testUtil.InitTestConfig(testUtil.SoftwarePlatform)
expectedOut := `The upgrade from Airflow 1.10.5 to 1.10.10 has been started. To complete this process, add an Airflow 1.10.10 image to your Dockerfile and deploy to Astronomer.`
Expand Down
5 changes: 3 additions & 2 deletions cmd/software/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
)

var (
errInvalidDAGDeploymentType = errors.New("please specify the correct DAG deployment type, one of the following: image, volume, git_sync")
ErrInvalidDAGDeploymentType = errors.New("please specify the correct DAG deployment type, one of the following: image, volume, git_sync(if enabled in platform config), dag_deploy(if enabled in platform config)")
errNFSLocationNotFound = errors.New("please specify the nfs location via --nfs-location flag")
errGitRepoNotFound = errors.New("please specify a valid git repository URL via --git-repository-url")
errInvalidExecutorType = errors.New("please specify correct executor, one of: local, celery, kubernetes, k8s")
Expand Down Expand Up @@ -87,8 +87,9 @@ func validateDagDeploymentArgs(dagDeploymentType, nfsLocation, gitRepoURL string
if dagDeploymentType != houston.ImageDeploymentType &&
dagDeploymentType != houston.VolumeDeploymentType &&
dagDeploymentType != houston.GitSyncDeploymentType &&
dagDeploymentType != houston.DagOnlyDeploymentType &&
dagDeploymentType != "" {
return errInvalidDAGDeploymentType
return ErrInvalidDAGDeploymentType
}
if dagDeploymentType == houston.VolumeDeploymentType && nfsLocation == "" {
return errNFSLocationNotFound
Expand Down
3 changes: 2 additions & 1 deletion cmd/software/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func TestValidateDagDeploymentArgs(t *testing.T) {
}{
{dagDeploymentType: houston.VolumeDeploymentType, nfsLocation: "test:/test", expectedError: nil},
{dagDeploymentType: houston.ImageDeploymentType, expectedError: nil},
{dagDeploymentType: houston.DagOnlyDeploymentType, expectedError: nil},
{dagDeploymentType: houston.GitSyncDeploymentType, gitRepoURL: "https://github.com/neel-astro/private-airflow-dags-test", expectedError: nil},
{dagDeploymentType: houston.GitSyncDeploymentType, gitRepoURL: "http://github.com/neel-astro/private-airflow-dags-test", expectedError: nil},
{dagDeploymentType: houston.GitSyncDeploymentType, gitRepoURL: "git@github.com:neel-astro/private-airflow-dags-test.git", expectedError: nil},
Expand All @@ -40,7 +41,7 @@ func TestValidateDagDeploymentArgsErrors(t *testing.T) {
expectedError string
}{
{dagDeploymentType: houston.VolumeDeploymentType, expectedError: "please specify the nfs location via --nfs-location flag"},
{dagDeploymentType: "unknown", expectedError: "please specify the correct DAG deployment type, one of the following: image, volume, git_sync"},
{dagDeploymentType: "unknown", expectedError: ErrInvalidDAGDeploymentType.Error()},
{dagDeploymentType: houston.ImageDeploymentType, expectedError: ""},
{dagDeploymentType: houston.GitSyncDeploymentType, expectedError: "please specify a valid git repository URL via --git-repository-url"},
{dagDeploymentType: houston.GitSyncDeploymentType, gitRepoURL: "/tmp/test/local-repo.git", expectedError: "please specify a valid git repository URL via --git-repository-url"},
Expand Down

0 comments on commit 0764455

Please sign in to comment.