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

Support deployers passing additional information to testers #87

Closed
rifelpet opened this issue Jan 27, 2021 · 3 comments
Closed

Support deployers passing additional information to testers #87

rifelpet opened this issue Jan 27, 2021 · 3 comments
Labels
sig/testing Categorizes an issue or PR as relevant to SIG Testing.

Comments

@rifelpet
Copy link
Contributor

Currently a tester is invoked with only arguments provided from the kubetest2 invocation (kubetest2 ... -- --tester-args-here) along with some environment variables. The deployer can optionally provide a kubeconfig file to the tester via an interface:

kubetest2/pkg/app/app.go

Lines 128 to 133 in d56fa28

if dWithKubeconfig, ok := d.(types.DeployerWithKubeconfig); ok {
if kconfig, err := dWithKubeconfig.Kubeconfig(); err == nil {
envsForTester = append(envsForTester, fmt.Sprintf("%s=%s", "KUBECONFIG", kconfig))
}
}

As I build kubetest2 support for Kops I'm noticing that certain testers have flags whose values are only known by the deployer rather than the process invoking kubetest2.

For example a kubectl test relies on a Host flag, but the cluster's apiserver host name isn't known at the time that kubetest2 is invoked, so appending it to the end of the kubetest2 command isn't possible. Many of the cloudConfig flags in test_context.go have values that aren't known until the deployer has deployed the cluster too.

It would be nice to have a way for a deployer to provide a set of arguments and/or environment variables to the tester. One idea is to add a DeployerWithTesterArgs interface similar to the DeployerWithKubeconfig interface:

type DeployerWithTesterArgs interface {
	Deployer

	TesterArgs() []string
}

One concern I have with that is the deployer would need to know which tester is being used in order to provide it the necessary arguments. The tester interface doesn't provide any sort of identifier that we could pass into the new interface's function. We could add a Name to Tester:

type DeployerWithTesterArgs interface {
	Deployer

	TesterArgs(name string) []string
}

type Tester struct {
	Name string
	TesterPath string
	TesterArgs []string
}

but this increasingly tight coupling feels contradictory to kubetest2's objectives.

Any thoughts or ideas on how to provide testers with additional info they need from deployers? I'm happy to implement something if we reach a consensus.

@amwat
Copy link
Contributor

amwat commented Jan 27, 2021

Yes, this was one of the objectives from the very start; to discourage non standard information being passed around.
Either way having an interface for testers won't help much either since the contract between deployers and testers is now "invoking as a binary".

Can this information be computed after we have access to the cluster? in which case we should definitely do that inside the tester.

If not, one workaround that was mentioned as part of the design is to generate well-known files under ARTIFACTS (which is also exposed to testers), but that comes at the risk of testers starting to rely on those existing. see:
https://docs.google.com/document/d/10Zfu0KiJVMxHL-rNMW9pOESdFcdCuw2ABrDFKVQqooM/edit#heading=h.5iltbm3drdip

@amwat
Copy link
Contributor

amwat commented Mar 19, 2021

Inferring the information as much as possible is the right way to go
which seems to be under investigation in kubernetes/kops#10847

/close

can reopen if we run into more problems

sig-testing issues (kubernetes-sigs) automation moved this from To Triage to Done Mar 19, 2021
@k8s-ci-robot
Copy link
Contributor

@amwat: Closing this issue.

In response to this:

Inferring the paths is the right way to go
which seems to be under investigation in kubernetes/kops#10847

/close

can reopen if we run into more problems

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@spiffxp spiffxp added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/testing Categorizes an issue or PR as relevant to SIG Testing.
Development

No branches or pull requests

4 participants