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

TestGetKubeConfig unit test failing #1294

Merged
merged 2 commits into from
Jun 7, 2023
Merged

TestGetKubeConfig unit test failing #1294

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c18e96
remove duplicate unit test
RomanBednar Jun 7, 2023
c32439b
do no set kube host and port env vars
RomanBednar Jun 7, 2023
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
14 changes: 0 additions & 14 deletions pkg/azurefile/azure_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,12 +370,6 @@ users:
os.Setenv("CONTAINER_SANDBOX_MOUNT_POINT", "C:\\var\\lib\\kubelet\\pods\\12345678-1234-1234-1234-123456789012")
defer os.Unsetenv("CONTAINER_SANDBOX_MOUNT_POINT")

os.Setenv("KUBERNETES_SERVICE_HOST", "foo")
defer os.Unsetenv("KUBERNETES_SERVICE_HOST")

os.Setenv("KUBERNETES_SERVICE_PORT", "bar")
defer os.Unsetenv("KUBERNETES_SERVICE_PORT")

tests := []struct {
desc string
kubeconfig string
Expand All @@ -400,14 +394,6 @@ users:
envVariableHasConfig: false,
envVariableConfigIsValid: false,
},
{
desc: "[failure] invalid kube config passed",
Copy link
Member

Choose a reason for hiding this comment

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

is it possible only skip this test in OpenShift coreos?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO an unit test that checks system directories is a bad test. [failure] invalid kube config passed tests getKubeConfig() with an empty kubeconfig file.

This leads to this call stack:

clientcmd.(*DeferredLoadingClientConfig).ClientConfig (merged_client_builder.go:120) k8s.io/client-go/tools/clientcmd
clientcmd.BuildConfigFromFlags (client_config.go:627) k8s.io/client-go/tools/clientcmd
azurefile.getKubeConfig (azure.go:156) sigs.k8s.io/azurefile-csi-driver/pkg/azurefile
azurefile.TestGetKubeConfig (azure_test.go:422) sigs.k8s.io/azurefile-csi-driver/pkg/azurefile

DeferredLoadingClientConfig.ClientConfig() then fails to load the empty kubeconfig, but as a fallback, it tries to load in-cluster config. And that's where it finds out that it's just a Pod in our CI and loads Kubernetes service account and therefore succeeds, instead of returning error.

azurefile-csi-driver/vendor/k8s.io/client-go/tools/clientcmd/merged_client_builder.go

Lines 119 to 123 in ae84f9a

// check for in-cluster configuration and use it
if config.icc.Possible() {
klog.V(4).Infof("Using in-cluster configuration")
return config.icc.ClientConfig()
}

With the env. variables unset, Possible() it will return error here:

azurefile-csi-driver/vendor/k8s.io/client-go/tools/clientcmd/client_config.go

Lines 603 to 609 in 817a13b

// Possible returns true if loading an inside-kubernetes-cluster is possible.
func (config *inClusterClientConfig) Possible() bool {
fi, err := os.Stat("/var/run/secrets/kubernetes.io/serviceaccount/token")
return os.Getenv("KUBERNETES_SERVICE_HOST") != "" &&
os.Getenv("KUBERNETES_SERVICE_PORT") != "" &&
err == nil && !fi.IsDir()
}

I don't know what magic Kubernetes CI does so that Possible() fails, most probably it does not create /var/run/secrets/kubernetes.io/serviceaccount/token. But an unit test result should not depend on presence of any files in /var/run.

kubeconfig: emptyKubeConfig,
enableWindowsHostProcess: false,
expectError: true,
envVariableHasConfig: false,
envVariableConfigIsValid: false,
},
{
desc: "[failure] empty Kubeconfig under container sandbox mount path",
kubeconfig: "",
Expand Down