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

Add --extracontext arg to subctl verify #1048

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

tpantelis
Copy link
Contributor

...to run lighthouse tests that require a third cluster.

Added a system test using the new arg that is run when lighthouse is deployed. This requires the cluster settings to specify a third cluster which can be problematic wrt CI env resources. So the System Test globalnet and lighthouse options were split into 2 jobs with lighthouse using a separate cluster settings containing the third cluster and only the control-plane node.

Fixes #276

@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr1048/tpantelis/third_context_for_verify
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@tpantelis tpantelis requested a review from aswinsuryan January 15, 2024 23:26
the --enable-disruptive flag is also specified. If running non-interactively (that is with no stdin),
--enable-disruptive must be specified otherwise disruptive verifications are skipped.
Long: `This command performs various tests to verify that a Submariner deployment between two clusters,
specified via the --context and --tocontext args, is functioning properly. Some tests require a third cluster,
Copy link
Contributor

Choose a reason for hiding this comment

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

How about specifying "Some servicediscovery tests require a third cluster" to make it clear that if someone is not using servicediscovery verify they don't need to use this argument.

Or, if service discovery is not enabled for verification give a warn or info message to user that --extracontext will be ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking not to make it specific to service discovery in case we ever add other test(s) that need a third cluster.

Copy link
Contributor

Choose a reason for hiding this comment

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

Making it genric may be a good idea as there could be use cases later for newer test in connectivity requiring a third cluster.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking not to make it specific to service discovery in case we ever add other test(s) that need a third cluster.

We can change the info message when we need it for other suites. As of now it is only service discovery, will help user know that this will apply only to service discovery and other tests don't need this option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I agree it would be useful. We can change it later if needed although that kind of thing tends to slip through the cracks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

...to run lighthouse tests that require a third cluster.

Added a system test using the new arg that is run when lighthouse is
deployed. This requires the cluster settings to specify a third cluster
which can be problematic wrt CI env resources. So the System Test
globalnet and lighthouse options were split into 2 jobs with lighthouse
using a separate cluster settings containing the third cluster and only
the control-plane node.

Fixes submariner-io#276

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
@tpantelis tpantelis force-pushed the third_context_for_verify branch from 3fe2ef6 to bb806f3 Compare January 18, 2024 13:29
@tpantelis tpantelis requested a review from vthapar January 22, 2024 12:48
@submariner-bot submariner-bot added the ready-to-test When a PR is ready for full E2E testing label Jan 22, 2024
@tpantelis tpantelis merged commit 01283ab into submariner-io:devel Jan 23, 2024
30 checks passed
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr1048/tpantelis/third_context_for_verify]

@dfarrell07 dfarrell07 added the release-note-needed Should be mentioned in the release notes label Jan 23, 2024
tpantelis added a commit to tpantelis/submariner-website that referenced this pull request Jan 23, 2024
Release notes for submariner-io/subctl#1048

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
tpantelis added a commit to submariner-io/submariner-website that referenced this pull request Jan 24, 2024
Release notes for submariner-io/subctl#1048

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
tpantelis added a commit to tpantelis/submariner-website that referenced this pull request Feb 27, 2024
Release notes for submariner-io/subctl#1048

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
tpantelis added a commit to submariner-io/submariner-website that referenced this pull request Feb 29, 2024
Release notes for submariner-io/subctl#1048

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
@tpantelis tpantelis deleted the third_context_for_verify branch April 4, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-test When a PR is ready for full E2E testing release-note-handled release-note-needed Should be mentioned in the release notes
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add a three-context variant for subctl verify
6 participants