feat: capture aks-node-controller errors into Guest Agent Events by Devinwong · Pull Request #7773 · Azure/AgentBaker

Devinwong · 2026-02-03T00:47:21Z

What this PR does / why we need it:
capture aks-node-controller errors into Guest Agent Events

Which issue(s) this PR fixes:

Fixes #

Copilot

Pull request overview

This PR adds Guest Agent event logging to the aks-node-controller wrapper script to capture both successful completions and errors for monitoring and telemetry purposes.

Changes:

Added createGuestAgentEvent function to generate Guest Agent events in JSON format
Events are now created for both successful completion and error cases
Added test coverage for error scenario using ShellSpec

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
parts/linux/cloud-init/artifacts/aks-node-controller-wrapper.sh	Implements Guest Agent event creation with a new `createGuestAgentEvent` function and calls it for both success and failure cases
spec/parts/linux/cloud-init/artifacts/aks_node_controller_wrapper_spec.sh	Adds test case to verify Guest Agent event is created on non-zero exit with correct TaskName, EventLevel, and Message

parts/linux/cloud-init/artifacts/aks-node-controller-wrapper.sh

spec/parts/linux/cloud-init/artifacts/aks_node_controller_wrapper_spec.sh

parts/linux/cloud-init/artifacts/aks-node-controller-wrapper.sh

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

- Make GuestAgentEvent and ReadEvents public for test assertions - Add CreateEventFunc type with NewCreateEventFunc factory for DI - Inject events directory via App struct instead of hardcoded path - Add TestApp helper struct for cleaner test setup - Simplify tests using ReadEvents helper

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Copilot · 2026-02-07T03:12:12Z

aks-node-controller/app.go

fmt.Sprintf("... %s", err) uses the %s verb with an error value, which formats as %!s(<type=value>) rather than the error message. Use %v or err.Error() so the Guest Agent Event contains the real error text.

Suggested change

message := fmt.Sprintf("aks-node-controller exited with error %s", err)

message := fmt.Sprintf("aks-node-controller exited with error %v", err)

Copilot · 2026-02-07T03:12:13Z

aks-node-controller/app.go

+	startTime := time.Now()
+	a.createEvent(cmd.taskName, "Starting", helpers.EventLevelInformational, startTime, startTime)
+


a.createEvent(...) is called unconditionally. If an App instance is constructed without createEvent set (e.g., in future tests/utility code), this will panic at runtime. Consider defaulting createEvent to a no-op in Run/run (or in an App constructor) when it is nil.

- Add EventLogger struct with LogEvent and Events methods - LogEvent writes events, Events reads them (for testing) - Cleaner API than function type with separate ReadEvents

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

aks-node-controller/helpers/guestagent.go

Copilot · 2026-02-07T07:27:40Z

aks-node-controller/helpers/guestagent.go

+	// Use nanosecond timestamp as filename, based on current time to ensure uniqueness
+	// This provides better collision avoidance than milliseconds
+	eventsFileName := fmt.Sprintf("%d.json", time.Now().UnixNano())
+	eventFilePath := filepath.Join(l.Dir, eventsFileName)


Using time.Now().UnixNano() as the filename is not guaranteed to be unique (clock resolution can be coarser than 1ns, and values can repeat), so back-to-back events can collide and overwrite each other, losing telemetry. Consider using an atomic counter/loop with O_EXCL, os.CreateTemp, or adding an additional uniqueness component (pid/random) to the filename.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Copilot · 2026-02-09T05:55:54Z

aks-node-controller/app_test.go

