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

Jump to bottom

Add new ways of exposing ScyllaCluster outside #1359

Merged
merged 2 commits into from
Sep 18, 2023
Merged

Add new ways of exposing ScyllaCluster outside #1359

merged 2 commits into from
Sep 18, 2023

Conversation

zimnx
Copy link
Collaborator

@zimnx zimnx commented Aug 29, 2023 •
edited by tnozicka

ExposeOptions of ScyllaCluster allows for choosing which Service type
is created for each cluster node, and which address is broadcasted to
clients and nodes. This allows for exposing ScyllaCluster to clients
from external networks, and provides an important building block for
connecting two distinct ScyllaClusters.

Fixes #1319

@scylla-operator-bot scylla-operator-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 29, 2023
@scylla-operator-bot scylla-operator-bot bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 29, 2023
@zimnx zimnx force-pushed the mz/expose-scylla-outside branch 4 times, most recently from f6be2a6 to ce3fb23 Compare September 1, 2023 15:16
@zimnx zimnx changed the title [WIP] Expose ScyllaCluster Add new ways of exposing ScyllaCluster outside Sep 1, 2023
@scylla-operator-bot scylla-operator-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 1, 2023
rzetelskik
rzetelskik reviewed Sep 5, 2023
View reviewed changes
Copy link
Member

@rzetelskik rzetelskik left a comment

Choose a reason for hiding this comment

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

first pass

pkg/sidecar/identity/member.go Show resolved Hide resolved
pkg/api/scylla/v1/types_cluster.go Outdated Show resolved Hide resolved
test/e2e/set/scyllacluster/scyllacluster_expose.go Outdated Show resolved Hide resolved
pkg/scyllafeatures/scyllafeatures.go Show resolved Hide resolved
pkg/api/scylla/validation/cluster_validation.go Outdated Show resolved Hide resolved
pkg/api/scylla/validation/cluster_validation.go Outdated Show resolved Hide resolved
pkg/api/scylla/validation/cluster_validation.go Outdated Show resolved Hide resolved
pkg/cmd/operator/sidecar.go Outdated Show resolved Hide resolved
test/e2e/set/scyllacluster/scyllacluster_expose.go Outdated Show resolved Hide resolved
@scylla-operator-bot scylla-operator-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 7, 2023
@scylla-operator-bot scylla-operator-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 7, 2023
@zimnx zimnx force-pushed the mz/expose-scylla-outside branch 6 times, most recently from 9e13b38 to 8bf1ac4 Compare September 11, 2023 21:00
@scylladb scylladb deleted a comment from scylla-operator-bot bot Sep 11, 2023
rzetelskik
rzetelskik reviewed Sep 12, 2023
View reviewed changes
pkg/cmd/operator/sidecar.go Outdated Show resolved Hide resolved
pkg/sidecar/config/config.go Outdated Show resolved Hide resolved
pkg/sidecar/config/config.go Outdated Show resolved Hide resolved
pkg/sidecar/config/config.go Outdated Show resolved Hide resolved
@zimnx
Copy link
Collaborator Author

zimnx commented Sep 12, 2023 •
edited

dump doesn't contain scylla-operator logs required to validate why webhook test failed.
/test e2e-gke-parallel

rzetelskik
rzetelskik reviewed Sep 12, 2023
View reviewed changes
pkg/api/scylla/v1/types_cluster.go Outdated Show resolved Hide resolved
pkg/sidecar/identity/member.go Outdated Show resolved Hide resolved
pkg/sidecar/identity/member.go Outdated Show resolved Hide resolved
pkg/sidecar/identity/member.go Outdated Show resolved Hide resolved
pkg/sidecar/identity/member_test.go Show resolved Hide resolved
pkg/controller/scyllacluster/resource.go Outdated Show resolved Hide resolved
pkg/controller/scyllacluster/resource.go Outdated Show resolved Hide resolved
pkg/controller/scyllacluster/resource.go Outdated Show resolved Hide resolved
pkg/controller/scyllacluster/sync_certs.go Show resolved Hide resolved
@zimnx zimnx force-pushed the mz/expose-scylla-outside branch 2 times, most recently from fa92e7c to a6e1fc3 Compare September 12, 2023 20:04
@zimnx zimnx force-pushed the mz/expose-scylla-outside branch 6 times, most recently from b332731 to a8d0f08 Compare September 14, 2023 22:59
@zimnx zimnx requested a review from tnozicka September 15, 2023 07:56
tnozicka
tnozicka reviewed Sep 15, 2023
View reviewed changes
Copy link
Member

