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

cli/demo: fix enterprise features for multitenancy #81737

Merged
merged 2 commits into from
May 25, 2022

Conversation

knz
Copy link
Contributor

@knz knz commented May 24, 2022

First commit from #81762.

Fixes #80270.
Fixes #40434.

This patch achieves two things:

  • it ensures that enterprise features work properly in multi-tenant cockroach demo when --disable-demo-license is not passed as argument.
  • it ensures cockroach demo does not require a working licensing
    endpoint to run (by not retrieving a license over the network on startup).

The net result will be less spurious failures in CI, as well as
avoiding startup delays/errors during interactive uses, which provide
poor UX.

Release note: None

@knz knz requested review from dt and a team May 24, 2022 12:21
@knz knz requested a review from a team as a code owner May 24, 2022 12:21
@knz knz requested a review from a team May 24, 2022 12:21
@knz knz requested a review from a team as a code owner May 24, 2022 12:21
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz force-pushed the 20220524-demo-license branch from b538edd to 0d8efb0 Compare May 24, 2022 16:00
@knz knz requested a review from rafiss May 24, 2022 16:30
knz added 2 commits May 24, 2022 19:22
…lti-region`

When using `--geo-partitioned-replicas`, some secondary regions are
added, which is only supported with multitenancy if a cluster setting
is also set. So this commit ensures the cluster setting is set.

NB: no unit tests in this commit because the next commit will ensure
this gets tested.

Release note: None
This patch achieves two things:

- it ensures that enterprise features work properly in multi-tenant `cockroach
  demo` when `--disable-demo-license` is not passed as argument.
- it ensures `cockroach demo` does not require a working licensing
  endpoint to run (by not retrieving a license over the network on startup).

The net result will be less spurious failures in CI, as well as
avoiding startup delays/errors during interactive uses, which provide
poor UX.

Release note: None
@knz knz force-pushed the 20220524-demo-license branch from 0d8efb0 to d373cb1 Compare May 24, 2022 17:23
craig bot pushed a commit that referenced this pull request May 25, 2022
81762: democluster: fix `cockroach demo movr --geo-partitioned-replicas --multi-region` r=arulajmani a=knz

Prereq to #81737.

When using `--geo-partitioned-replicas`, some secondary regions are
added, which is only supported with multitenancy if a cluster setting
is also set. So this commit ensures the cluster setting is set.

NB: no unit tests in this commit because the next commit will ensure
this gets tested.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

this is great! do you think the change is backportable? the field teams have said that the call to the license server is a problem for them, since we don't have good SLAs for that license server and it has gone down in the past.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dt)

@knz
Copy link
Contributor Author

knz commented May 25, 2022

Yes this is backportable. As-is to 22.1, and then with a new custom patch to 21.2 and earlier.

@knz
Copy link
Contributor Author

knz commented May 25, 2022

bors r=rafiss

@rafiss
Copy link
Collaborator

rafiss commented May 25, 2022

sounds good -- FWIW, there are no more scheduled 21.1.x releases.

@craig
Copy link
Contributor

craig bot commented May 25, 2022

Build succeeded:

@craig craig bot merged commit 63924f9 into cockroachdb:master May 25, 2022
@blathers-crl
Copy link

blathers-crl bot commented May 25, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from 3965a6c to blathers/backport-release-21.2-81737: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 21.2.x failed. See errors above.


error creating merge commit from d373cb1 to blathers/backport-release-22.1-81737: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.1.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants