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

docs: inotify behavior on atomic configMap/secret volumes is not documented #112677

Closed
ahmetb opened this issue Sep 22, 2022 · 14 comments
Closed
Labels
kind/documentation Categorizes issue or PR as related to documentation. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage.

Comments

@ahmetb
Copy link
Member

ahmetb commented Sep 22, 2022

Background:
Kubernetes automatically updates files on mounted secret and configMap volume types, as respective Secret/ConfigMap resources change.

Problem:
When someone runs a program on Kubernetes watching filesystem updates on secret and configMap volume types volumes (which is typically done using inotify(7) syscall on POSIX systems), they will be getting only "file deleted" (IN_DELETE_SELF) inotify event when the files are updated. (This is because how Kubernetes/kubelet uses AtomicWriter to make updates to these files.)

This behavior is quite different than what the program would see when it's running outside Kubernetes, since a normal filesystem would return inotify events like IN_CLOSE_WRITE (file opened for writing closed) or IN_MODIFY (file written) events. Furthermore, "file deleted" event causes the inotify monitor to break. Therefore, the code needs to handle this to re-establish the inotify watch upon deletion (otherwise it would only receive the one delete event, and nothing else).

This caveat is not documented.

Solution:
Since this behavior might break existing programs that watch files when the program is migrated to run on Kubernetes, it would be ideal to note this pitfall in the project documentation (this page?).

It currently seems that this information is tucked away in some blog posts [1], [2], [3] and would be good to surface in official documentation as well.

/kind bug
/area docs

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Sep 22, 2022
@k8s-ci-robot
Copy link
Contributor

@ahmetb: The label(s) area/docs cannot be applied, because the repository doesn't have them.

In response to this:

Background:
Kubernetes automatically updates files on mounted secret and configMap volume types, as respective Secret/ConfigMap resources change.

Problem:
When someone runs a program on Kubernetes watching filesystem updates on secret and configMap volume types volumes (which is typically done using inotify(7) syscall on POSIX systems), they will be getting only "file deleted" (IN_DELETE_SELF) inotify event when the files are updated. (This is because how Kubernetes/kubelet uses AtomicWriter to make updates to these files.)

This behavior is quite different than what the program would see when it's running outside Kubernetes, since a normal filesystem would return inotify events like IN_CLOSE_WRITE (file opened for writing closed) or IN_MODIFY (file written) events. Furthermore, "file deleted" event causes the inotify monitor to break. Therefore, the code needs to handle this to re-establish the inotify watch upon deletion (otherwise it would only receive the one delete event, and nothing else).

Solution:
Since this behavior might break existing programs that watch files when the program is migrated to run on Kubernetes, it would be ideal to note this pitfall in the project documentation (this page?).

/kind bug
/area docs

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Sep 22, 2022
@k8s-ci-robot
Copy link
Contributor

@ahmetb: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Sep 22, 2022
@ahmetb
Copy link
Member Author

ahmetb commented Sep 22, 2022

/remove-label bug
/sig docs
/kind documentation

@k8s-ci-robot k8s-ci-robot added sig/docs Categorizes an issue or PR as relevant to SIG Docs. kind/documentation Categorizes issue or PR as related to documentation. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 22, 2022
@k8s-ci-robot
Copy link
Contributor

@ahmetb: The label(s) /remove-label bug cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor, official-cve-feed

In response to this:

/remove-label bug
/sig docs
/kind documentation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@BenTheElder
Copy link
Member

Nits:

since a normal filesystem would return inotify events

It's a "normal file system", the behavior is entirely in file manipulation in ways that can happen on any filesystem. (I get this is mostly semantics, but that matters for the docs!)

Code that only triggers on IN_CLOSE_WRITE can be broken by this technique outside of Kubernetes. using rename(2) for atomic file changes is a documented use case:

One useful feature of rename is that the meaning of newname changes “atomically” from any previously existing file by that name to its new meaning (i.e., the file that was called oldname). There is no instant at which newname is non-existent “in between” the old meaning and the new meaning. If there is a system crash during the operation, it is possible for both names to still exist; but newname will always be intact if it exists at all.

https://www.gnu.org/software/libc/manual/html_node/Renaming-Files.html#Renaming-Files

https://lwn.net/Articles/789600/ has a discussion of even adding new ways for applications to do this.

I do think this is something we should mention that simple inotify probably won't work, but I'm not sure we should document the implementation details vs suggesting reliable alternatives that don't depend on the implementation (like periodically reading and diffing vs the last read).

I also wouldn't be surprised to see other systems leveraging this known-technique outside kubernetes for the same reason Kubernetes does ... to prevent the even worse pitfall of clients reading partially-rewritten content.

@BenTheElder
Copy link
Member

/sig node storage

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. labels Sep 23, 2022
@ahmetb
Copy link
Member Author

ahmetb commented Sep 23, 2022

The main problem is the class of programs reloading configuration based on the filesystem writes that are probably handling a "delete" event by printing an error saying "configuration file no longer exists, exiting" and subsequently issuing an exit() redundantly (whereas they could just attempt to retry the watch).
Maybe I am exaggerating the problem irl, but a program that's purely looking at write/update events won't get any of that on Kubernetes –and that's notable?

@BenTheElder
Copy link
Member

Maybe I am exaggerating the problem irl, but a program that's purely looking at write/update events won't get any of that on Kubernetes –and that's notable?

Agree that this is true, however pointing out that behavior is already a broken approach outside of Kubernetes, as the atomic file swap technique is general to Linux and can be done by hand even with just mv ./newconfig.yaml ./config.yaml* outside of Kubernetes.

A lot of programs probably don't handle this properly and may encounter it first on Kubernetes. I agree we should warn people, but I disagree that it's only on Kubernetes or result of a special filesystem. It's broken usage of inotify either way.

* so long as both files are on the same disk/filesystem, mv will use rename => atomic file replacement.

@SergeyKanzhelev
Copy link
Member

/remove-kind bug

@k8s-ci-robot k8s-ci-robot removed the kind/bug Categorizes issue or PR as related to a bug. label Sep 28, 2022
@sftim
Copy link
Contributor

sftim commented Dec 15, 2022

I don't think this is important to document. If you're able to do something misguided without using Kubernetes, we typically don't document that Kubernetes continues to make that possible. The list of caveats could be very very long!

If we commit to using renameat() or rename() on Linux for atomic writes and we commit to an equivalent atomic mechanism on other node OSs (Windows, and anything we support in the future), then we can and should document that commitment to atomic updates.

If that commitment is not part of defined Kubernetes conformance, I'm even more wary of documenting the detail.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 15, 2023
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 14, 2023
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

@k8s-ci-robot k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale May 14, 2023
@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Categorizes issue or PR as related to documentation. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Projects
None yet
Development

No branches or pull requests

6 participants