✨ Add informer test #610

MikeSpreitzer · 2026-01-09T09:41:20Z

Summary

This PR extends the E2E test suite with a test of the generated client code.

Related issue(s)

Fixes #

MikeSpreitzer · 2026-01-09T18:42:42Z

/cc @pdettori
/cc @rishi-jat

Signed-off-by: Mike Spreitzer <[email protected]>

rishi-jat · 2026-01-09T19:19:50Z

cmd/watch-objs/main.go

+
+	restConfig, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, overrides).ClientConfig()
+	if err != nil {
+		logger.Error(err, "Failed to construct resstConfig")


Typo mistake : "resstConfig" should be "restConfig"

rishi-jat · 2026-01-09T19:20:16Z

cmd/watch-objs/main.go

+		eh.logger.Error(err, "Failed to Get object from lister", "kind", eh.kind, "name", mrObj.GetName())
+	}
+	if fromCache.GetName() != mrObj.GetName() {
+		eh.logger.Error(nil, "Lister returned objecgt of different name", "kind", eh.kind, "name", mrObj.GetName(), "nameFromLister", fromCache.GetName())


Typo mistake : "objecgt" should be "object"

rishi-jat · 2026-01-09T19:24:44Z

cmd/watch-objs/main.go

+	// Get retrieves the PostCreateHook from the index for a given name.
+	// Objects returned here must be treated as read-only.
+	Get(name string) (ObjectType, error)
+}


The interface is generic but the comments specifically mention PostCreateHook. Should probably say "objects" instead since this is used for both ControlPlane and PostCreateHook.

rishi-jat · 2026-01-09T19:25:21Z

test/e2e/test-informers.sh

+
+kubectl delete cp cptest
+
+kill %1


Why kill immediately after delete? Shouldn't we verify the informer actually saw the delete event before exiting? Right now we only test add events but not delete, which seems like we're testing half the functionality.

Good suggestion. Done.

rishi-jat · 2026-01-09T19:28:07Z

test/e2e/test-informers.sh

+logfile=log-$$
+trap "rm $logfile" EXIT
+
+go run "$REPO_ROOT/cmd/watch-objs" -n default -v=4 &> $logfile &


What happens if the first waitfor times out and the script exits? The watch-objs process will keep running since it's not in the trap. Should probably capture $! and kill it in the trap cleanup.

I tried that, but it killed only the parent process (21554) not the child process. Following is what I saw in a separate shell while the test was running.

mspreitz@mjs13 kubestellar % ps axlwww | grep watch-obj 501 21554 21550 0 31 0 411944560 50464 - S+ s003 0:00.90 go run /Users/mspreitz/go/src/github.com/kubestellar/kubeflex/cmd/watch-objs -n default -v=4 501 21559 21554 0 31 0 411878880 27760 - S+ s003 0:00.02 /Users/mspreitz/Library/Caches/go-build/e6/e6c0db46b13594d86d1f284fb463741738d87f93230b0d1ed304fd055b872566-d/watch-objs -n default -v=4 501 21594 75121 0 31 0 410593296 1104 - R+ s004 0:00.00 grep watch-obj

I have revised the test to use the built executable in bin/, that solves the two-process problem.

Makes sense.

Signed-off-by: Mike Spreitzer <[email protected]>

Copilot

Pull request overview

This PR adds end-to-end testing for the generated client code by introducing a new watch-objs command and corresponding test script.

Changes:

Added a new watch-objs command that demonstrates usage of generated informers for KubeFlex objects
Created an E2E test script that validates informer functionality by testing add and delete notifications
Updated dependencies to explicitly require github.com/spf13/pflag and k8s.io/klog/v2

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
test/e2e/test-informers.sh	New test script that validates informer notifications for ControlPlane objects
test/e2e/run.sh	Integrates the new informer test into the E2E test suite
go.mod	Promotes pflag and klog/v2 from indirect to direct dependencies
cmd/watch-objs/main.go	New command implementing an informer-based client for testing generated code
cmd/watch-objs/README.md	Documentation for the watch-objs command

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-09T20:15:46Z

test/e2e/test-informers.sh

+    let count=1
+    while true; do
+        sleep 5
+        if { eval "$cmd" ; } ; then return 0; fi
+        let count=count+1


The 'let' command is deprecated in modern bash scripts. Use arithmetic expansion '(( count = 1 ))' or 'declare -i count=1' instead for better portability and clarity.

Suggested change

let count=1

while true; do

sleep 5

if { eval "$cmd" ; } ; then return 0; fi

let count=count+1

local -i count=1

while true; do

sleep 5

if { eval "$cmd" ; } ; then return 0; fi

(( count++ ))

Copilot · 2026-01-09T20:15:46Z

test/e2e/test-informers.sh

+    let count=1
+    while true; do
+        sleep 5
+        if { eval "$cmd" ; } ; then return 0; fi
+        let count=count+1


The 'let' command is deprecated in modern bash scripts. Use arithmetic expansion '(( count++ ))' or '(( count += 1 ))' instead for better portability and clarity.

Suggested change

let count=1

while true; do

sleep 5

if { eval "$cmd" ; } ; then return 0; fi

let count=count+1

count=1

while true; do

sleep 5

if { eval "$cmd" ; } ; then return 0; fi

(( count++ ))

duplicate comment

Copilot · 2026-01-09T20:15:47Z

test/e2e/test-informers.sh

+REPO_ROOT=$(cd "$SCRIPT_DIR"; cd ../..; pwd)
+
+logfile=log-$$
+bin/watch-objs -n default -v=4 &> $logfile &


