Broken snapshotter state on SIGINT

[sondavidb]

When calling SIGINT on the daemon, it cannot be loaded into a working state without some extra effort. This is because SIGINT triggers the cleanup flag, which wipes the content store. However, the metadata is never synced to update this state. This leaves me with two questions.

First, looking through previous issues I found #703 (comment) made note of the snapshotter removing its snapshot directories on cleanup. Could I ask for an explanation for this behavior? I would imagine that one would not want to re-pull images after restarting the daemon.

Second, could this broken state be fixed by updating the metadata before line 662 in Close() in snapshot.go? I tried to do it myself but couldn't figure out a way to get the snapshot keys. However, since cleanup wipes the container store clean regardless, as a proof of concept, I called os.Remove() right before line 662, like so:

    if err := os.Remove(filepath.Join(o.root, "metadata.db")); err != nil {
        log.G(ctx).WithError(err).Warn("failed to wipe metadata")
    }
    return o.ms.Close()

This lets the daemon start in a mostly working state, but you must still manually remove and re-pull the image, as the image is still in the containerd content store. This could probably fixed by natively interacting with the db via the metadata package. Hence, this is a pretty inelegant solution IMO.

EDIT: I found that Walk gets all the keys, so calling that then calling Remove on each of them removes them from the metadata. However, it still leaves it in the containerd content store, so we still have the same issue of being unable to re-pull the image unless you remove it manually and pull again.

I can open a PR with these changes if people want this, as it removes the need to set allow_invalid_mounts_on_restart=true to start the daemon to remove the snapshot. It's still imperfect for the above reason but it would be a nice QOL change IMO.

[ktock]

Could I ask for an explanation for this behavior? I would imagine that one would not want to re-pull images after restarting the daemon.

For avoiding leaving empty snapshot directory that looks valid but the emptiness is actually not the correct contents as the snapshots. You can use non-SIGINT signals for avoiding this behaviour.

I found that Walk gets all the keys, so calling that then calling Remove on each of them removes them from the metadata. However, it still leaves it in the containerd content store, so we still have the same issue of being unable to re-pull the image unless you remove it manually and pull again.
I can open a PR with these changes if people want this, as it removes the need to set allow_invalid_mounts_on_restart=true to start the daemon to remove the snapshot. It's still imperfect for the above reason but it would be a nice QOL change IMO.

Thanks for the suggestion. So user still need some manual operation for keeping consistency between containerd & snapshotter metadata db? It would be great if we have docs about how to fix it manually.

[sondavidb]

Thanks for the reply!

For avoiding leaving empty snapshot directory that looks valid but the emptiness is actually not the correct contents as the snapshots. You can use non-SIGINT signals for avoiding this behavior.

I understand, I'm more trying to understand if this is intentional behavior, or if this is something we would want to change.

Thanks for the suggestion. So user still need some manual operation for keeping consistency between containerd & snapshotter metadata db? It would be great if we have docs about how to fix it manually.

Correct. Assuming we want to keep current behavior of removing the snapshots on cleanup, if we update the metadata when the daemon is killed with SIGINT, the daemon can start without changing allow_invalid_mounts_on_restart in the TOML. However, the user then must remove the image if they want to re-pull it, as the containerd store will believe the image already exists, but all other functionality should be normal.

For an explanation: This behavior is caused by the cleanup functions wiping the snapshots in the stargz content store and the change not being reflected in metadata.db. This could either be fixed by either updating the metadata after removing the directories (though, as mentioned above, this is only a partial fix), or not removing the directories at all.

Based on your feedback, I'd be happy to open a PR with some changes. Currently I am looking at three options:

• Doc-only change of how to return the daemon to a working state after SIGINT.
• Update cleanup function on SIGINT to update metadata.db so that the daemon can start up in a semi-functioning state, without the need to set the TOML variable. This change will also include documentation to get daemon back to a fully working state.
• Update cleanup function on SIGINT so that directories are not removed. This would allow the daemon to get back to a fully working state on SIGINT, and thus would fully solve the issue, but would also change what appears to be intentional behavior.

Please let me know which one would be the most preferred. Apologies for the long comment. Thanks again!

[sondavidb]

Hi, just wanted to bump this issue again.

From what I've gathered, we are suggesting people to use SIGTERM to kill the daemon to avoid this problematic behavior. This keeps the uncomrpessed snapshot files on disk, as well as keeps the mounts. I think it would be best to ship a fix where SIGINT will unmount the FUSE file systems but still keep snapshot files on disk. This would imitate already existing behavior, but have the added benefit of not having to manually remove FUSE mounts.

I've created a PR at #1526 to implement this, but feel free to close/suggest modifications to this behavior. Thanks!

[wswsmao]

#1884 is the final solution

We can categorize signals into two types based on whether environment cleanup is needed.
When users send SIGINT or SIGTERM, it indicates that cleanup is required. In this case, as shown in #1884, we will clean up everything, including metadata.db.

On the other hand, if users send SIGKILL, it indicates that no cleanup is necessary. In this scenario, we will not perform any cleanup actions. After restarting the snapshotter, all information can be restored, and users will not need to re-pull the image.

[sondavidb]

Got it. In the PR I opened the SIGINT behavior would be to keep snapshots on disk unmounted, and remount upon restart, but it sounds like we prefer the behavior to be an entirely clean slate on daemon restart? (Just for my understanding — I'm not opinionated in either direction :p)