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

fix: do not validate config in ddev clean, delete -O, stop -U #6209

Merged
merged 4 commits into from
May 21, 2024

Conversation

stasadev
Copy link
Member

@stasadev stasadev commented May 17, 2024

The Issue

While testing #6208 I noticed a regression in:

ddev config --project-name=d10
ddev config --project-name=d10-rename
ddev config --project-name=d10-rename2
this project root /home/stas/code/ddev/d10 already contains a project named d10. You may want to remove the existing project with "ddev stop --unlist d10"

ddev stop --unlist d10
Project 'd10-rename' was found in configured directory /home/stas/code/ddev/d10 and it is already used by project 'd10'. If you have changed the name of the project, please "ddev stop --unlist d10" 
Failed to get project(s): the d10 project has an invalid hostname: 'd10.', see https://en.wikipedia.org/wiki/Hostname#Syntax for valid hostname requirements

And you are stuck with ddev stop --unlist d10 until you manually clean ~/.ddev/project_list.yaml (or run ddev list)


Also the same problem occurs when you are working on some new project type, then you change the DDEV binary and try to delete the test project with this new type, for example, you have foobar type:

ddev delete -Oy
Failed to get project(s): the test project has an invalid app type: foobar

And there is no way to ddev delete -Oy until you fix the config.

How This PR Solves The Issue

Do not run ValidateConfig() on a command with project arguments that don't actually need validation:

  • ddev clean
  • ddev delete --omit-snapshot
  • ddev stop --unlist

The general idea is that we don't need to check the config if we delete something (I think it's annoying, just let me delete it, I don't care), and sometimes users can get stuck in a loop and we don't want that.


Allows to delete non-existent projects with ddev delete -O.

Manual Testing Instructions

ddev config --project-name=d10
ddev config --project-name=d10-rename
ddev config --project-name=d10-rename2

ddev stop --unlist d10
Project d10 is already stopped. 
Project d10 has been stopped.

ddev stop --unlist d10-rename
Project d10-rename is already stopped. 
Project d10-rename has been stopped.

# manually edit `type` to something else in .ddev/config.yaml and run
ddev delete -Oy

And check Docker volumes and the contents of ddev-global-cache for:

ddev config --project-name=d10
ddev start
ddev stop --unlist
cd ..

docker volume ls | grep d10

docker run --rm -it -v ddev-global-cache:/mnt/ddev-global-cache ddev/ddev-webserver:v1.23.1 bash
ls /mnt/ddev-global-cache/*/d10-{web,db} /mnt/ddev-global-cache/traefik/*/d10.{yaml,crt,key}

ddev delete -O d10

docker volume ls | grep d10
# should show nothing

docker run --rm -it -v ddev-global-cache:/mnt/ddev-global-cache ddev/ddev-webserver:v1.23.1 bash
ls /mnt/ddev-global-cache/*/d10-{web,db} /mnt/ddev-global-cache/traefik/*/d10.{yaml,crt,key}
# should show nothing

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@stasadev stasadev requested a review from a team as a code owner May 17, 2024 18:37
Copy link

github-actions bot commented May 17, 2024

@stasadev stasadev force-pushed the 20240517_stasadev_validate_config branch from cb6943f to 279568e Compare May 17, 2024 18:59
@stasadev
Copy link
Member Author

I'm not sure, but maybe we don't need the validation in ddev delete and ddev stop at all.
Let me know if I need to change anything.

@rfay
Copy link
Member

rfay commented May 17, 2024

I would like ddev delete to delete related volumes even if the project can't be validated or doesn't exist any more. For example, ddev delete -Oy tempproj should kill off volumes that would have been for tempproj and probably things in ddev-global-cache even if the files had long since been removed. This doesn't necessarily belong in this PR, but you asked :)

@stasadev
Copy link
Member Author

I would like ddev delete to delete related volumes even if the project can't be validated or doesn't exist any more.

Good idea and doesn't require much change, done.

@stasadev stasadev force-pushed the 20240517_stasadev_validate_config branch from 485c0fb to 1be4a49 Compare May 20, 2024 19:00
@rfay rfay force-pushed the 20240517_stasadev_validate_config branch from 1be4a49 to 10e45b0 Compare May 21, 2024 13:57
@rfay
Copy link
Member

rfay commented May 21, 2024

Rebased. I was testing and the problems mentioned in the OP are gone... because #6208 was fixed :)

@stasadev
Copy link
Member Author

Yes, that's right, now you have to manually break the config.

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

This seems like a really nice improvement, thanks! This has been a piece of friction for DDEV users for a long time. And allowing deletion of non-existing projects (getting rid of their databases and mutagen state) has been on my personal to-do list for some time.

@rfay rfay merged commit 30884ea into ddev:master May 21, 2024
13 of 18 checks passed
@stasadev stasadev deleted the 20240517_stasadev_validate_config branch May 21, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants