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

std::Directory can not require its children #459

Open
edvgui opened this issue Sep 20, 2023 · 6 comments
Open

std::Directory can not require its children #459

edvgui opened this issue Sep 20, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@edvgui
Copy link
Contributor

edvgui commented Sep 20, 2023

I am facing a mindbreaking error when trying to delete a directory /a/b before deleting the directory /a. It seems that an implicit dependency makes the children always require the parent.

(edvgui_project) guillaume@pavilion:~/Documents/edvgui_project$ inmanta -vvX export
inmanta.module           WARNING Setting a pip index through the `repo -> url` option with type `package` in the project.yml file is deprecated. Please set the pip index url through the `pip -> index_urls` option instead.
inmanta.module           INFO    verifying project
inmanta.module           WARNING Module yum is present in requires but it is not used by the model.
inmanta.module           WARNING Module exec is present in requires but it is not used by the model.
inmanta.module           INFO    The following modules are currently installed:
inmanta.module           INFO    V2 modules:
inmanta.module           INFO      ip: 1.2.16
inmanta.module           INFO      net: 1.0.18
inmanta.module           INFO      std: 4.1.7
inmanta.execute.schedulerINFO    Total compilation time 0.017113
Traceback (most recent call last):
  File "/home/guillaume/.virtualenvs/edvgui_project/lib/python3.9/site-packages/inmanta/app.py", line 776, in app
    options.func(options)
  File "/home/guillaume/.virtualenvs/edvgui_project/lib/python3.9/site-packages/inmanta/app.py", line 599, in export
    results = export.run(
  File "/home/guillaume/.virtualenvs/edvgui_project/lib/python3.9/site-packages/inmanta/export.py", line 395, in run
    self._validate_graph()
  File "/home/guillaume/.virtualenvs/edvgui_project/lib/python3.9/site-packages/inmanta/export.py", line 319, in _validate_graph
    find_cycle(res, set())
  File "/home/guillaume/.virtualenvs/edvgui_project/lib/python3.9/site-packages/inmanta/export.py", line 313, in find_cycle
    raise e
  File "/home/guillaume/.virtualenvs/edvgui_project/lib/python3.9/site-packages/inmanta/export.py", line 310, in find_cycle
    find_cycle(dep, working)
  File "/home/guillaume/.virtualenvs/edvgui_project/lib/python3.9/site-packages/inmanta/export.py", line 313, in find_cycle
    raise e
  File "/home/guillaume/.virtualenvs/edvgui_project/lib/python3.9/site-packages/inmanta/export.py", line 310, in find_cycle
    find_cycle(dep, working)
  File "/home/guillaume/.virtualenvs/edvgui_project/lib/python3.9/site-packages/inmanta/export.py", line 306, in find_cycle
    raise DependencyCycleException(current)
inmanta.export.DependencyCycleException: Cycle in dependencies: [std::Directory[diplodocus,path=/home/rocky/.config/cloud-at-home/nginx],v=141, std::Directory[diplodocus,path=/home/rocky/.config/cloud-at-home],v=141]

I get this with the following model:

import ip

diplo = ip::Host(
    name="diplodocus",
    os=std::linux,
    ip="192.168.1.52",
    remote_user="rocky",
    remote_agent=true,
)

config_directory = std::Directory(
    path=f"/home/rocky/.config/cloud-at-home",
    mode=777,
    owner="rocky",
    group="rocky",
    host=diplo,
    reload=true,
    send_event=true,
    purged=true,
)
std::Directory(
    path=f"{config_directory.path}/nginx",
    mode=777,
    owner="rocky",
    group="rocky",
    host=diplo,
    reload=true,
    send_event=true,
    purged=true,
    provides=config_directory
)

This is driving me nuts.

@edvgui
Copy link
Contributor Author

edvgui commented Sep 20, 2023

I just discovered where it is coming from:

def dir_before_file(model, resources):

But I really don't see why this should be the case. This is only valid when creating directories/files. Deleting a child dir before its parent is indeed not required, but imo it should not be impossible.

@edvgui
Copy link
Contributor Author

edvgui commented Sep 21, 2023

I think we should either:

  1. Remove this logic, the model user is supposed to handle the resource wiring, why not there?
  2. Update the logic to detect loops directly in there and raise an exception. It is hard enough to find the root cause of a loop, let alone when the root cause comes from a piece of code you didn't even know existed and is not even run in the same context as the model.
  3. Update the logic for the "purge" scenario

I ordered those proposals by order of preference (1 is my favorite)

@bartv
Copy link
Contributor

bartv commented Sep 21, 2023

  1. is breaking

@edvgui
Copy link
Contributor Author

edvgui commented Sep 21, 2023

  1. is breaking

I know, still my favorite. But this is why I am making other proposals.

@sanderr sanderr added the bug Something isn't working label Sep 21, 2023
@sanderr
Copy link
Contributor

sanderr commented Sep 21, 2023

I too am in favor of option 1, even if it's breaking. inmanta/inmanta-core#5677 shows that it is very difficult to get it right, and it is in all other cases considered to be the responsibility of the model developer anyway. I think we should try to fix this before the iso7 release, because it's a breaking change and it fits well with inmanta/inmanta-core#5677.

@sanderr
Copy link
Contributor

sanderr commented Oct 24, 2023

The main difficulty with purge-on-delete (linked above) was with cross-agent dependencies. That does not apply here so my previous comment is not necessarily accurate. It may be better to just fix the specific bug and leave the rest in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants