Bump to v2.0.8: Bootstrap VMWatch Process if vmWatchSettings present by frank-pang-msft · Pull Request #33 · Azure/applicationhealth-extension-linux

frank-pang-msft · 2023-08-18T23:48:20Z

Overview

This PR contains changes to support running VMWatch (amd64 and arm64) as an executable via goroutines and channels.

VMWatch is a standardized, lightweight, and open-sourced testing framework designed to enhance the monitoring and management of guest VMs on the Azure platform, including both 1P and 3P instances. VMWatch is engineered to collect vital health signals across multiple dimensions, which will be seamlessly integrated into Azure's quality systems. By leveraging these signals, VMWatch will enable Azure to swiftly detect and prevent regressions induced by platform updates or configuration changes, identify gaps in platform telemetry, and ultimately improve the guest experience for all Azure customers.

Behavior

VMWatch will run asynchronously as a separate process than ApplicationHealth, so the probing of application health will not be affected by the state of VMWatch. Depending on extension settings, VMWatch can be enabled/disabled, and also specify the test names and parameter overrides to VMWatch binary. The status of VMWatch will be displayed in the extension x.status files and also in GET VM Instance View. Main process will attempt to start VMWatch binary up to 3 times, after which VMWatch status will be set to failed.

Process Leaks

To ensure that VMWatch processes do not accumulate, applicationhealth-shim will be responsible for killing existing VMWatch processes by looking for processes running with the VMWatch binary names according to the architecture type. For unexpected process termination, if for some reason the main applicationhealth-extension process is terminated, we also ensure that the VMWatch process is also killed by subscribing to shutdown/termination signals in the main process, and killing the VMWatch based off process ID.

Example Binary Execution

Example execution from integration testing
SIGNAL_FOLDER=/var/log/azure/Microsoft.ManagedServices.ApplicationHealthLinux/events VERBOSE_LOG_FILE_FULL_PATH=/var/log/azure/Microsoft.ManagedServices.ApplicationHealthLinux/vmwatch.log ./var/lib/waagent/Extension/bin/VMWatch/vmwatch_linux_amd64 --config /var/lib/waagent/Extension/bin/VMWatch/vmwatch.conf --input-filter disk_io:outbound_connectivity

Release/Packaging

In addition to the arm64 or amd64 VMWatch binaries, vmwatch.conf will be expected to be present in the bin/VMWatch directory for VMWatch process to read. VMWatch will also be populating and sharing eventsFolder with ApplicationHealth, so events can be viewed in Kusto. The verbose logs of VMWatch will be written to vmwatch.log.

… commands, plus integration test template

…og file paths

…trapVMWatch

…cpu/memory usage

misc/HandlerManifest.json

main/vmWatch.go

zmyzheng

Left some comments

integration-test/test/6_vmwatch.bats

…irectory to bin/VMWatch/ + implement VMWatch process retries + update integration tests

…zure/applicationhealth-extension-linux into dev/frankpang/bootstrapVMWatch

integration-test/env/README.md

.gitignore

.github/workflows/merge_validation_v2.yml

main/cmds.go

main/schema.go

main/handlersettings.go

Hafthor

just a first pass at this review -- might try to take another look at it later.

main/vmWatch.go

… execution of process

…nd read of channel. Also only every 60 seconds

Hafthor · 2023-09-15T20:38:10Z

main/vmWatch.go

+			vmWatchErr = fmt.Errorf("%w\n Additonal Details: %+v", vmWatchErr, r)
+			ctx.Log("error", "Recovered %+v", r)
+		}
+		ctx.Log("error", fmt.Sprintf("Signaling VMWatchStatus is Failed due to reaching max of %d retries", VMWatchMaxProcessAttempts))


note that this defer func will happen whenever the executeVMWatch func exits... it's like a finally block... not a catch block. I think you want this error handling stuff to be conditionally run on if executeVMWatch does not reach the bottom of the func.. like perhaps if vmWatchErr != nil

Hafthor · 2023-09-15T20:41:07Z

main/vmWatch.go

+			err = fmt.Errorf("Error: %w\n Additonal Details: %+v", err, r)
+			ctx.Log("error", "Recovered %+v", r)
+		}
+		killVMWatch(ctx, cmd)


I don't think you want to killVMWatch on successful exit of executeVMWatchHelper func.

Hafthor · 2023-09-15T20:45:45Z

main/vmWatch.go