+	exitCode := tt.App.Run(context.Background(), []string{"aks-node-controller", "provision", "--provision-config=parser/testdata/test_aksnodeconfig.json", "--dry-run"})
+	assert.Equal(t, 0, exitCode)
+	if reflect.ValueOf(tt.App.cmdRun).Pointer() != reflect.ValueOf(cmdRunnerDryRun).Pointer() {
 		t.Fatal("app.cmdRunner is expected to be cmdRunnerDryRun")


The failure message here still refers to app.cmdRunner, but the field was renamed to cmdRun. Updating the message will make test failures clearer and avoid confusion during future refactors.

Suggested change

t.Fatal("app.cmdRunner is expected to be cmdRunnerDryRun")

t.Fatal("app.cmdRun is expected to be cmdRunnerDryRun")

Copilot · 2026-02-09T05:55:54Z

aks-node-controller/helpers/guestagent.go

+	// Use nanosecond timestamp as filename, based on current time to ensure uniqueness
+	// This provides better collision avoidance than milliseconds
+	eventsFileName := fmt.Sprintf("%d.json", time.Now().UnixNano())
+	eventFilePath := filepath.Join(l.Dir, eventsFileName)


LogEvent uses time.Now().UnixNano() alone as the event filename. This is not guaranteed to be unique on all platforms/VMs (clock resolution can be coarser than 1ns), and concurrent/rapid successive calls can overwrite an earlier event file, losing telemetry and causing test flakiness. Consider generating filenames via os.CreateTemp (or add a monotonic counter/O_EXCL retry loop) to guarantee uniqueness.

Copilot · 2026-02-09T05:55:55Z

aks-node-controller/app.go

+	startTime := time.Now()
+	a.eventLogger.LogEvent(cmd.taskName, "Starting", helpers.EventLevelInformational, startTime, startTime)
+
+	err := cmd.handler(a, ctx, args)
+	endTime := time.Now()
+	if err != nil {
+		message := fmt.Sprintf("aks-node-controller exited with error %s", err.Error())
+		a.eventLogger.LogEvent(cmd.taskName, message, helpers.EventLevelError, startTime, endTime)
+	} else {
+		a.eventLogger.LogEvent(cmd.taskName, "Completed", helpers.EventLevelInformational, startTime, endTime)
 	}


run() emits a "Starting" guest agent event and then a second event for "Completed"/error. Existing guest-agent event emitters in parts/linux/cloud-init/artifacts/ generally emit a single event per operation (Timestamp=startTime, OperationId=endTime). Emitting two events per command increases event volume and also makes it hard to correlate start/end because OperationId will differ between the two events. If the goal is to capture errors, consider only emitting an event on failure (or use a single event emitted at the end with start/end timing in the Message).

djsly · 2026-02-09T19:43:33Z

aks-node-controller/main.go

-	app := App{cmdRunner: cmdRunner}
+	app := App{
+		cmdRun:      cmdRunner,
+		eventLogger: helpers.NewEventLogger("/var/log/azure/Microsoft.Azure.Extensions.CustomScript/events"),


could we make this not hardcoded and instead come from the service config ? this file won't exists on non Azure VM

We create the file. And ignore any errors in the process.

There are plenty of hardcoded values in our scripts. Making everything configurable can be harder to track and read. Is there a place where we want to change it?

I thought the path is outside of our control.

feat: capture aks-node-controller errors into

cc49f41

Copilot AI review requested due to automatic review settings February 3, 2026 00:47

Devinwong requested review from AbelHu, awesomenix, cameronmeissner, djsly, ganeshkumarashok, juan-lee, junjiezhang1997, lilypan26, mxj220, pdamianov-dev, phealy, r2k1, timmy-wright and zachary-bailey as code owners February 3, 2026 00:47

Devinwong temporarily deployed to test February 3, 2026 00:47 — with GitHub Actions Inactive

Copilot started reviewing on behalf of Devinwong February 3, 2026 00:48 View session

add shellspec guard

057acc2

Devinwong temporarily deployed to test February 3, 2026 00:52 — with GitHub Actions Inactive

Copilot AI reviewed Feb 3, 2026

View reviewed changes

update

34bd4ec

Devinwong temporarily deployed to test February 3, 2026 00:58 — with GitHub Actions Inactive

update

f8b06ce

Copilot AI review requested due to automatic review settings February 3, 2026 01:00

Devinwong temporarily deployed to test February 3, 2026 01:00 — with GitHub Actions Inactive

Copilot started reviewing on behalf of Devinwong February 3, 2026 01:00 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

add bash in UT

2f6530d

Devinwong temporarily deployed to test February 3, 2026 01:09 — with GitHub Actions Inactive

Copilot started reviewing on behalf of Devinwong February 6, 2026 03:27 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

move event generation from main to app

2f7d444

r2k1 temporarily deployed to test February 6, 2026 19:53 — with GitHub Actions Inactive

Copilot AI review requested due to automatic review settings February 7, 2026 03:08

r2k1 temporarily deployed to test February 7, 2026 03:08 — with GitHub Actions Inactive

Copilot started reviewing on behalf of r2k1 February 7, 2026 03:08 View session

Copilot AI reviewed Feb 7, 2026

View reviewed changes

refactor: replace CreateEventFunc with EventLogger struct

d0b2479

- Add EventLogger struct with LogEvent and Events methods - LogEvent writes events, Events reads them (for testing) - Cleaner API than function type with separate ReadEvents

r2k1 temporarily deployed to test February 7, 2026 03:27 — with GitHub Actions Inactive

Devinwong enabled auto-merge (squash) February 7, 2026 05:18

simplify testing, log on continue on errors from Events method

3150963

Copilot AI review requested due to automatic review settings February 7, 2026 07:23

r2k1 temporarily deployed to test February 7, 2026 07:23 — with GitHub Actions Inactive

Copilot started reviewing on behalf of r2k1 February 7, 2026 07:23 View session

r2k1 approved these changes Feb 7, 2026

View reviewed changes

rename cmdRunner => cmdRun

5c641b5

r2k1 temporarily deployed to test February 7, 2026 07:25 — with GitHub Actions Inactive

Copilot AI reviewed Feb 7, 2026

View reviewed changes

r2k1 approved these changes Feb 7, 2026

View reviewed changes

Update aks-node-controller/helpers/guestagent.go

7adebf2

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 9, 2026 05:51

Devinwong temporarily deployed to test February 9, 2026 05:51 — with GitHub Actions Inactive

Copilot started reviewing on behalf of Devinwong February 9, 2026 05:51 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

awesomenix approved these changes Feb 9, 2026

View reviewed changes

Devinwong merged commit 392281e into main Feb 9, 2026
34 of 35 checks passed

Devinwong deleted the devinwon/controller-guest-agent-Event branch February 9, 2026 18:47

djsly reviewed Feb 9, 2026

View reviewed changes

	message := fmt.Sprintf("aks-node-controller exited with error %s", err)
	message := fmt.Sprintf("aks-node-controller exited with error %v", err)

		startTime := time.Now()
		a.createEvent(cmd.taskName, "Starting", helpers.EventLevelInformational, startTime, startTime)

	t.Fatal("app.cmdRunner is expected to be cmdRunnerDryRun")
	t.Fatal("app.cmdRun is expected to be cmdRunnerDryRun")

Conversation

Devinwong commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

djsly Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

r2k1 Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

Devinwong commented Feb 3, 2026 •

edited

Loading