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

🐛 server: relax ShardVirtualWorkspace flag validation #2659

Merged
merged 1 commit into from
Jan 23, 2023

Conversation

hardys
Copy link

@hardys hardys commented Jan 20, 2023

Summary

In #2407 we introduced this validation, but it doesn't handle the case where the VW server is not standalone, but you still want to specify --shard-virtual-workspace-url

This scenario is likely when deploying a sharded environment, since you'll want the VW traffic to go via the front-proxy regardless of whether the VW server is standalone or not.

Related issue(s)

Fixes an issue introduced via #2407

In kcp-dev#2407 we introduced this validation, but it doesn't handle the
case where the VW server is not standalone, but you still want to
specify --shard-virtual-workspace-url

This scenario is likely when deploying a sharded environment, since
you'll want the VW traffic to go via the front-proxy regardless
of whether the VW server is standalone or not.
@openshift-ci openshift-ci bot requested review from ncdc and sttts January 20, 2023 13:35
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncdc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 20, 2023
@hardys
Copy link
Author

hardys commented Jan 20, 2023

/test e2e-sharded

Looks like #2654

@hardys
Copy link
Author

hardys commented Jan 20, 2023

/retest

@hardys
Copy link
Author

hardys commented Jan 23, 2023

/retest

Appears to be stuck in merge pool, not sure if retest will help but giving it a try...

@hardys
Copy link
Author

hardys commented Jan 23, 2023

/refresh

@hardys
Copy link
Author

hardys commented Jan 23, 2023

/test e2e-shared

Raised #2665 to track the test failure, I can't reproduce locally so I'm assuming it's a flake

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants