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

KEP-0008: Multi-cluster - Proposals for TestCase, TestStep Changes #344

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

KEP-0008: Multi-cluster - Proposals for TestCase, TestStep Changes #344

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 15 additions & 6 deletions keps/0008-multi-cluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,29 @@ Currently, KUTTL only supports a single test cluster. This KEP describes how we
* Support applying resources across more than one test cluster.
* Support asserting on resources across more than one test cluster.

## 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`.
Copy link
Member

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?

Copy link
Author

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.


Recall that `TestSuite` contains a map of `KindConfig`s, entries in the `runOnCluster` would refer to one of the keys in this map. Where a user has specified homogenous clusters using `GlobalKindConfig` and `NumClusters`, a uniform naming scheme should exist to make identification of the right cluster to run on possible.

The proposal is to add a new setting to the `TestStep` object: `kubeconfig`. This setting would allow the user to specify an alternative kubeconfig path to use for a given test step.

```
apiVersion: kudo.dev/v1alpha1
kind: TestStep
kubeconfig: ./secondary-cluster.yaml
runOnCluster:
- my-cluster-1
- my-cluster-2
```

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.
Copy link
Member

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.

Copy link
Author

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.
Copy link
Member

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

Copy link
Author

@Miles-Garnsey Miles-Garnsey Mar 17, 2022 •
edited
Loading

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.


Note that the `kubeconfig` setting on the `TestStep` would be unaffected by the global Kubernetes configuration, so the `--kubeconfig` flag, `$KUBECONFIG` environment variable, etc, will be ignored for these steps.
The kubeconfig file location, and the name of the context for the target cluster will be made available as environment variables `KUBECONFIG` and `KUBECONTEXT` in the scripts and commands.

A namespace is generated for each `TestCase` and this needs to be created in each cluster referenced by `TestSteps` within the `TestCase`. At the beginning of the `TestCase`, the generated namespace will be created in every cluster used in the `TestCase`. The namespaces will also be deleted at the end if `--skip-delete` is not set.

## Proposals - Reporting API changes

Within the reporting structs, `Failure` may need to be modified to provide an indication of which cluster a given `TestStep failed on`.