-
Notifications
You must be signed in to change notification settings - Fork 2
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
vm/vm-under-test: Move first boot to new service #56
vm/vm-under-test: Move first boot to new service #56
Conversation
Moving the snippet disabling the services to a function. Signed-off-by: Ram Lavi <[email protected]>
Moving the snippet installing packages to a function. Signed-off-by: Ram Lavi <[email protected]>
Moving the snippet disabling swap to a function. Signed-off-by: Ram Lavi <[email protected]>
f073dd8
to
ea27364
Compare
passes e2e test on a CNV4.15 cluster:
@orelmisan do we want to add a verbose print of the /proc/cmdline to the checkup's vm image so that we would be able to see if the cmdline ever breaks for any reason? |
854ab74
to
8bae30c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR @RamLavi
Looks very promising.
vms/vm-under-test/scripts/first-boot
Outdated
if systemctl --type swap list-units | grep -q '.swap'; then | ||
systemctl mask "$(systemctl --type swap list-units | grep '.swap' | awk '{print $1}')" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please give a few words in the commit message about what these commands do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DONE
Makefile
Outdated
@@ -38,7 +38,7 @@ build: | |||
-v $(PWD)/_go-cache:/root/.cache/go-build:Z \ | |||
--workdir $(PROJECT_WORKING_DIR) \ | |||
$(GO_IMAGE_NAME):$(GO_IMAGE_TAG) \ | |||
go build -v -o ./bin/kubevirt-realtime-checkup ./cmd/ | |||
go build -v -buildvcs=false -o ./bin/kubevirt-realtime-checkup ./cmd/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
The script is turning off swap space services by searching ans masking them. Currently if the "systemctl --type swap list-units" fails because there are no units with swap type then the entire script fails. Fix this by checking if there are any Units before masking them. Signed-off-by: Ram Lavi <[email protected]>
The first-boot script running by the virt-builder command is run on a service created by virt-builder. This service is guaranteed to run in the final stages of the boot process [0]. This may create a race with the checkup, that is waiting on the agentConnected condition [1] being added in order to run the checkup's executor package. The race is happening since the guest-agent service also runs during the final stages of the boot process. In order to eliminate this race, removing the first-boot script in favor of the new service, and moving the first boot script content into a new service. This service is - manually created on the customize-vm script. - explicitly scheduled to run before the guest-agent service runs. - set to run once, using ConditionPathExists unit option [2]. Before the script reboots it creates an empty file, so that the service will not run again. [0] https://www.libguestfs.org/virt-builder.1.html [1] https://github.com/kiagnose/kubevirt-realtime-checkup/blob/97b5fbe47fc5df6513671db6a40e8e31b0815580/pkg/internal/checkup/checkup.go#L144 [2] https://www.freedesktop.org/software/systemd/man/latest/systemd.unit.html#ConditionPathExists= Signed-off-by: Ram Lavi <[email protected]>
It makes sense to log the guest's kernel args for debugging purposes. Signed-off-by: Ram Lavi <[email protected]>
8bae30c
to
ecec1c7
Compare
Change: Fix comments. |
passes e2e on CNV4.15 cluster:
logs show kernel is correctly updated:
|
@RamLavi Please check it again after you merge the PR with |
When creating the vm-under-test container-disk image, the first-boot script running by the virt-builder command is run on a service created by virt-builder.
This service is guaranteed to run in the final stages of the boot process [0].
This behavior may create a race with the checkup, that is waiting on the
agentConnected condition [1] being added in order to run the checkup's
executor package.
The race is happening since the guest-agent service also runs during the final stages of the boot process.
This PR is solving this race by moving the content of the first-boot script to a new service that is guaranteed to run before the guest-agent service.
[0] https://www.libguestfs.org/virt-builder.1.html
[1]
kubevirt-realtime-checkup/pkg/internal/checkup/checkup.go
Line 144 in 97b5fbe