@tnozicka tnozicka left a comment

Choose a reason for hiding this comment

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

thanks for the updates, got through it all this time, rest lgtm

pkg/controller/scyllacluster/sync_certs.go Outdated Show resolved Hide resolved
pkg/controller/scyllacluster/sync_certs.go Outdated Show resolved Hide resolved
pkg/controller/scyllacluster/sync_certs.go Show resolved Hide resolved
pkg/controller/scyllacluster/sync_services.go Show resolved Hide resolved
pkg/controller/scyllacluster/sync_statefulsets.go Show resolved Hide resolved
test/e2e/set/scyllacluster/scyllacluster_expose.go Outdated Show resolved Hide resolved
test/e2e/set/scyllacluster/scyllacluster_expose.go Outdated Show resolved Hide resolved
test/e2e/set/scyllacluster/scyllacluster_tls.go Show resolved Hide resolved
test/e2e/utils/helpers.go Outdated Show resolved Hide resolved
test/e2e/utils/helpers.go Show resolved Hide resolved
@scylla-operator-bot scylla-operator-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2023
@scylla-operator-bot scylla-operator-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2023
@zimnx zimnx force-pushed the mz/expose-scylla-outside branch 2 times, most recently from 549f2ba to 3dc8720 Compare September 17, 2023 14:29
tnozicka
tnozicka reviewed Sep 18, 2023
View reviewed changes
pkg/controller/scyllacluster/sync.go Outdated Show resolved Hide resolved
pkg/controller/scyllacluster/sync_certs.go Outdated Show resolved Hide resolved
pkg/controller/scyllacluster/validation.go Outdated Show resolved Hide resolved
@zimnx
Add new ways of exposing ScyllaCluster outside
eb144ac 
ExposeOptions of ScyllaCluster allows for choosing which Service type
is created for each cluster node, and which address is broadcasted to
clients and nodes. This allows for exposing ScyllaCluster to clients
from external networks, and provides an important building block for
connecting two distinct ScyllaClusters.
@zimnx
Update autogenerated
debc9e6
@zimnx zimnx requested a review from tnozicka September 18, 2023 11:27
tnozicka
tnozicka approved these changes Sep 18, 2023
View reviewed changes
Copy link
Member

@tnozicka tnozicka left a comment

Choose a reason for hiding this comment

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

thanks for the updates!

/approve
/lgtm

🚀

@scylla-operator-bot scylla-operator-bot bot added the lgtm Indicates that a PR is ready to be merged. label Sep 18, 2023
@scylla-operator-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rzetelskik, tnozicka, zimnx

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

@tnozicka tnozicka mentioned this pull request Sep 18, 2023
Closed
@scylla-operator-bot scylla-operator-bot bot merged commit c6b93ee into scylladb:master Sep 18, 2023
11 checks passed
@tnozicka tnozicka mentioned this pull request Sep 18, 2023
Closed
@zimnx zimnx deleted the mz/expose-scylla-outside branch September 18, 2023 17:21
@tnozicka tnozicka mentioned this pull request Sep 20, 2023
Merged
@brickpattern brickpattern mentioned this pull request Sep 25, 2023
Closed
@zimnx zimnx mentioned this pull request Sep 29, 2023
Merged
@zimnx zimnx added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rzetelskik rzetelskik rzetelskik left review comments

@tnozicka tnozicka tnozicka approved these changes

Assignees

@tnozicka tnozicka

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add option to configure ScyllaClusters with different IPs
3 participants
@zimnx @rzetelskik @tnozicka