-
Notifications
You must be signed in to change notification settings - Fork 87
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
KEP-0008: Multi-cluster - Proposals for TestCase, TestStep Changes #344
base: main
Are you sure you want to change the base?
KEP-0008: Multi-cluster - Proposals for TestCase, TestStep Changes #344
Conversation
I've added some info here too around the reporting APIs. Initially I thought maybe a modification to I am open to feedback there. |
…ulti cluster tests. Signed-off-by: Miles-Garnsey <[email protected]>
1779ffa
to
095455a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
open questions below. Reporting section is simple and looks good
## Proposal | ||
## Proposals - TestStep API changes | ||
|
||
The proposal is to add a new setting within `TestStep`. We propose this field be called `runOnCluster` and of type `[]string`. This setting would allow the user to specify a list of the clusters set up by the `TestSuite` on which to run `TestSteps`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps I'm not following now... I was seeing runOnCluster
as 1 cluster string
type. here it is an array. Is the proposal intending to have a single defined test to be responsible for more than 1 cluster? if true... do you see a defined order? or run concurrently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the proposal intending to have a single defined test to be responsible for more than 1 cluster? if true... do you see a defined order? or run concurrently?
Yes, I am thinking that a TestStep
will run on multiple clusters indeed. I was thinking that running concurrently was probably best, as I think we want synchronisation to be handled via the commencement/end of a TestStep
.
What do you think? My rationale is that (at least for the tests I've been writing) we'd probably want most steps to run on all clusters, and it is the exception where we want the clusters to diverge. But if others have different requirements I'm keen to hear about them.
``` | ||
|
||
If the kubeconfig setting is not set, then the global Kubernetes client is used. | ||
If the runOnCluster setting is not set, then the `TestStep` will be run on all clusters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems like we should have a configuration phase which sets all cluster to run on. In this approach, it would be good if we see no defined clusters to mean what it means today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be good if we see no defined clusters to mean what it means today.
I'm hoping that this backwards compatibility is a consequence of what I'm proposing indeed.
I was conceptualising the current arrangement as one where the tests are run on all clusters BUT there is actually only one cluster to run on. Which gives backwards compatibility if runOnCluster
(or however we name that parameter) is unset.
Let me know if that makes sense.
it seems like we should have a configuration phase which sets all cluster to run on.
Yes, I think that this should happen per TestStep
is the only thing, and be based on clusters declared via the TestSuite
so that we can refer to a kubeconfig + kube context pair using strings.
|
||
If the kubeconfig setting is set, then it will be used for all Kubernetes operations within the step: commands (the KUBECONFIG environment variable will be set to the kubeconfig setting, relative to the KUTTL CLI's working directory), applied resources, asserts, and errors. This means that a single step can only be configured to use a single kubeconfig, but multiple steps can be used if a test case needs to interact with more than one cluster. This allows, for example, a federated resource to be created in one step and then another step can be used to verify that it actually exists on the destination cluster. | ||
Where multiple clusters are specified (including if the `runOnCluster` field is unset), each script and command will be run in parallel on the desired clusters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are your thoughts of defining the cluster in the config phase, then there is no unset at test run. This may be the first phase of work... to mod the structures for multi-cluster support, then make it work for the current single cluster support, then add more code for full multi-cluster support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defining the cluster in the config phase
It depends on whether we're thinking the config phase is the TestSuite
config phase or a TestStep
config phase. My feeling is that the TestSuite
should declare the clusters that will be used and the TestStep
should then define which subset of those clusters the TestStep
will run on.
I do generally agree that we should probably define the API, make it work for the current single-cluster behaviour, and then add multi-cluster. Seems like the safest way to ensure that we don't break any existing behaviour.
What this PR does / why we need it:
KEP-0008 is currently light on detail. To get it ready for implementation this PR fleshes out proposed changes to the TestCase and TestStep APIs.
The changes proposed are designed to allow for specification of what clusters a given
TestStep
should run on, as well as providing injection of appropriate environment variables identifying the kubeconfig and kube context that scripts and commands may require to access a given cluster.One outstanding question, is whether (in addition to making environment variables available in scripts and commands) we should have some sort of automatic kubeconfig/kube context injection mechanism, so that a kubectl command which does not explicitly use the KUBECONFIG or KUBECONTEXT environment variables automatically has the appropriate argument inserted. If we did include this, we should also include a field to disable that functionality.