Skip to content

Verify package remove does what it says on the tin #3639

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

Open
mkcp opened this issue Apr 7, 2025 · 0 comments
Open

Verify package remove does what it says on the tin #3639

mkcp opened this issue Apr 7, 2025 · 0 comments
Assignees
Labels
documentation 📘 Improvements or additions to documentation possible-bug 🐛

Comments

@mkcp
Copy link
Contributor

mkcp commented Apr 7, 2025

Describe what should be investigated or refactored

We've gotten reports from the community that zarf package remove can leave behind orphaned state and occasionally has unexpected behavior. Before adding new features to the operation, we should investigate how it handles existing resources, potential issues like slowdowns, timeouts, and retries, and how complete it is at cleaning up an existing package deployment.

Links to any relevant code

Docs

Removes a Zarf package that has been deployed already (runs offline). Remove reverses the deployment order, the last component is removed first.

Cmd

zarf/src/cmd/package.go

Lines 659 to 710 in 36c06e2

func newPackageRemoveCommand(v *viper.Viper) *cobra.Command {
o := &packageRemoveOptions{}
cmd := &cobra.Command{
Use: "remove { PACKAGE_SOURCE | PACKAGE_NAME } --confirm",
Aliases: []string{"u", "rm"},
Args: cobra.MaximumNArgs(1),
Short: lang.CmdPackageRemoveShort,
Long: lang.CmdPackageRemoveLong,
PreRun: o.preRun,
RunE: o.run,
ValidArgsFunction: getPackageCompletionArgs,
}
cmd.Flags().BoolVar(&config.CommonOptions.Confirm, "confirm", false, lang.CmdPackageRemoveFlagConfirm)
_ = cmd.MarkFlagRequired("confirm")
cmd.Flags().StringVar(&pkgConfig.PkgOpts.OptionalComponents, "components", v.GetString(VPkgDeployComponents), lang.CmdPackageRemoveFlagComponents)
cmd.Flags().BoolVar(&pkgConfig.PkgOpts.SkipSignatureValidation, "skip-signature-validation", false, lang.CmdPackageFlagSkipSignatureValidation)
return cmd
}
func (o *packageRemoveOptions) preRun(_ *cobra.Command, _ []string) {
// If --insecure was provided, set --skip-signature-validation to match
if config.CommonOptions.Insecure {
pkgConfig.PkgOpts.SkipSignatureValidation = true
}
}
func (o *packageRemoveOptions) run(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
packageSource, err := choosePackage(ctx, args)
if err != nil {
return err
}
filter := filters.Combine(
filters.ByLocalOS(runtime.GOOS),
filters.BySelectState(pkgConfig.PkgOpts.OptionalComponents),
)
cluster, _ := cluster.NewCluster() //nolint:errcheck
removeOpt := packager2.RemoveOptions{
Source: packageSource,
Cluster: cluster,
Filter: filter,
SkipSignatureValidation: pkgConfig.PkgOpts.SkipSignatureValidation,
PublicKeyPath: pkgConfig.PkgOpts.PublicKeyPath,
}
err = packager2.Remove(ctx, removeOpt)
if err != nil {
return err
}
return nil

Packager2 remove

https://github.com/zarf-dev/zarf/blob/main/src/internal/packager2/remove.go#L36-L167

og packager remove

https://github.com/zarf-dev/zarf/blob/main/src/pkg/packager/remove.go#L33

Additional context

While verifying the behavior of the remove operation, there's a good opportunity to add in quality of life improvements like debug logging (with duration) and additional doc comments on exported fields (like usage info on the options struct).

Relates to:
#3525

@mkcp mkcp added this to Zarf Apr 7, 2025
@mkcp mkcp moved this to Backlog in Zarf Apr 7, 2025
@mkcp mkcp added documentation 📘 Improvements or additions to documentation possible-bug 🐛 labels Apr 7, 2025
@mkcp mkcp self-assigned this Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation 📘 Improvements or additions to documentation possible-bug 🐛
Projects
Status: Backlog
Development

No branches or pull requests

1 participant