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

Check earlier for log device #78

Merged
merged 2 commits into from
May 3, 2024
Merged

Check earlier for log device #78

merged 2 commits into from
May 3, 2024

Conversation

lmbarros
Copy link
Contributor

@lmbarros lmbarros commented May 2, 2024

Previously, we'd check for the log device (passed with --log-to) only
after the user confirmed to proceed with the migration. And then, if the
device was not valid, takeover would keep running without storing the
stage 2 logs anywhere.

In other words, a mistake in the command-line arguments could cause
important logs to be lost.

With this commit, we check the the log device at a much earlier point
(before asking the user for confirmation to proceed), and if the device
provided is no good, we log an error message and exit. This allows the
user to fix the command-line arguments and run again.

@lmbarros lmbarros force-pushed the lmb/check-earlier branch 2 times, most recently from a779c71 to 7908cca Compare May 2, 2024 20:48
lmbarros added 2 commits May 2, 2024 17:49
This is in preparation for a bigger change that will come next.

Signed-off-by: Leandro Motta Barros <[email protected]>
Change-type: patch
Previously, we'd check for the log device (passed with `--log-to`) only
after the user confirmed to proceed with the migration. And then, if the
device was not valid, takeover would keep running without storing the
stage 2 logs anywhere.

In other words, a mistake in the command-line arguments could cause
important logs to be lost.

With this commit, we check the the log device at a much earlier point
(before asking the user for confirmation to proceed), and if the device
provided is no good, we log an error message and exit. This allows the
user to fix the command-line arguments and run again.

Signed-off-by: Leandro Motta Barros <[email protected]>
Change-type: minor
@lmbarros lmbarros force-pushed the lmb/check-earlier branch from 7908cca to 34febb5 Compare May 2, 2024 20:49
@lmbarros lmbarros marked this pull request as ready for review May 2, 2024 20:53
@flowzone-app flowzone-app bot enabled auto-merge May 2, 2024 20:57
@lmbarros lmbarros requested review from acostach and rahul-thakoor May 2, 2024 21:10
Copy link
Contributor

@rahul-thakoor rahul-thakoor left a comment

Choose a reason for hiding this comment

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

Tested:

  1. no log device
    2024-05-03 04:37:27 INFO Early checks passed

  2. no partition

2024-05-03 04:43:04 ERROR [takeover::stage1] The log device '/dev/sda' is not a partition, so it can't be used for stage 2 logs
2024-05-03 04:43:04 ERROR [takeover::stage1::checks] the requested log device is not suitable for writing stage2 logs
2024-05-03 04:43:04 ERROR [takeover] Migrate stage 1 returned an error: An error occurred in an upstream function, context: Failed early checks, exiting
  caused by: The error was displayed upstream
  1. partition
2024-05-03 04:44:52 DEBUG [takeover::stage1::block_device_info::partition] PartitionInfo::new: for /dev/sda1 got PartitionInfo { uuid: Some("03CF-0DFD"), block_size: Some(512), fs_type: Some("vfat"), label: None, part_uuid: None }
2024-05-03 04:44:52 DEBUG [takeover::stage1::block_device_info] found  partition '"sda1"' in '/sys/block/sda/sda1'
2024-05-03 04:44:52 DEBUG [takeover::stage1::block_device_info] new: got device: BlockDevice { name: "sda", device_num: DeviceNum { major: 8, minor: 0 }, mounted: None, parent: "None" }
2024-05-03 04:44:52 INFO  Early checks passed

@flowzone-app flowzone-app bot merged commit 513742f into master May 3, 2024
53 checks passed
@flowzone-app flowzone-app bot deleted the lmb/check-earlier branch May 3, 2024 04:46
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.

2 participants