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

Appending to a file is not idempotent #642

Open
bgilbert opened this issue Oct 10, 2018 · 3 comments
Open

Appending to a file is not idempotent #642

bgilbert opened this issue Oct 10, 2018 · 3 comments

Comments

@bgilbert
Copy link
Contributor

Bug

Operating System Version

Any

Ignition Version

0.28.0

Environment

Any

Expected Behavior

If storage.files[].append is true, the contents are appended exactly once.

Actual Behavior

In the presence of failures, the contents can be appended multiple times.

Reproduction Steps

On Container Linux:

{
  "ignition": {
    "version": "2.2.0"
  },
  "storage": {
    "files": [
      {
        "filesystem": "root",
        "path": "/foo",
        "append": true,
        "contents": {
          "source": "data:,hello%0A"
        },
        "mode": 420
      },
      {
        "filesystem": "root",
        "path": "/bar",
        "append": true,
        "contents": {
          "source": "https://httpbin.org/status/404,404,404,200"
        },
        "mode": 420
      }
    ]
  }
}

Other Information

On Container Linux, if Ignition fails, the machine will automatically reboot after 5 minutes and Ignition will rerun. With the above config, fetching /bar will fail 75% of the time but will eventually succeed, at which point /foo will contain one line per attempt.

We can fix this by appending to files in two passes:

  1. Before the first time we append to a particular file in the files stage, check if file.ignition-orig exists. If so, overwrite file with file.ignition-orig before appending to file. If not, copy file to file.ignition-orig.
  2. At the end of the files stage, delete the .ignition-orig files.

That approach appropriates *.ignition-orig as private namespace for Ignition. One alternative is to create an .ignition-temp directory at the root of the filesystem and put the backup files there.

@jlebon
Copy link
Member

jlebon commented Oct 10, 2018

Neat, I didn't know about https://httpbin.org/.

There's a potential optimization we could do on OSTree-based systems for files in /etc: there's already a backup of the shipped file in /usr/etc. Though there are some instance-specific files that don't have such a backup, e.g. /etc/fstab. But we could easily detect those by the fact that they're not in /usr/etc.

I'll just also mention that making append operations idempotent is also relevant for other software that handle Ignition configs like the machine config daemon (though right now it just sorta assumes that it won't be handed a config that asks for appending files).

jlebon added a commit to jlebon/machine-config-operator that referenced this issue Oct 10, 2018
It's inherently non-idempotent. See upstream Ignition issue on this:
coreos/ignition#642
@ajeddeloh
Copy link
Contributor

@bgilbert The proposed fix would help, but wouldn't completely fix the problem. If something fails during or after deleting the .orig files then we have the same problem.

@jlebon we'd need to bake the logic into ignition to append from /usr/etc to /etc but just on ostree systems. I'm not fanatical about that. I'd rather just redeploy the root if Ignition failed. It also still leaves us with the same problem on /var or any secondary filesystems.

@bgilbert
Copy link
Contributor Author

The assumption was that nothing would fail after the commit point. The approach isn't bulletproof, but would at least address Ignition config problems and transient network conditions.

I agree that the ostree-specific logic doesn't seem worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants