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 assertion, errors Changes #343

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+8 −0
Open

KEP-0008: Multi-cluster - Proposals for assertion, errors Changes #343

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
8 changes: 8 additions & 0 deletions keps/0008-multi-cluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ 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 - Testassert and test errors

There is already a `TestAssert`` API which allows the test assertions to be configured. We propose to add to this a field `runOnCluster` of type `[]string` which will specify a set of named contexts on which to run the assertion. The strings will link back to the keys of the map defined in the proposed `MultiClusterConfig` struct, or (if a global config has been specified with the `NumClusters` option), to the KinD clusters deployed by the `TestSuite` setup steps.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
There is already a `TestAssert`` API which allows the test assertions to be configured. We propose to add to this a field `runOnCluster` of type `[]string` which will specify a set of named contexts on which to run the assertion. The strings will link back to the keys of the map defined in the proposed `MultiClusterConfig` struct, or (if a global config has been specified with the `NumClusters` option), to the KinD clusters deployed by the `TestSuite` setup steps.
There is already a `TestAssert` API which allows the test assertions to be configured. We propose to add to this a field `runOnCluster` of type `[]string` which will specify a set of named contexts on which to run the assertion. The strings will link back to the keys of the map defined in the proposed `MultiClusterConfig` struct, or (if a global config has been specified with the `NumClusters` option), to the KinD clusters deployed by the `TestSuite` setup steps.

Copy link
Member

Choose a reason for hiding this comment

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

it is now more clear on proposed direction...
wondering if it would be reasonable to just define cluster as opposed to runOnCluster
Also... It seems reasonable to have the test configuration phase define the TestAssert (and the "cluster lookup") so there isn't a look up at time of run... which may change the name runOnCluster differently.

One issue I see is at startup we hit the defined cluster and create a kubeconfig locally. There is an issue reported to make sure that is cleaned up. What we will need to define is: we will use a locally stored file... we they have different names and how (perhaps in the multi-cluster-config). What may be usefully to understand is the kubeconfig file is used I believe for different purposes but largely for cmd executions kubectl xyz. We will need to work thru how the commands will be configured while support more than 1 cluster.

Copy link
Author

Choose a reason for hiding this comment

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

wondering if it would be reasonable to just define cluster as opposed to runOnCluster

Yes, I think runOnCluster is clumsy. Probably calling the field Clusters is good, (*especially if we're to make this an array, note that's what I'm thinking!)

We will need to work thru how the commands will be configured while support more than 1 cluster.

Some discussion of that here:

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.

So I'm thinking that we should make KUBECONFIG and KUBECONTEXT env variables available, but also potentially inject those variables automatically for kubectl commands. I'd value your thoughts there, since I'm pretty sure existing functionality already does something like that.


Additionally we propose to add a special value `ALL` which will run the test assertion on all clusters.

It seems that there may not currently be a `TestError` API, so it is an open question as to whether that should be implemented as part of this work to make assertions and errors consistent.

## Proposal

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.
Expand Down