+
+	// VMWatch should run indefinitely, if process exists we expect an error
+	err = cmd.Wait()
+	err = fmt.Errorf("[%v][PID %d] Attempt %d: VMWatch process exited. Error: %w\nOutput: %s", time.Now().UTC().Format(time.RFC3339), pid, attempt, err, combinedOutput.String())


I would think you would want to test the return of cmd.Wait() and only fmt.Errorf wrap it when err != nil.

main/vmWatch.go

…n Handler Environment (#39) * - moved handlerenv.go and seqno.go from "github.com/Azure/azure-docker-extension/pkg/vmextension" - Added EventsFolder with other missing parameters. * -removed vmextension lib dependency from VMwatch and other Files. - Updates HandlerEnviroment.json test file. - Updated VMwatch Integration Tests. * - Bump to v2.0.8

…trapVMWatch

…33) ## Overview This PR contains changes to support running VMWatch (amd64 and arm64) as an executable via goroutines and channels. > VMWatch is a standardized, lightweight, and open-sourced testing framework designed to enhance the monitoring and management of guest VMs on the Azure platform, including both 1P and 3P instances. VMWatch is engineered to collect vital health signals across multiple dimensions, which will be seamlessly integrated into Azure's quality systems. By leveraging these signals, VMWatch will enable Azure to swiftly detect and prevent regressions induced by platform updates or configuration changes, identify gaps in platform telemetry, and ultimately improve the guest experience for all Azure customers. ## Behavior VMWatch will run asynchronously as a separate process than ApplicationHealth, so the probing of application health will not be affected by the state of VMWatch. Depending on extension settings, VMWatch can be enabled/disabled, and also specify the test names and parameter overrides to VMWatch binary. The status of VMWatch will be displayed in the extension x.status files and also in GET VM Instance View. Main process will attempt to start VMWatch binary up to 3 times, after which VMWatch status will be set to failed. ## Process Leaks To ensure that VMWatch processes do not accumulate, applicationhealth-shim will be responsible for killing existing VMWatch processes by looking for processes running with the VMWatch binary names according to the architecture type. For unexpected process termination, if for some reason the main applicationhealth-extension process is terminated, we also ensure that the VMWatch process is also killed by subscribing to shutdown/termination signals in the main process, and killing the VMWatch based off process ID. ## Example Binary Execution Example execution from integration testing ` SIGNAL_FOLDER=/var/log/azure/Microsoft.ManagedServices.ApplicationHealthLinux/events VERBOSE_LOG_FILE_FULL_PATH=/var/log/azure/Microsoft.ManagedServices.ApplicationHealthLinux/VE.RS.ION/vmwatch.log ./var/lib/waagent/Extension/bin/VMWatch/vmwatch_linux_amd64 --config /var/lib/waagent/Extension/bin/VMWatch/vmwatch.conf --input-filter disk_io:outbound_connectivity ` ## Release/Packaging In addition to the arm64 or amd64 VMWatch binaries, `vmwatch.conf` will be expected to be present in the bin/VMWatch directory for VMWatch process to read. VMWatch will also be populating and sharing eventsFolder with ApplicationHealth, so events can be viewed in Kusto. The verbose logs of VMWatch will be written to `vmwatch.log`. --------- Co-authored-by: klugorosado <142627157+klugorosado@users.noreply.github.com>

frank-pang-msft added 15 commits August 16, 2023 19:52

Bootstrapping has no integration test regressions

fe0dfd5

Add cleanup of VMWatch process during shutdown signals and upon other…

9078203

… commands, plus integration test template

Added integration tests for VMWatch

5fd629f

Linting

bbba094

Fix file vet issues

ef79852

attempt to fix handler command: install - creates the data dir

e9a1176

nit integration tests

386d170

Use handlerenvironment to dictate vmwatch signal folder and verbose l…

39821e1

…og file paths

Include missing changes in previous commit

bab9e56

Merge remote-tracking branch 'origin/master' into dev/frankpang/boots…

38448b5

…trapVMWatch

Remove unnecessary changes

e96d5e9

Try to fix docker installation error in go workflow

eafdb41

Fix integration tests

057a679

Update HandlerManifest with process names for guest agent to monitor …

2c8547b

…cpu/memory usage

Run linting

f61b8d7

frank-pang-msft marked this pull request as ready for review August 31, 2023 15:54

frank-pang-msft requested review from klugorosado and zmyzheng August 31, 2023 16:15

zmyzheng reviewed Sep 1, 2023

View reviewed changes

misc/HandlerManifest.json Outdated Show resolved Hide resolved

