Ensure cleanup of IT resources and processes #968

takoverflow · 2025-02-18T05:26:34Z

What this PR does / why we need it:

Adds cleanup of test resources before running the suite
Refactored the resource cleanup logic into separate functions
Ensure that the processes started by the framework are properly killed
Removed unused container image based logic
Switch to event based validation for machineset freeze/unfreeze instead of looking at logs
Added support for virtual provider

Which issue(s) this PR fixes:
Fixes #789

Special notes for your reviewer:

Release note:

Integration test framework enhancements for resource and process cleanup

gardener-robot · 2025-02-18T05:26:42Z

@takoverflow Thank you for your contribution.

Iterates through a list of known processes for MC and MCM started by the test suite and ensures they are terminated by calling `Kill()` on them to prevent un-necessary failures due to "socket being used" by the leftover processes.

elankath · 2025-03-10T03:35:26Z

pkg/controller/machine_safety.go

@@ -330,6 +330,7 @@ func (c *controller) freezeMachineSetAndDeployment(ctx context.Context, machineS
 		}
 	}

+	c.recorder.Eventf(machineSet, "Normal", "FrozeMachineSet", "SafetyController: Froze MachineSet %s due to replica overshooting", machineSet.Name)


Could you please make constants for this FrozeMachineSet ? We have done this inconsistenly, See controller.emitRollbackNormalEven. But let us clean up the places instead ofusing literal strings. Also we should use corev1.EventTypeNormal

elankath · 2025-03-10T03:41:59Z

pkg/util/provider/machinecontroller/machine_test.go

 							Name: "aws",
 						},
 					},
 				},
 				expect: field.ErrorList{},
 			}),
 		)
+		DescribeTable("#machine validation fails with no class name",


thanks for additional missing test

elankath

Adding a release note mentioning the cleanup should be OK.

elankath

Please confirm that IT tests with these changes have been executed with at-least one real provider. Ex: machine-controller-manager-provider-aws

Only minor changes needed. Request merge after that, so one can leverage this PR for local IT against virtual provider for testing.

takoverflow · 2025-03-10T10:41:50Z

Pipeline IT targeting this PR ran successfully: AWS concourse #87

Ref: https://onsi.github.io/ginkgo/MIGRATING_TO_V2#changed-command-line-flags Ref: https://onsi.github.io/ginkgo/MIGRATING_TO_V2#improved-profiling-support

takoverflow requested a review from a team as a code owner February 18, 2025 05:26

gardener-robot added needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Feb 18, 2025

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 18, 2025

gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 18, 2025

takoverflow force-pushed the it-cleanup branch from f348656 to 168f440 Compare February 18, 2025 13:29

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 18, 2025

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 18, 2025

takoverflow force-pushed the it-cleanup branch from 168f440 to d882cc1 Compare February 20, 2025 15:40

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 20, 2025

takoverflow force-pushed the it-cleanup branch from d882cc1 to ec244f6 Compare February 21, 2025 09:59

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 21, 2025

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 21, 2025

takoverflow force-pushed the it-cleanup branch from ec244f6 to a9d56d6 Compare February 25, 2025 09:36

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 25, 2025

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 25, 2025

takoverflow added 4 commits February 26, 2025 11:51

refactor: extract cleanup fns & allow for cleanup in beforesuite

21fd746

cleanup: remove CONTAINER_IMAGE checks and related fn

830ab97

refactor: added event based machineset freeze check instead of logs

663c421

takoverflow force-pushed the it-cleanup branch from a9d56d6 to 663c421 Compare February 26, 2025 06:55

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 26, 2025

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 27, 2025

framework: add changes to support virtual provider

f3ae69f

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 3, 2025

takoverflow requested a review from aaronfern March 3, 2025 03:48

Address review comments

d610c1d

takoverflow force-pushed the it-cleanup branch from 374508e to d610c1d Compare March 3, 2025 04:55

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 3, 2025

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 3, 2025

nit: rename to a sensible name

e34f396

takoverflow force-pushed the it-cleanup branch from 662050d to e34f396 Compare March 3, 2025 05:27

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 3, 2025

elankath reviewed Mar 10, 2025

View reviewed changes

elankath requested changes Mar 10, 2025

View reviewed changes

Added constants for events, address review comments

bf357eb

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 10, 2025

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 10, 2025

bugfix: add supported argument on fetching process to kill

8024b71

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 10, 2025

chore: update ginkgo version and flags

2998539

Ref: https://onsi.github.io/ginkgo/MIGRATING_TO_V2#changed-command-line-flags Ref: https://onsi.github.io/ginkgo/MIGRATING_TO_V2#improved-profiling-support

takoverflow force-pushed the it-cleanup branch from 8b503fa to 2998539 Compare March 10, 2025 11:14

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 10, 2025

takoverflow requested a review from elankath March 11, 2025 03:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure cleanup of IT resources and processes #968

Ensure cleanup of IT resources and processes #968

takoverflow commented Feb 18, 2025 •

edited

Loading

gardener-robot commented Feb 18, 2025

elankath Mar 10, 2025

elankath Mar 10, 2025

elankath left a comment •

edited

Loading

elankath left a comment

takoverflow commented Mar 10, 2025

Ensure cleanup of IT resources and processes #968

Are you sure you want to change the base?

Ensure cleanup of IT resources and processes #968

Conversation

takoverflow commented Feb 18, 2025 • edited Loading

gardener-robot commented Feb 18, 2025

elankath Mar 10, 2025

Choose a reason for hiding this comment

elankath Mar 10, 2025

Choose a reason for hiding this comment

elankath left a comment • edited Loading

Choose a reason for hiding this comment

elankath left a comment

Choose a reason for hiding this comment

takoverflow commented Mar 10, 2025

takoverflow commented Feb 18, 2025 •

edited

Loading

elankath left a comment •

edited

Loading