Skip to content

Conversation

olamilekan000
Copy link
Contributor

change adds timeout to cloud init monitoring process.

fixes issue

@olamilekan000 olamilekan000 force-pushed the add-timeout-context-to-progress-monitoring branch 4 times, most recently from 9a5bf59 to 4c19c9a Compare August 25, 2025 02:06
@@ -292,6 +300,17 @@ func (a *HostAgent) emitEvent(_ context.Context, ev events.Event) {
}
}

func (a *HostAgent) emitCloudInitProgressEvent(ctx context.Context, progress *events.CloudInitProgress) {
a.statusMu.RLock()
currentStatus := a.currentStatus
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

@olamilekan000 olamilekan000 force-pushed the add-timeout-context-to-progress-monitoring branch from 4c19c9a to 38e2c38 Compare August 25, 2025 02:57
defer func() {
if cmd.Process != nil {
logrus.Debug("Cleaning up cloud-init monitoring process")
cmd.Process.Kill()
Copy link
Contributor

Choose a reason for hiding this comment

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

This code will never be executed because it won't reach here until the tail process stops in the VM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it

olalekanodukoya@lima-defaultz:/Users/olalekanodukoya/works/opensource/lima$ ps aux | grep tail
olaleka+    1704 33.3  0.0   6672  1920 pts/0    S+   06:49   0:00 grep --color=auto tail

Copy link
Contributor

Choose a reason for hiding this comment

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

confirmed that ssh process for tail killed. 👍

@olamilekan000 olamilekan000 force-pushed the add-timeout-context-to-progress-monitoring branch from 38e2c38 to 94933aa Compare August 25, 2025 05:56
Signed-off-by: olalekan odukoya <odukoyaonline@gmail.com>
@olamilekan000 olamilekan000 force-pushed the add-timeout-context-to-progress-monitoring branch from 94933aa to 3f8402c Compare August 25, 2025 05:59
@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Aug 26, 2025
Copy link
Contributor

@norio-nomura norio-nomura left a comment

Choose a reason for hiding this comment

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

LGTM! 👍🏻

However, I am concerned that the strings passed to exitReason and Warn() are the same.
Is there a mechanism for Golang to automatically optimize the use of the same string?

@olamilekan000
Copy link
Contributor Author

LGTM! 👍🏻

However, I am concerned that the strings passed to exitReason and Warn() are the same. Is there a mechanism for Golang to automatically optimize the use of the same string?

AFIK, It does. When the Go compiler sees the same string literal (e.g., "Failed to create stdout pipe for cloud-init monitoring") multiple times in the source code, it doesn’t generate a new copy of that string every time. Instead, it puts one copy of that string in the program’s read-only data segment, and every usage just points to it.

https://goplay.tools/snippet/KDjdapaDigp

Copy link
Contributor

@norio-nomura norio-nomura left a comment

Choose a reason for hiding this comment

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

Thank you for letting me know that duplicate strings are optimized. 🙏🏻
LGTM!

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 1262923 into lima-vm:master Aug 27, 2025
36 checks passed
@norio-nomura
Copy link
Contributor

Thanks! 🙏🏻

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.

Add timeout to cloud-init progress monitoring
3 participants