zmyzheng reviewed Sep 1, 2023

View reviewed changes

main/vmWatch.go Show resolved Hide resolved

frank-pang-msft added the enhancement New feature or request label Sep 1, 2023

zmyzheng reviewed Sep 1, 2023

View reviewed changes

klugorosado reviewed Sep 11, 2023

View reviewed changes

integration-test/test/6_vmwatch.bats Outdated Show resolved Hide resolved

frank-pang-msft and others added 3 commits September 12, 2023 12:56

Remove cpu/memory limits in HandlerManifest + update VMWatch binary d…

ca73b0a

…irectory to bin/VMWatch/ + implement VMWatch process retries + update integration tests

Merge branch 'master' into dev/frankpang/bootstrapVMWatch

0f6ec4e

Update test.Dockerfile

ce89552

zmyzheng approved these changes Sep 12, 2023

View reviewed changes

frank-pang-msft added 3 commits September 12, 2023 14:47

Rename workflow

c4d42b1

Merge branch 'dev/frankpang/bootstrapVMWatch' of https://github.com/A…

5488445

…zure/applicationhealth-extension-linux into dev/frankpang/bootstrapVMWatch

Add formatting & linting

19c3f0a

frank-pang-msft commented Sep 13, 2023

View reviewed changes

integration-test/env/README.md Outdated Show resolved Hide resolved

.gitignore Outdated Show resolved Hide resolved

.github/workflows/merge_validation_v2.yml Outdated Show resolved Hide resolved

main/cmds.go Outdated Show resolved Hide resolved

main/schema.go Outdated Show resolved Hide resolved

frank-pang-msft commented Sep 13, 2023

View reviewed changes

main/handlersettings.go Show resolved Hide resolved

Hafthor reviewed Sep 13, 2023

View reviewed changes

frank-pang-msft added 5 commits September 14, 2023 15:47

Minor nits

c0225cf

Update integration tests + code changes to resolve comments regarding…

a6cfc63

… execution of process

Formatting + Linting + Vet

e6025db

Add logic for recover and defer for executing VMWatch. Proper close a…

198ba22

…nd read of channel. Also only every 60 seconds

fix integration tests

726adaa

frank-pang-msft changed the title ~~Bootstrap VMWatch Process if vmWatchSettings present~~ Bump to v2.0.7: Bootstrap VMWatch Process if vmWatchSettings present Sep 15, 2023

frank-pang-msft added 2 commits September 15, 2023 13:15

Bump to v2.0.7

e6bdc4e

revert unnecessary changes to schema.go

7ce2f7f

Hafthor reviewed Sep 15, 2023

View reviewed changes

frank-pang-msft force-pushed the dev/frankpang/bootstrapVMWatch branch from e7e1129 to 7ce2f7f Compare September 15, 2023 22:29

frank-pang-msft added 2 commits September 15, 2023 15:37

Small fix to killVMWatch

688e744

Fix logic for killing VMWatch

7934366

zmyzheng reviewed Sep 18, 2023

View reviewed changes

main/vmWatch.go Outdated Show resolved Hide resolved

frank-pang-msft changed the title ~~Bump to v2.0.7: Bootstrap VMWatch Process if vmWatchSettings present~~ Bump to v2.0.8: Bootstrap VMWatch Process if vmWatchSettings present Sep 29, 2023

frank-pang-msft added 9 commits November 15, 2023 23:45

Merge remote-tracking branch 'origin/master' into dev/frankpang/boots…

8fbc6ee

…trapVMWatch

nits after merging from master

6ce2e42

feature branch should trigger workflow

d5e8c02

Extra asterisk is not necessary

982cbb7

Add back feature/**

7f7c945

Fix load directory

1e04c3f

Add container name to mk_container

6bee769

Fix naming

6ca1b85

Add back VE.RS.ION

22d82e5

frank-pang-msft changed the base branch from master to feature/v2/bootstrapVMWatch November 16, 2023 18:22

frank-pang-msft merged commit 463490d into feature/v2/bootstrapVMWatch Nov 17, 2023

Conversation

frank-pang-msft commented Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Behavior

Process Leaks

Example Binary Execution

Release/Packaging

Uh oh!

Uh oh!

Uh oh!

zmyzheng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hafthor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hafthor Sep 15, 2023

Choose a reason for hiding this comment

Uh oh!

Hafthor Sep 15, 2023

Choose a reason for hiding this comment

Uh oh!

Hafthor Sep 15, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

frank-pang-msft commented Aug 18, 2023 •

edited

Loading