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

ORION-3127: initial implementation for fsoc solution delete #324

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

bemidji3
Copy link
Contributor

Description

This PR contains changes to add a new solution command: fsoc solution delete. This will allow our users to delete non-stable tagged versions of their solutions via fsoc.

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change
  • Refactor
  • Documentation
  • Other (please describe)

Checklist

  • I have read the contributing guidelines
  • Existing issues have been referenced (where applicable)
  • I have verified this change is not present in other open pull requests
  • Functionality is documented
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

@codecov-commenter
Copy link

codecov-commenter commented Mar 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 85 lines in your changes are missing coverage. Please review.

Project coverage is 10.33%. Comparing base (00d0c6f) to head (8e624fe).
Report is 28 commits behind head on main.

Files Patch % Lines
cmd/solution/delete.go 0.00% 84 Missing ⚠️
cmd/solution/solution.go 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #324       +/-   ##
===========================================
- Coverage   26.88%   10.33%   -16.56%     
===========================================
  Files          44      135       +91     
  Lines        4564    10513     +5949     
===========================================
- Hits         1227     1086      -141     
- Misses       3242     9352     +6110     
+ Partials       95       75       -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…and user wants to delete it again and wait for deletion to complete
Please also note this is an asynchronous operation and thus it may take some time for the status to reflect properly.`,
Example: `fsoc solution delete mysolution`,
Run: deleteSolution,
Annotations: map[string]string{config.AnnotationForConfigBypass: ""},
Copy link
Contributor

Choose a reason for hiding this comment

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

This command relies on the configuration, as it reaches out to the API.

Suggested change
Annotations: map[string]string{config.AnnotationForConfigBypass: ""},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved


var solutionDeleteCmd = &cobra.Command{
Use: "delete <solution-name>",
Args: cobra.MinimumNArgs(1),
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
Args: cobra.MinimumNArgs(1),
Args: cobra.NArgs(1),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

Use: "delete <solution-name>",
Args: cobra.MinimumNArgs(1),
Short: "Delete a non-stable tagged version of a solution",
Long: `This command deletes a non-stable tagged version of a solution uploaded by your tenant.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the descriptions, I propose we remove the "non-stable tagged" from most places and just have a simple sentence in the Long text that says something along the lines of "Solutions with stable tag cannot be deleted" (or may not be deleted), mostly to set expectations. Fsoc doesn't enforce this / we rely on the backend.

WRT removing "all active subscriptions": given that we support deleting only non-stable solutions and non-stable solutions are tenant-private, I propose we do unsubscribe as part of the delete (and there are no other subscriptions possible). This will remove the incidental complexity around subscriptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that we should remove "non-stable tagged" from here and will replace it like you have suggested. Regarding the auto-unsubscribe functionality: I think this should be behind a cli flag as we want our users to be cognizant of their subscriptions to solutions and the subscription process. Wdyt?

}

if !skipConfirmationMessage {
fmt.Printf("WARNING! This command will remove all of the objects and types that are associated with this solution and will purge all data related to those objects and types. It will also remove all solution metadata (including, but not limited to, subscriptions and other related objects).\nProceed with caution! \nPlease type the name of the solution you want to delete and hit enter confirm that you want to delete the solution with name: %s and tag: %s \n", solutionName, solutionTag)
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
fmt.Printf("WARNING! This command will remove all of the objects and types that are associated with this solution and will purge all data related to those objects and types. It will also remove all solution metadata (including, but not limited to, subscriptions and other related objects).\nProceed with caution! \nPlease type the name of the solution you want to delete and hit enter confirm that you want to delete the solution with name: %s and tag: %s \n", solutionName, solutionTag)
fmt.Printf("WARNING! This command will remove all objects and types that are associated with this solution and will purge all data related to those objects and types. It will also remove all solution metadata (including, but not limited to, subscriptions and other related objects).\nProceed with caution! \nPlease type the name of the solution you want to delete and hit enter confirm that you want to delete the solution with name: %s and tag: %s \n", solutionName, solutionTag)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

Comment on lines 125 to 126
if !noWait {
output.PrintCmdStatus(cmd, fmt.Sprintf("Solution deletion initiated for solution with name: %s and tag: %s\n", solutionName, solutionTag))
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
if !noWait {
output.PrintCmdStatus(cmd, fmt.Sprintf("Solution deletion initiated for solution with name: %s and tag: %s\n", solutionName, solutionTag))
output.PrintCmdStatus(cmd, fmt.Sprintf("Solution deletion initiated for solution with name: %s and tag: %s\n", solutionName, solutionTag))
if !noWait {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

Comment on lines 148 to 150
} else {
output.PrintCmdStatus(cmd, "Solution deletion initiated, skip waiting for transaction to complete")
}
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
} else {
output.PrintCmdStatus(cmd, "Solution deletion initiated, skip waiting for transaction to complete")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

for deletionObjData.Status == "" || deletionObjData.Status == "inProgress" || deletionObjData.IsEmpty() || newDeletionObjectId == existingSolutionDeletionObjectId {
output.PrintCmdStatus(cmd, fmt.Sprintf("Waited %f seconds for solution with name: %s and tag: %s to be marked as deleted\n", time.Since(waitStartTime).Seconds(), solutionName, solutionTag))
if time.Since(waitStartTime).Seconds() > float64(waitForDeletionDuration) {
log.Fatalf("Failed to validate solution with name %s and tag: %s was deleted: timed out", solutionName, solutionTag)
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
log.Fatalf("Failed to validate solution with name %s and tag: %s was deleted: timed out", solutionName, solutionTag)
log.Fatalf("Timed out waiting for solution with name %s and tag: %s to be deleted. Deletion continues, please check status for outcome.", solutionName, solutionTag)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

if deletionObjData.Status == "successful" {
output.PrintCmdStatus(cmd, fmt.Sprintf("Solution with name: %s and tag: %s deleted successfully", solutionName, solutionTag))
} else {
output.PrintCmdStatus(cmd, fmt.Sprintf("Issue deleting solution with name: %s and tag %s. Error message: %s", solutionName, solutionTag, deletionObjData.DeleteMessage))
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
output.PrintCmdStatus(cmd, fmt.Sprintf("Issue deleting solution with name: %s and tag %s. Error message: %s", solutionName, solutionTag, deletionObjData.DeleteMessage))
output.PrintCmdStatus(cmd, fmt.Sprintf("Failed to delete solution with name: %s and tag %s. Error message: %s", solutionName, solutionTag, deletionObjData.DeleteMessage))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

deletionObj := getSolutionDeletionObject(solutionTag, solutionName)
deletionObjData = deletionObj.DeletionData
newDeletionObjectId = deletionObj.ID
log.Infof("Got value of new deletion objectID :%s", newDeletionObjectId)
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
log.Infof("Got value of new deletion objectID :%s", newDeletionObjectId)

Do you still need this? OK if yes, just let's use WithField or similar and reflect not only the objectID but maybe the status

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

@pnickolov pnickolov merged commit 381d774 into cisco-open:main Mar 27, 2024
8 checks passed
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.

None yet

3 participants