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

Conversation

Simpcyclassy
Copy link
Member

@Simpcyclassy Simpcyclassy commented Sep 4, 2024

Description

This PR introduces functionality to support custom deployment revision descriptions during the astro deploy CLI command. It adds a mechanism to capture and assign meaningful descriptions for deployment revisions, enhancing traceability and clarity in the deployment process.

Changes Implemented:

  • Description Flag Handling:

    • Added a --description flag to the astro deploy command, allowing users to specify a custom description for the deployment revision.
    • If no custom description is provided, the command defaults to a generic description using the GetDefaultDeployDescription method, which dynamically assigns a description based on the deploy command used (e.g., DAGs-only - astro deploy --dags OR full image deploy - astro deploy).
  • Label Integration:

    • The custom or default description is appended as a label (io.astronomer.deploy.revision.description) to the deployLabels slice, allowing the deployment system to track the description as part of the Docker image metadata.

Benefits:

  • Improved Traceability:

    • By allowing custom deploy revision descriptions, users can now provide meaningful context for each deployment (e.g., "Fix issue with triggerer pods").
    • This ensures that deployment logs and UI reflect the purpose behind each deployment, making it easier for teams to audit and understand deployment histories.
  • Default Descriptions:

    • If no description is provided by the user, the system will automatically assign a default description based on the deployment type. This ensures that every deployment has some form of description, even if it’s not user-defined, improving consistency and traceability.

Example Usage:

  • Custom Description:

    astro deploy --description="Fix issue with triggerer pods"
    

    Result: Fix issue with triggerer pods

  • DAGs-Only Deploy (Default Description):

    astro deploy --dags
    

    Result: Deployed via <astro deploy --dags>

Why This Change?

This update allows developers and deployment teams to provide additional context for each deployment revision, helping them better manage, track, and audit their deployment histories. By introducing both custom and default descriptions, it provides a flexible and structured way to document the purpose of each deployment.

🎟 Issue(s)

Related astronomer/issues#6437

🧪 Functional Testing

List the functional testing steps to confirm this feature or fix.

📸 Screenshots

image

📋 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Ran make lint before taking out of draft
  • Added/updated applicable tests
  • Tested against Astro-API (if necessary).
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

@Simpcyclassy Simpcyclassy self-assigned this Sep 10, 2024
@Simpcyclassy Simpcyclassy marked this pull request as ready for review September 10, 2024 22:01
Copy link
Contributor

@neel-astro neel-astro left a comment

Choose a reason for hiding this comment

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

left minor suggestion and question

cmd/software/deploy.go Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
@@ -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

cmd/software/deploy.go Outdated Show resolved Hide resolved
@@ -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.

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

@Simpcyclassy
Copy link
Member Author

Changes addressed @neel-astro @rujhan-arora-astronomer

Copy link
Contributor

@neel-astro neel-astro left a comment

Choose a reason for hiding this comment

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

Had a second pass, left some minor comments to squash, LGTM otherwise :)

@@ -104,8 +106,12 @@ func deployAirflow(cmd *cobra.Command, args []string) error {
if isDagOnlyDeploy {
return DagsOnlyDeploy(houstonClient, appConfig, ws, deploymentID, config.WorkingPath, nil, true)
}

if description == "" {
description = utils.GetDefaultDeployDescription(cmd, args)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description = utils.GetDefaultDeployDescription(cmd, args)
description = utils.GetDefaultDeployDescription(isDagOnlyDeploy)

nit: we could simply send the isDagOnlyDeploy instead of re-parsing/reading the value for the dags flag. Then in the GetDefaultDeployDescription function check if isDagOnlyDeploy != nil && *isDagOnlyDeploy == true

Copy link
Member Author

@Simpcyclassy Simpcyclassy Sep 13, 2024

Choose a reason for hiding this comment

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

I could also simply do if isDagOnlyDeploy right?

@@ -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.

are we looking to release both the changes together?

software/deploy/deploy_test.go Show resolved Hide resolved
@danielhoherd danielhoherd merged commit 0a98b4f into main Sep 17, 2024
3 checks passed
@danielhoherd danielhoherd deleted the feature/add-description-flag branch September 17, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants