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.

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

Use the correct kubeconfig when running in-cluster #437

Merged
merged 6 commits into from
Dec 9, 2022
Merged
Changes from 5 commits
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
59 changes: 32 additions & 27 deletions pkg/test/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ func (h *Harness) RunTestEnv() (*rest.Config, error) {

// Config returns the current Kubernetes configuration - either from the environment
// or from the created temporary control plane.
// As a side effect, on first successful call this method also writes a kubernetes client config file in YAML format
// to a file called "kubeconfig" in the current directory.
Copy link
Member

Choose a reason for hiding this comment

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

👏

func (h *Harness) Config() (*rest.Config, error) {
h.configLock.Lock()
defer h.configLock.Unlock()
Expand All @@ -253,51 +255,54 @@ func (h *Harness) Config() (*rest.Config, error) {
return nil, err
}
h.config.WarningHandler = rest.NewWarningWriter(os.Stderr, rest.WarningWriterOptions{Deduplicate: true})
inCluster, err := testutils.InClusterConfig()
Copy link
Member

@kensipe kensipe Dec 9, 2022

Choose a reason for hiding this comment

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

I'm most concern with this removal... but it is likely, based on code review, the issue you are looking to solve. I vaguely remember needing this to satisfy the way operator SDK scorecard uses kuttl.

if err != nil {
return nil, err
}
if inCluster {
return h.config, nil
}
}
if err != nil {
return h.config, err
return nil, err
Copy link
Member

Choose a reason for hiding this comment

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

👏

}

// if not the mocked control plane
// Newly started clusters aren't ready until default service account is ready.
// We need to wait until one is present. Otherwise, we sometimes hit an error such as:
// error looking up service account <namespace>/default: serviceaccount "default" not found
//
// We avoid doing this for the mocked control plane case (because in that case the default service
// account is not provided anyway?)
// We still do this when running inside a cluster, because the cluster kuttl is pointed *at* might
// be different from the cluster it is running *in*, and it does not hurt when it is the same cluster.
if !h.TestSuite.StartControlPlane {
// newly started clusters aren't ready until default service account is ready
// fixes: error looking up service account <namespace>/default: serviceaccount "default" not found
// we avoid this with "inCluster" as the cluster must be already be up since we're running on it
err = testutils.WaitForSA(h.config, "default", "default")
if err != nil {
// if there is a namespace provided but no "default"/"default" SA found, also check a SA in the provided NS
if h.TestSuite.Namespace != "" {
tempErr := testutils.WaitForSA(h.config, "default", h.TestSuite.Namespace)

// if it still does not have a SA then return the first "default"/"default" error
if tempErr != nil {
return h.config, err
}
} else {
return h.config, err
}
if err := h.waitForFunctionalCluster(); err != nil {
return nil, err
}
h.T.Logf("Successful connection to cluster at: %s", h.config.Host)
}

// The creation of the "kubeconfig" is necessary for out of cluster execution of kubectl
// The creation of the "kubeconfig" is necessary for out of cluster execution of kubectl,
// as well as in-cluster when the supplied KUBECONFIG is some *other* cluster.
f, err := os.Create("kubeconfig")
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should make this a test temp file / folder... however this is consistent with existing behavior deserves it's own PR if we want it. Thanks for the Godocs! really helpful!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think this should be in a temporary file (and created with an appropriate umask BTW), see also this related issue.

if err != nil {
return h.config, err
return nil, err
}

defer f.Close()

return h.config, testutils.Kubeconfig(h.config, f)
}

func (h *Harness) waitForFunctionalCluster() error {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this should be in kubernetes.go but it's just a refactor of code in the existing file... looks good!

err := testutils.WaitForSA(h.config, "default", "default")
if err == nil {
return nil
}
// if there is a namespace provided but no "default"/"default" SA found, also check a SA in the provided NS
if h.TestSuite.Namespace != "" {
tempErr := testutils.WaitForSA(h.config, "default", h.TestSuite.Namespace)
if tempErr == nil {
return nil
}
}
// either way, return the first "default"/"default" error
return err
}

// Client returns the current Kubernetes client for the test harness.
func (h *Harness) Client(forceNew bool) (client.Client, error) {
h.clientLock.Lock()
Expand Down