The path 'bin/watch-objs' is relative and may fail depending on the current working directory. Consider using an absolute path or ensuring the script changes to a known directory (like REPO_ROOT) before executing this command.

Suggested change

bin/watch-objs -n default -v=4 &> $logfile &

"$REPO_ROOT/bin/watch-objs" -n default -v=4 &> "$logfile" &

Not a bad idea.
But there is plenty of precedent in this and other test scripts for that assumption.
Maybe do a comprehensive improvement in a separate PR.

Copilot · 2026-01-09T20:15:47Z

test/e2e/test-informers.sh

+./bin/kflex create cptest --type k8s --chatty-status=false
+./bin/kflex ctx


These commands use './bin/kflex' while line 24 uses 'bin/watch-objs' (without the leading dot). For consistency, either prefix all paths with './' or omit it from all.

https://en.wikipedia.org/wiki/Wikipedia:Emerson_and_Wilde_on_consistency

Copilot · 2026-01-09T20:15:47Z

cmd/watch-objs/main.go

+}
+
+func (eh eventHandler[ObjectType]) OnAdd(obj any, isInitial bool) {
+	mrObj := obj.(mrObject)


The type assertion could panic if the object is not of the expected type. Consider using the two-value form 'mrObj, ok := obj.(mrObject)' and logging an error message if the assertion fails, providing details about the unexpected type.

It is exactly right for this program to crash if the object is not a mrObject. This is supposed to be a test, and the assertion will fail only when the test should fail.

Copilot · 2026-01-09T20:15:47Z

cmd/watch-objs/main.go

+}
+
+func (eh eventHandler[ObjectType]) OnUpdate(oldObj, obj any) {
+	mrObj := obj.(mrObject)


The type assertion could panic if the object is not of the expected type. Consider using the two-value form 'mrObj, ok := obj.(mrObject)' and logging an error message if the assertion fails, providing details about the unexpected type.

duplicate comment

Copilot · 2026-01-09T20:15:48Z

cmd/watch-objs/main.go

+	if dfsu, is := obj.(cache.DeletedFinalStateUnknown); is {
+		obj = dfsu.Obj
+	}
+	mrObj := obj.(mrObject)


The type assertion could panic if the object is not of the expected type. Consider using the two-value form 'mrObj, ok := obj.(mrObject)' and logging an error message if the assertion fails, providing details about the unexpected type.

duplicate comment

rishi-jat · 2026-01-09T20:23:32Z

/lgtm

kubestellar-prow · 2026-01-09T20:23:37Z

LGTM label has been added.

Details

Git tree hash: 833e25cb4bd23230b45d246e3a05515c61184bdc

MikeSpreitzer · 2026-01-09T20:29:27Z

/approve

kubestellar-prow · 2026-01-09T20:29:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MikeSpreitzer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [MikeSpreitzer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kubestellar-prow bot added dco-signoff: yes Indicates the PR's author has signed the DCO. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 9, 2026

MikeSpreitzer force-pushed the add-informer-test branch 3 times, most recently from 9957d6a to 7f8893f Compare January 9, 2026 18:08

kubestellar-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 9, 2026

MikeSpreitzer force-pushed the add-informer-test branch 3 times, most recently from 8f38e6e to e2c47bc Compare January 9, 2026 18:41

kubestellar-prow bot requested review from pdettori and rishi-jat January 9, 2026 18:42

MikeSpreitzer force-pushed the add-informer-test branch 2 times, most recently from c7cd863 to 873fafe Compare January 9, 2026 18:48

Add informer test to E2E test suite

19265f8

Signed-off-by: Mike Spreitzer <[email protected]>

MikeSpreitzer force-pushed the add-informer-test branch from 873fafe to 19265f8 Compare January 9, 2026 18:51

rishi-jat reviewed Jan 9, 2026

View reviewed changes

Fixes and improvements in client code test

a69fffa

Signed-off-by: Mike Spreitzer <[email protected]>

MikeSpreitzer force-pushed the add-informer-test branch 3 times, most recently from a75f3cf to 21fdd98 Compare January 9, 2026 20:09

Use built watch-objs executable

d2d432f

Signed-off-by: Mike Spreitzer <[email protected]>

MikeSpreitzer force-pushed the add-informer-test branch from 21fdd98 to d2d432f Compare January 9, 2026 20:13

MikeSpreitzer requested a review from Copilot January 9, 2026 20:14

Copilot AI reviewed Jan 9, 2026

View reviewed changes

kubestellar-prow bot assigned rishi-jat Jan 9, 2026

kubestellar-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 9, 2026

kubestellar-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 9, 2026

kubestellar-prow bot merged commit 49323ec into kubestellar:main Jan 9, 2026
9 checks passed

MikeSpreitzer deleted the add-informer-test branch January 9, 2026 20:31

github-actions bot mentioned this pull request Jan 9, 2026

[Doc Update] ✨ Add informer test kubestellar/docs#538

Closed

	bin/watch-objs -n default -v=4 &> $logfile &
	"$REPO_ROOT/bin/watch-objs" -n default -v=4 &> "$logfile" &

		./bin/kflex create cptest --type k8s --chatty-status=false
		./bin/kflex ctx

✨ Add informer test #610

✨ Add informer test #610

Uh oh!

Conversation

MikeSpreitzer commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related issue(s)

Uh oh!

MikeSpreitzer commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rishi-jat commented Jan 9, 2026

Uh oh!

kubestellar-prow bot commented Jan 9, 2026

Uh oh!

MikeSpreitzer commented Jan 9, 2026

MikeSpreitzer commented Jan 9, 2026 •

edited

Loading