[tmpnet] Define reusable flags for configuring kubernetes client access #3945
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the handling of Kubernetes configuration by introducing reusable flag variables so that both the tmpnetctl command and bootstrap-monitor tests consistently consume kubeconfig settings.
- Refactored main.go to remove manual kubeconfig flag handling and adopt the new reusable flags.
- Added a new flags package (kube_config.go) encapsulating kubeconfig flag registration.
- Updated e2e tests and Taskfile.yml to leverage the new flag definitions.
Reviewed Changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/fixture/tmpnet/tmpnetctl/main.go | Removed manual flag registration; replaced with reusable kubeconfig flag handling. |
| tests/fixture/tmpnet/flags/kube_config.go | Introduced a new reusable flag set for kubeconfig configuration. |
| tests/fixture/bootstrapmonitor/e2e/e2e_test.go | Updated kubeconfig retrieval to use the new flag variables. |
| Taskfile.yml | Added a new ginkgo-build task to run tests against the current working directory. |
Files not reviewed (3)
- flake.nix: Language not supported
- scripts/start_kind_cluster.sh: Language not supported
- scripts/tests.e2e.bootstrap_monitor.sh: Language not supported
Comments suppressed due to low confidence (1)
tests/fixture/tmpnet/flags/kube_config.go:47
- [nitpick] The type 'varFunc[string]' is not defined in this diff and may be unclear to readers. Consider adding documentation or a comment clarifying its purpose and definition.
func (v *KubeconfigVars) register(stringVar varFunc[string], docPrefix string) {
| @@ -89,6 +89,10 @@ tasks: | |||
| desc: Generates protobuf | |||
| cmd: ./scripts/protobuf_codegen.sh | |||
|
|
|||
| ginkgo-build: | |||
There was a problem hiding this comment.
For some reason ginkgo build . is no longer working, this task enables invoking ginkgo from anywhere in the tree against the cwd rather than having to use a root-based path.
| @@ -45,6 +45,7 @@ | |||
|
|
|||
| # Kube tools | |||
| kubectl # Kubernetes CLI | |||
| k9s # Kubernetes TUI | |||
There was a problem hiding this comment.
Handy for diagnosing kube issues
| @@ -0,0 +1,12 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
This script is reused in subsequent PRs
maru-ava
force-pushed
the
tmpnet-kubeconfig-flags
branch
from
0ae4351 to
29d35d9
Compare
| @@ -221,6 +222,6 @@ func getLatestImageDetails( | |||
|
|
|||
| func getClientset(log logging.Logger) (*kubernetes.Clientset, error) { | |||
| log.Info("Initializing clientset") | |||
| kubeConfigPath := os.Getenv("KUBECONFIG") | |||
| kubeConfigPath := os.Getenv(flags.KubeConfigPathEnvVar) | |||
There was a problem hiding this comment.
Note that the bootstrap-monitor doesn't accept flags, it's only intended to run in kubernetes with an in-cluster config.
maru-ava
changed the title
scripts/start_kind_cluster.sh
Outdated
| START_CLUSTER_ARGS=() | ||
| for arg in "$@"; do | ||
| if [[ "${arg}" =~ "--kubeconfig" || "${arg}" =~ "--kubeconfig-context" ]]; then | ||
| START_CLUSTER_ARGS+=("${arg}") | ||
| fi | ||
| done |
There was a problem hiding this comment.
This looks wrong to me. If you invoke this with "start_kind_cluster --kubeconfig something" then it will invoke tmpnetctl with only "--kubeconfig" and not supply the "something" argument.
Might work as long as you always invoke with "--kubeconfig=something", but I've never seen commands invoked like that.
There was a problem hiding this comment.
I've never seen commands invoked like that.
My shell history would like a word with you...
This is a test script that works without any arguments, accepting --kubeconfig and --kubeconfig-context is a convenience. I'm not sure it's too much to ask to require the form --arg=value, but I take your point that it should be documented.
There was a problem hiding this comment.
My shell history would like a word with you...
🤣
This would have definitely express the intent:
if [[ "${arg}" =~ "--kubeconfig=" || "${arg}" =~ "--kubeconfig-context=" ]]; then
Previously `tmpnetctl start-kind-cluster` and the bootstrap-monitor determined their kubeconfig and context separately. Simpler to do it one way and this will be reusable by the kube node runtime.
maru-ava
force-pushed
the
tmpnet-kubeconfig-flags
branch
from
98c745c to
55a18b1
Compare
PR Chain: tmpnet+kube
This PR chain enables tmpnet to deploy temporary networks to Kubernetes. Early PRs refactor tmpnet to support the addition in #3615 of a new tmpnet node runtime for kube.
Why this should be merged
Previously
tmpnetctl start-kind-clusterand the bootstrap-monitor determined their kubeconfig and context separately. Simpler to do it one way and this will be reusable by the kube node runtime.How this was tested
CI
Need to be documented in RELEASES.md?
N/A