Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions cmd/watch-objs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# The watch-objs command

This is a simple example client of the generated code.

This client focuses on one Kubernetes namespace and makes an informer on all the
KubeFlex objects in that namespace.
When informed of any add/delete/update of such an object, a line is logged;
V(2) for add/delete, V(4) for update.
128 changes: 128 additions & 0 deletions cmd/watch-objs/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package main

import (
"context"
"flag"
"fmt"
"os"
"time"

"github.com/spf13/pflag"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/klog/v2"

api "github.com/kubestellar/kubeflex/api/v1alpha1"
kfclient "github.com/kubestellar/kubeflex/pkg/generated/clientset/versioned"
kfinformers "github.com/kubestellar/kubeflex/pkg/generated/informers/externalversions"
)

const ControllerName = "ensure-label"

func main() {
loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
overrides := &clientcmd.ConfigOverrides{}

klog.InitFlags(flag.CommandLine)
pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
pflag.CommandLine.StringVar(&loadingRules.ExplicitPath, "kubeconfig", loadingRules.ExplicitPath, "Path to the kubeconfig file to use")
pflag.CommandLine.StringVar(&overrides.CurrentContext, "context", overrides.CurrentContext, "The name of the kubeconfig context to use")
pflag.CommandLine.StringVar(&overrides.Context.AuthInfo, "user", overrides.Context.AuthInfo, "The name of the kubeconfig user to use")
pflag.CommandLine.StringVar(&overrides.Context.Cluster, "cluster", overrides.Context.Cluster, "The name of the kubeconfig cluster to use")
pflag.CommandLine.StringVarP(&overrides.Context.Namespace, "namespace", "n", overrides.Context.Namespace, "The name of the Kubernetes Namespace to work in (NOT optional)")
pflag.Parse()
ctx := context.Background()
logger := klog.FromContext(ctx)

logger.V(1).Info("Start", "time", time.Now())

pflag.CommandLine.VisitAll(func(f *pflag.Flag) {
logger.V(1).Info("Flag", "name", f.Name, "value", f.Value.String())
})

if len(overrides.Context.Namespace) == 0 {
fmt.Fprintln(os.Stderr, "Namespace must not be the empty string")
os.Exit(1)
} else {
logger.Info("Focusing on one namespace", "name", overrides.Context.Namespace)
}

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

Choose a reason for hiding this comment

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

Typo mistake : "resstConfig" should be "restConfig"

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.

os.Exit(10)
}
if len(restConfig.UserAgent) == 0 {
restConfig.UserAgent = ControllerName
} else {
restConfig.UserAgent += "/" + ControllerName
}

kfClient, err := kfclient.NewForConfig(restConfig)
if err != nil {
logger.Error(err, "Failed to construct client")
os.Exit(10)
}
sif := kfinformers.NewSharedInformerFactoryWithOptions(kfClient, 0, kfinformers.WithNamespace(overrides.Context.Namespace))
cpPreInf := sif.Tenancy().V1alpha1().ControlPlanes()
pchPreInf := sif.Tenancy().V1alpha1().PostCreateHooks()
cpInformer, cpLister := cpPreInf.Informer(), cpPreInf.Lister()
pchInformer, pchLister := pchPreInf.Informer(), pchPreInf.Lister()
cpInformer.AddEventHandler(eventHandler[*api.ControlPlane]{logger, "ControlPlane", cpLister})
pchInformer.AddEventHandler(eventHandler[*api.PostCreateHook]{logger, "PostCreateHook", pchLister})
sif.Start(ctx.Done())
if !cache.WaitForCacheSync(ctx.Done(), cpInformer.HasSynced, pchInformer.HasSynced) {
logger.Error(nil, "Failed to sync informer caches")
os.Exit(20)
}
<-ctx.Done()
}

type mrObject interface {
metav1.Object
runtime.Object
}

