Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup test namespace #42

Merged
merged 3 commits into from
Jul 30, 2024
Merged

cleanup test namespace #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1020e27
move test namespace cleanup to the setup script
apesternikov Jul 30, 2024
c24567a
do not for namespace deletion
apesternikov Jul 30, 2024
9c90f72
fix the v7 build
apesternikov Jul 30, 2024
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
1 change: 1 addition & 0 deletions .bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
common --enable_bzlmod
build --nolegacy_external_runfiles
build --remote_download_all
build --verbose_failures
build --@io_bazel_rules_go//go/config:pure
test --test_output=errors
Expand Down
1 change: 1 addition & 0 deletions e2e/.bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
build --action_env HOME --test_env HOME
common:bzlmod --enable_bzlmod
build --remote_download_all
build --nolegacy_external_runfiles
build --@io_bazel_rules_go//go/config:pure
15 changes: 9 additions & 6 deletions skylib/k8s_test_namespace.sh.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,22 @@ echo "Cluster: ${CLUSTER}" >&2
echo "User: ${USER}" >&2

set +e

ns_cleanup() {
echo "Performing namespace ${NAMESPACE} cleanup..."
${KUBECTL} --kubeconfig=${KUBECONFIG} --cluster=${CLUSTER} --user=${USER} delete namespace --wait=false ${NAMESPACE}
}
Comment on lines 48 to +53
Copy link

Choose a reason for hiding this comment

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

The set +e command disables the immediate exit on error, which can lead to the script continuing execution even if critical commands fail. This can cause unexpected behavior and make debugging difficult. Consider handling errors explicitly where needed and re-enabling set -e after the critical section.


if [ -n "${K8S_TEST_NAMESPACE:-}" ]
then
# use provided namespace
NAMESPACE=${K8S_TEST_NAMESPACE}
# do not delete namespace after the test is complete
DELETE_NAMESPACE_FLAG=""
elif [ -n "${K8S_MYNAMESPACE:-}" ]
then
# do not create random namesspace
NAMESPACE=$(whoami)
# do not delete namespace after the test is complete
DELETE_NAMESPACE_FLAG=""
else
# create random namespace
DELETE_NAMESPACE_FLAG="-delete_namespace"
COUNT="0"
while true; do
NAMESPACE=`whoami`-$(( (RANDOM) + 32767 ))
Expand All @@ -71,6 +72,8 @@ else
exit 1
fi
done
Copy link

Choose a reason for hiding this comment

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

The while loop that generates a random namespace does not have a termination condition if the namespace creation keeps failing. This can lead to an infinite loop. Consider adding a maximum retry limit to avoid potential infinite loops.

# delete namespace after the test is complete or failed
trap ns_cleanup EXIT
fi
echo "Namespace: ${NAMESPACE}" >&2
set -e
Expand Down Expand Up @@ -110,4 +113,4 @@ function waitpids() {
# create k8s objects
%{statements}

%{it_sidecar} -namespace=${NAMESPACE} %{sidecar_args} ${DELETE_NAMESPACE_FLAG} "$@"
%{it_sidecar} -namespace=${NAMESPACE} %{sidecar_args} "$@"
27 changes: 7 additions & 20 deletions testing/it_sidecar/it_sidecar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,13 @@ func (i *arrayFlags) Set(value string) error {
}

var (
namespace = flag.String("namespace", os.Getenv("NAMESPACE"), "kubernetes namespace")
timeout = flag.Duration("timeout", time.Second*30, "execution timeout")
deleteNamespace = flag.Bool("delete_namespace", false, "delete namespace as part of the cleanup")
pfconfig = portForwardConf{services: make(map[string][]uint16)}
kubeconfig string
waitForApps arrayFlags
allowErrors bool
disablePodLogs bool
namespace = flag.String("namespace", os.Getenv("NAMESPACE"), "kubernetes namespace")
timeout = flag.Duration("timeout", time.Second*30, "execution timeout")
pfconfig = portForwardConf{services: make(map[string][]uint16)}
kubeconfig string
waitForApps arrayFlags
allowErrors bool
disablePodLogs bool
)

func init() {
Expand Down Expand Up @@ -329,17 +328,6 @@ func portForward(ctx context.Context, clientset *kubernetes.Clientset, config *r
return nil
}

func cleanup(clientset *kubernetes.Clientset) {
log.Print("Cleanup")
if *deleteNamespace && *namespace != "" {
log.Printf("deleting namespace %s", *namespace)
s := meta_v1.DeletePropagationBackground
if err := clientset.CoreV1().Namespaces().Delete(context.Background(), *namespace, meta_v1.DeleteOptions{PropagationPolicy: &s}); err != nil {
log.Printf("Unable to delete namespace %s: %v", *namespace, err)
}
}
}

var ErrTimedOut = errors.New("timed out")
var ErrStdinClosed = errors.New("stdin closed")
var ErrTermSignalReceived = errors.New("TERM signal received")
Expand Down Expand Up @@ -382,7 +370,6 @@ func main() {
log.Fatal(err)
}
clientset = kubernetes.NewForConfigOrDie(config)
defer cleanup(clientset)

go func() {
err := stern.Run(ctx, *namespace, clientset, allowErrors, disablePodLogs)
Comment on lines 374 to 375
Copy link

Choose a reason for hiding this comment

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

The goroutine starting at line 374 calls stern.Run but does not handle any potential errors returned by this function. If stern.Run fails, the error will be lost, making debugging difficult.

Recommendation: Handle the error returned by stern.Run within the goroutine, possibly by logging it or sending it to a channel for further processing.

Expand Down