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

I245 add suport for forceful project deletion #536

Conversation

MaheshPunjabi
Copy link
Contributor

What was changed?
from #245

  • api: projects_force_delete
    Add an API extension string for this in doc/api-extensions.md and internal/version/api.go. call it "projects_force_delete"

Work-In-Progress:

  • Update the projectDelete API handler,in "cmd/incusd/api_project.go" to handle the force variable and if set to true, then go through everything the project owns and delete it. You probably want to start with instances, then volumes and buckets, then profiles, then networks (starting with acls, zones, peering before networks themselves).

  • Re-generate the swagger YAML data (make update-api).

  • Add a DeleteProjectForce function to "client/incus_projects.go" which passes a ?force=true to the API. add that function to the ImageServer interface.

  • Add new "force" argument to "cmd/incus/project.go". when passed, call DeleteProjectForce

  • Re-generate the translation template for the new CLI string.


Tests:
gofmt -d passed(for api.go)
make completed fine

@github-actions github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Feb 26, 2024
@stgraber stgraber marked this pull request as draft February 26, 2024 03:48
@stgraber
Copy link
Member

stgraber commented Mar 9, 2024

@MaheshPunjabi how's this going? Need any help?

@MaheshPunjabi
Copy link
Contributor Author

MaheshPunjabi commented Mar 9, 2024

@MaheshPunjabi how's this going? Need any help?
@stgraber Yes, thank you. I've posted a couple of questions on line 1570, 71 and 91.
Also, I'm still trying to figure out the step "Empty the default profile" . Should I just call projectCreateDefaultProfile ?

@stgraber
Copy link
Member

Ah, in the future please use messages in the PR directly or use Github's in-line comment feature as we don't normally review the code within draft PRs so will not notice code comments like the ones you've left.

cmd/incusd/api_project.go Outdated Show resolved Hide resolved
cmd/incusd/api_project.go Outdated Show resolved Hide resolved
cmd/incusd/api_project.go Outdated Show resolved Hide resolved
cmd/incusd/api_project.go Outdated Show resolved Hide resolved
cmd/incusd/api_project.go Outdated Show resolved Hide resolved
cmd/incusd/api_project.go Outdated Show resolved Hide resolved
@MaheshPunjabi
Copy link
Contributor Author

@stgraber Can you take a quick look at the changes in recent commits to see if they look good? I'm doubtful on the "Empty the default profile" part.

@stgraber
Copy link
Member

I think the DB code to empty the default profile should work. That said, I don't think it's a good idea to directly poke the database as in many other cases (networks, storage, instance, ...) there is more to Incus than just some DB records and this will not properly clean things.

A more reliable approach would be to use our own Go API to perform those actions, effectively acting as if the incus command line tool was issuing those commands.

Basically doing something like:

s, err := incus.ConnectIncusUnix("", nil)
if err != nil {
    return err
}

And then using s to call things like DeleteInstance, DeleteProfile, UpdateProfile (to empty the default profile, DeleteStoragePoolVolume, DeleteStoragePoolBucket`, ...

You can compute the actual list of stuff to delete based on a GetProject call and looking at the UsedBy field. This also makes it possible to immediately fail if there are things in UsedBy that the function doesn't yet know how to delete (future proofing).

@stgraber
Copy link
Member

Going to take this one and the work done in #850 to get something we can merge in 6.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes to the REST API Documentation Documentation needs updating
Development

Successfully merging this pull request may close these issues.

None yet

2 participants