type GenericLister[ObjectType mrObject] interface {
// List lists all PostCreateHooks in the indexer.
// Objects returned here must be treated as read-only.
List(selector labels.Selector) (ret []ObjectType, err error)
// 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)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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


type eventHandler[ObjectType mrObject] struct {
logger klog.Logger
kind string
lister GenericLister[ObjectType]
}

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

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

eh.logger.V(2).Info("Notified of add", "kind", eh.kind, "name", mrObj.GetName())
fromCache, err := eh.lister.Get(mrObj.GetName())
if err != nil {
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())
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo mistake : "objecgt" should be "object"

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.

}
}

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

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicate comment

eh.logger.V(4).Info("Notified of update", "kind", eh.kind, "name", mrObj.GetName())
}

func (eh eventHandler[ObjectType]) OnDelete(obj any) {
if dfsu, is := obj.(cache.DeletedFinalStateUnknown); is {
obj = dfsu.Obj
}
mrObj := obj.(mrObject)
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicate comment

eh.logger.V(2).Info("Notified of delete", "kind", eh.kind, "name", mrObj.GetName())
}
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ require (
github.com/openshift/client-go v0.0.0-20240925210910-aaed17e719c5
github.com/pkg/errors v0.9.1
github.com/spf13/cobra v1.9.1
github.com/spf13/pflag v1.0.6
go.uber.org/zap v1.27.0
gopkg.in/yaml.v2 v2.4.0
helm.sh/helm/v3 v3.17.4
k8s.io/api v0.32.10
k8s.io/apiextensions-apiserver v0.32.10
k8s.io/apimachinery v0.32.10
k8s.io/client-go v0.32.10
k8s.io/klog/v2 v2.130.1
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
sigs.k8s.io/controller-runtime v0.20.4
)
Expand Down Expand Up @@ -123,7 +125,6 @@ require (
github.com/shopspring/decimal v1.4.0 // indirect
github.com/sirupsen/logrus v1.9.3 // indirect
github.com/spf13/cast v1.7.0 // indirect
github.com/spf13/pflag v1.0.6 // indirect
github.com/x448/float16 v0.8.4 // indirect
github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
Expand Down Expand Up @@ -153,7 +154,6 @@ require (
k8s.io/apiserver v0.32.10 // indirect
k8s.io/cli-runtime v0.32.2 // indirect
k8s.io/component-base v0.32.10 // indirect
k8s.io/klog/v2 v2.130.1 // indirect
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f // indirect
k8s.io/kubectl v0.32.2 // indirect
oras.land/oras-go v1.2.6 // indirect
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ if [[ "$(yq -o=json .extensions "$cfgfile" )" =~ ^[[] ]]; then
mv -f -- "$cfgfile" "${cfgfile}.bak"
mv -- $$ "$cfgfile"
fi

${SRC_DIR}/test-informers.sh
${SRC_DIR}/manage-type-k8s.sh
${SRC_DIR}/test-controller-image-update.sh
${SRC_DIR}/manage-type-vcluster.sh
Expand Down
45 changes: 45 additions & 0 deletions test/e2e/test-informers.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#!/usr/bin/env bash

set -euo pipefail
set -x

function waitfor() {
cmd="$1"
let count=1
while true; do
sleep 5
if { eval "$cmd" ; } ; then return 0; fi
let count=count+1
Comment on lines +8 to +12
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

meh

Comment on lines +8 to +12
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

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++ ))

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicate comment

if (( count > 15 )); then
echo 'Timeout waiting for `'"$cmd" >&2
return 1
fi
done
}

SCRIPT_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &> /dev/null && pwd)
REPO_ROOT=$(cd "$SCRIPT_DIR"; cd ../..; pwd)

logfile=log-$$
trap "rm $logfile" EXIT

go run "$REPO_ROOT/cmd/watch-objs" -n default -v=4 &> $logfile &
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.



if ! waitfor 'grep -q "Notified of add.*PostCreateHook.*name=\"synthetic-crd\"" '$logfile; then
cat $logfile
exit 1
fi


./bin/kflex create cptest --type k8s --chatty-status=false
./bin/kflex ctx
Comment on lines +34 to +35
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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


if ! waitfor 'grep -q "Notified of add.*ControlPlane.*name=\"cptest\"" '$logfile; then
cat $logfile
exit 1
fi

kubectl delete cp cptest

kill %1
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion. Done.