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

feat: add --description flag to astro deploy command #1713

Merged
merged 17 commits into from
Sep 17, 2024
Merged
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ cover.out
coverage-2.txt
coverage.txt
dist/
hello-astro/
meta/
packages.txt
requirements.txt
Expand Down
14 changes: 11 additions & 3 deletions cmd/software/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ var (
DeployAirflowImage = deploy.Airflow
DagsOnlyDeploy = deploy.DagsOnlyDeploy
isDagOnlyDeploy bool
description string
)

var deployExample = `
Expand Down Expand Up @@ -56,6 +57,7 @@ func NewDeployCmd() *cobra.Command {
cmd.Flags().BoolVarP(&saveDeployConfig, "save", "s", false, "Save deployment in config for future deploys")
cmd.Flags().BoolVarP(&ignoreCacheDeploy, "no-cache", "", false, "Do not use cache when building container image")
cmd.Flags().StringVar(&workspaceID, "workspace-id", "", "workspace assigned to deployment")
cmd.Flags().StringVar(&description, "description", "", "Reason for the deploy update")

if !context.IsCloudContext() && houston.VerifyVersionMatch(houstonVersion, houston.VersionRestrictions{GTE: "0.34.0"}) {
cmd.Flags().BoolVarP(&isDagOnlyDeploy, "dags", "d", false, "Push only DAGs to your Deployment")
Expand Down Expand Up @@ -101,16 +103,22 @@ func deployAirflow(cmd *cobra.Command, args []string) error {
return deploy.ErrBYORegistryDomainNotSet
}
}

if description == "" {
description = utils.GetDefaultDeployDescription(isDagOnlyDeploy)
}

if isDagOnlyDeploy {
return DagsOnlyDeploy(houstonClient, appConfig, ws, deploymentID, config.WorkingPath, nil, true)
return DagsOnlyDeploy(houstonClient, appConfig, ws, deploymentID, config.WorkingPath, nil, true, description)
}

// Since we prompt the user to enter the deploymentID in come cases for DeployAirflowImage, reusing the same deploymentID for DagsOnlyDeploy
deploymentID, err = DeployAirflowImage(houstonClient, config.WorkingPath, deploymentID, ws, byoRegistryDomain, ignoreCacheDeploy, byoRegistryEnabled, forcePrompt)
deploymentID, err = DeployAirflowImage(houstonClient, config.WorkingPath, deploymentID, ws, byoRegistryDomain, ignoreCacheDeploy, byoRegistryEnabled, forcePrompt, description)
if err != nil {
return err
}

err = DagsOnlyDeploy(houstonClient, appConfig, ws, deploymentID, config.WorkingPath, nil, true)
err = DagsOnlyDeploy(houstonClient, appConfig, ws, deploymentID, config.WorkingPath, nil, true, description)
// Don't throw the error if dag-deploy itself is disabled
if errors.Is(err, deploy.ErrDagOnlyDeployDisabledInConfig) || errors.Is(err, deploy.ErrDagOnlyDeployNotEnabledForDeployment) {
return nil
Expand Down
34 changes: 28 additions & 6 deletions cmd/software/deploy_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package software

import (
"fmt"

"github.com/astronomer/astro-cli/houston"
testUtil "github.com/astronomer/astro-cli/pkg/testing"
"github.com/astronomer/astro-cli/software/deploy"
Expand All @@ -25,11 +27,14 @@ func (s *Suite) TestDeploy() {
EnsureProjectDir = func(cmd *cobra.Command, args []string) error {
return nil
}
DeployAirflowImage = func(houstonClient houston.ClientInterface, path, deploymentID, wsID, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled, prompt bool) (string, error) {
DeployAirflowImage = func(houstonClient houston.ClientInterface, path, deploymentID, wsID, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled, prompt bool, description string) (string, error) {
if description == "" {
return deploymentID, fmt.Errorf("description should not be empty")
}
return deploymentID, nil
}

DagsOnlyDeploy = func(houstonClient houston.ClientInterface, appConfig *houston.AppConfig, wsID, deploymentID, dagsParentPath string, dagDeployURL *string, cleanUpFiles bool) error {
DagsOnlyDeploy = func(houstonClient houston.ClientInterface, appConfig *houston.AppConfig, wsID, deploymentID, dagsParentPath string, dagDeployURL *string, cleanUpFiles bool, description string) error {
return nil
}

Expand All @@ -42,23 +47,40 @@ func (s *Suite) TestDeploy() {
err = execDeployCmd([]string{"test-deployment-id", "--save"}...)
s.NoError(err)

// Test when description is provided using the flag --description
err = execDeployCmd([]string{"test-deployment-id", "--description", "Initial deployment", "--force"}...)
s.NoError(err)

// Test when the default description is used
DeployAirflowImage = func(houstonClient houston.ClientInterface, path, deploymentID, wsID, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled, prompt bool, description string) (string, error) {
expectedDesc := "Deployed via <astro deploy>"
if description != expectedDesc {
return deploymentID, fmt.Errorf("expected description to be '%s', but got '%s'", expectedDesc, description)
}
return deploymentID, nil
}

err = execDeployCmd([]string{"test-deployment-id", "--force"}...)
s.NoError(err)

// Restore DagsOnlyDeploy to default behavior
DagsOnlyDeploy = deploy.DagsOnlyDeploy

s.Run("error should be returned for astro deploy, if DeployAirflowImage throws error", func() {
DeployAirflowImage = func(houstonClient houston.ClientInterface, path, deploymentID, wsID, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled, prompt bool) (string, error) {
DeployAirflowImage = func(houstonClient houston.ClientInterface, path, deploymentID, wsID, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled, prompt bool, description string) (string, error) {
return deploymentID, deploy.ErrNoWorkspaceID
}

err := execDeployCmd([]string{"-f"}...)
s.ErrorIs(err, deploy.ErrNoWorkspaceID)

DeployAirflowImage = func(houstonClient houston.ClientInterface, path, deploymentID, wsID, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled, prompt bool) (string, error) {
DeployAirflowImage = func(houstonClient houston.ClientInterface, path, deploymentID, wsID, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled, prompt bool, description string) (string, error) {
return deploymentID, nil
}
})

s.Run("error should be returned for astro deploy, if dags deploy throws error and the feature is enabled", func() {
DagsOnlyDeploy = func(houstonClient houston.ClientInterface, appConfig *houston.AppConfig, wsID, deploymentID, dagsParentPath string, dagDeployURL *string, cleanUpFiles bool) error {
DagsOnlyDeploy = func(houstonClient houston.ClientInterface, appConfig *houston.AppConfig, wsID, deploymentID, dagsParentPath string, dagDeployURL *string, cleanUpFiles bool, description string) error {
return deploy.ErrNoWorkspaceID
}
err := execDeployCmd([]string{"-f"}...)
Expand All @@ -83,7 +105,7 @@ func (s *Suite) TestDeploy() {
BYORegistryEnabled: true,
},
}
DagsOnlyDeploy = func(houstonClient houston.ClientInterface, appConfig *houston.AppConfig, wsID, deploymentID, dagsParentPath string, dagDeployURL *string, cleanUpFiles bool) error {
DagsOnlyDeploy = func(houstonClient houston.ClientInterface, appConfig *houston.AppConfig, wsID, deploymentID, dagsParentPath string, dagDeployURL *string, cleanUpFiles bool, description string) error {
return deploy.ErrNoWorkspaceID
}
err := execDeployCmd([]string{"-f"}...)
Expand Down
8 changes: 8 additions & 0 deletions cmd/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,11 @@ func EnsureProjectDir(cmd *cobra.Command, args []string) error {

return nil
}

func GetDefaultDeployDescription(isDagOnlyDeploy bool) string {
if isDagOnlyDeploy {
return "Deployed via <astro deploy --dags>"
}

return "Deployed via <astro deploy>"
}
10 changes: 10 additions & 0 deletions cmd/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,13 @@ func TestEnsureProjectDir(t *testing.T) {
err = EnsureProjectDir(&cobra.Command{}, []string{})
assert.NoError(t, err)
}

func TestGetDefaultDeployDescription(t *testing.T) {
// Test case where --dags flag is not set
description := GetDefaultDeployDescription(false)
assert.Equal(t, "Deployed via <astro deploy>", description)

// Test case where --dags flag is set
descriptionWithDags := GetDefaultDeployDescription(true)
assert.Equal(t, "Deployed via <astro deploy --dags>", descriptionWithDags)
}
4 changes: 2 additions & 2 deletions houston/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ var (
workspaceUuid: $workspaceId
releaseName: $releaseName
executor: $executor
airflowVersion: $airflowVersion
airflowVersion: $airflowVersion
namespace: $namespace
config: $config
cloudRole: $cloudRole
Expand Down Expand Up @@ -96,7 +96,7 @@ var (
workspaceUuid: $workspaceId
releaseName: $releaseName
executor: $executor
airflowVersion: $airflowVersion
airflowVersion: $airflowVersion
namespace: $namespace
config: $config
cloudRole: $cloudRole
Expand Down
18 changes: 11 additions & 7 deletions software/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ var tab = printutil.Table{
Header: []string{"#", "LABEL", "DEPLOYMENT NAME", "WORKSPACE", "DEPLOYMENT ID"},
}

func Airflow(houstonClient houston.ClientInterface, path, deploymentID, wsID, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled, prompt bool) (string, error) {
func Airflow(houstonClient houston.ClientInterface, path, deploymentID, wsID, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled, prompt bool, description string) (string, error) {
deploymentID, deployments, err := getDeploymentIDForCurrentCommand(houstonClient, wsID, deploymentID, prompt)
if err != nil {
return deploymentID, err
Expand Down Expand Up @@ -98,7 +98,7 @@ func Airflow(houstonClient houston.ClientInterface, path, deploymentID, wsID, by
fmt.Printf(houstonDeploymentPrompt, releaseName)

// Build the image to deploy
err = buildPushDockerImage(houstonClient, &c, deploymentInfo, releaseName, path, nextTag, cloudDomain, byoRegistryDomain, ignoreCacheDeploy, byoRegistryEnabled)
err = buildPushDockerImage(houstonClient, &c, deploymentInfo, releaseName, path, nextTag, cloudDomain, byoRegistryDomain, ignoreCacheDeploy, byoRegistryEnabled, description)
if err != nil {
return deploymentID, err
}
Expand All @@ -120,7 +120,7 @@ func deploymentExists(deploymentID string, deployments []houston.Deployment) boo
return false
}

func buildPushDockerImage(houstonClient houston.ClientInterface, c *config.Context, deploymentInfo *houston.Deployment, name, path, nextTag, cloudDomain, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled bool) error {
func buildPushDockerImage(houstonClient houston.ClientInterface, c *config.Context, deploymentInfo *houston.Deployment, name, path, nextTag, cloudDomain, byoRegistryDomain string, ignoreCacheDeploy, byoRegistryEnabled bool, description string) error {
// Build our image
fmt.Println(imageBuildingPrompt)

Expand Down Expand Up @@ -170,6 +170,10 @@ func buildPushDockerImage(houstonClient houston.ClientInterface, c *config.Conte

imageHandler := imageHandlerInit(imageName)

if description != "" {
deployLabels = append(deployLabels, "io.astronomer.deploy.revision.description="+description)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: silly question, how does this description pop up in the UI when it's a DAG-only deploy because CLI is only setting it on the image, and I am not sure we do an image deploy when the --dags flag is passed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, should we extend this flag to --dag-only command too? @Simpcyclassy

Copy link
Member Author

Choose a reason for hiding this comment

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

Not a silly question at all. That is what this PR #1716 is for. It would send the description to dag server which creates a revision on houston

Copy link
Contributor

Choose a reason for hiding this comment

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

are we looking to release both the changes together?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Copy link
Contributor

Choose a reason for hiding this comment

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

why are we setting io.astronomer.deploy.revision.description

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we are setting the description through the api request in the image

}

buildConfig := types.ImageBuildConfig{
Path: config.WorkingPath,
NoCache: ignoreCacheDeploy,
Expand Down Expand Up @@ -331,12 +335,12 @@ func validateIfDagDeployURLCanBeConstructed(deploymentInfo *houston.Deployment)
return nil
}

func getDagDeployURL(deploymentInfo *houston.Deployment) string {
func getDagDeployURL(deploymentInfo *houston.Deployment, description string) string {
c, _ := config.GetCurrentContext()
return fmt.Sprintf("https://deployments.%s/%s/dags/upload", c.Domain, deploymentInfo.ReleaseName)
return fmt.Sprintf("https://deployments.%s/%s/dags/upload?description=%s", c.Domain, deploymentInfo.ReleaseName, description)
}

func DagsOnlyDeploy(houstonClient houston.ClientInterface, appConfig *houston.AppConfig, wsID, deploymentID, dagsParentPath string, dagDeployURL *string, cleanUpFiles bool) error {
func DagsOnlyDeploy(houstonClient houston.ClientInterface, appConfig *houston.AppConfig, wsID, deploymentID, dagsParentPath string, dagDeployURL *string, cleanUpFiles bool, description string) error {
// Throw error if the feature is disabled at Houston level
if !isDagOnlyDeploymentEnabled(appConfig) {
return ErrDagOnlyDeployDisabledInConfig
Expand Down Expand Up @@ -368,7 +372,7 @@ func DagsOnlyDeploy(houstonClient houston.ClientInterface, appConfig *houston.Ap
if err != nil {
return err
}
uploadURL = getDagDeployURL(deploymentInfo)
uploadURL = getDagDeployURL(deploymentInfo, description)
} else {
uploadURL = *dagDeployURL
}
Expand Down
Loading