fix(agent): No empty cloudinit #886
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File mode 0600 was not intended for this specific case, as the setup step expects to be able to list and write into the directory. It was not noticed because the directory was previously created by cloudinit.New. Not that cloudinit won't create the directory if there is nothing to write into, the bug was revealed.
If marshalConfig returns `nil, nil` there is no error but also nothing for cloud-init to do. Thus, there should be no agent.yaml in disk. So, we check for that condition and attempt to remove agent.yaml from the cloudinit dataDir, if it exists. Missing file is not an error.
We previously didn't bother with the contents of agent.yaml But now that it will only exist after the registry notifications reach the cloudinit component, we need to wait for that file to appear before proceeding with any assertion. And now we no longer expect to find an empty file, but a file that matches what was set via the registry.
Using the existence of goldens as the way to signal the test cases that agent.yaml should exist or not.
CarlosNihelton
changed the title
|
This is the same as #881 , but to merge on main. I merged that PR on the wrong base branch by mistake 😅 |
didrocks
approved these changes
Aug 28, 2024
|
Indeed, I just restored the branch |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the config is empty, the yaml serialization outputs {}. We still added some commentary in the agent.yaml, resulting in a bunch of nothing that confused cloud-init, because it's enablement is based on the existence of the files, their contents are only checked later. That wouldn't be a problem per se, as the WSL datasource would recognize there's nothing to do and bail out, but that would have costed unnecesary CPU cicles, and worse than that, the current implementation of cloud-init makes it log as an error the fact that there is nothing to do at this point, which results in more noise if the user see the output of cloud-init status --long.
The fix is rather simple: do not write an empty object into agent.yaml. If the config is empty we delete that file. This way, if there is nothing from the user nor the agent's perspective, cloud-init will remain trully disabled.
I updated some tests that relied on the fact that cloud-init would create the agent.yaml file unconditionally, which is no longer the case.
UDENG-4169