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

Debugging Volume Creation Post-Reboot through Enhanced Logging #3666

Merged

Conversation

OhmSpectator
Copy link
Member

This pull request encompasses a series of commits aimed at addressing a potential issue with volume creation following a system reboot. These changes primarily revolve around enhancing logging mechanisms in various modules, with a particular focus on the volumemgr module. By improving the clarity and detail of logs, this PR seeks to facilitate easier identification and resolution of issues related to volume management post-reboot.

…ules.

This commit introduces additional logging warnings in the baseosmgr,
downloader, volumemgr, and related modules. The added warnings are triggered
when there is an unexpected change in the total size of a blob, specifically
when the total size is altered from a non-zero value to a different non-zero
value.

Signed-off-by: Nikolay Martyanov <nikolay@zededa.com>
This commit adds a log entry in the `CtrGetImage` function to record exceptions
encountered during the verification of the containerd client. Additionally, it
introduces warning logs in the `verifyCtr` function to report when the
Containerd context is in an error state. These enhancements aim to improve the
visibility and traceability of errors related to client verification and
context handling in containerd operations.

Signed-off-by: Nikolay Martyanov <nikolay@zededa.com>
This commit introduces multiple logging enhancements across various functions
in the volumemgr module to improve the tracking of ContentFormat setting in
VolumeStatus.

Key changes include:

- Logging the setting of ContentFormat in handleDeferredVolumeCreate(),
  indicating when the content format is set based on the volume's key.

- Adding logs in populateExistingVolumesFormatObjects() to record the saving of
  volume format for each volume.

- In getVolumeStatusByLocation(), enhanced logging is added to parse the format
  from the location, indicate the found format, and log the format as a digit.
  Additionally, a log entry is added to confirm the found volume and its content
  format.

- doUpdateVol() now includes logs for creating a blank volume with a specific
  format and setting the VolumeStatus.ContentFormat based on the ContentTree.

These enhancements aim to provide clearer insights into the process of setting
and updating the ContentFormat in VolumeStatus, facilitating better debugging
and monitoring of volume management operations.

Signed-off-by: Nikolay Martyanov <nikolay@zededa.com>
@OhmSpectator OhmSpectator requested review from deitch and eriknordmark and removed request for rvs and eriknordmark December 13, 2023 14:54
Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, I wish we did not have to do this.

@OhmSpectator
Copy link
Member Author

OhmSpectator commented Dec 13, 2023

Ugh, I wish we did not have to do this.

I am interested in the concept of simplified debugging through the decoupling of Pillar. How exactly do you see it?
I hope it's a proper place to discuss it...

@deitch
Copy link
Contributor

deitch commented Dec 13, 2023

It isn't the right place to discuss it. Let's set up a conversation around it offline.

@OhmSpectator
Copy link
Member Author

Let's set up a conversation around it offline.

I would prefer to have it documented somewhere... That's why I thought it could be a good place. But ok, let's discuss it in any other way you prefer =)

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine with me.
An alternative would be to move some of these (like the change to TotalSize from some non-zero value) into the LogModify function for VolumeStatus, but that wouldn't capture which code did the modification. And it wouldn't help with tracking down how ContentType is set/changed.
So let's go with these changes as is.

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

Successfully merging this pull request may close these issues.

3 participants