Skip to content

Yugabyte: Working GCP implementation#1190

Merged
barroco merged 4 commits intointeruss:masterfrom
Orbitalize:yugabyte_in_gcp
Jun 17, 2025
Merged

Yugabyte: Working GCP implementation#1190
barroco merged 4 commits intointeruss:masterfrom
Orbitalize:yugabyte_in_gcp

Conversation

@the-glu
Copy link
Contributor

@the-glu the-glu commented May 27, 2025

This PR introduce changes in the teraform files to support yugabyte in GCP.

It follow #1186 . You probably want to review the last commit only.

It needs the following PRs to make it working even with yugabyte not working correctly with server_broadcast_address ( yugabyte/yugabyte-db#27367 ), we do 'cheat' by injecting into the host file the public host we want, making yugabyte broadcasting the public hostname as private rpc address.

It does the following:

  • Added a new terraform option datastore_type, and needed yugabyte options
  • Fixed and added a public name option in certificate generation for yugabyte nodes
    • The previous PR doesn't include the extension file, generating certificate only valid for the 'main' host
  • Add correct load balances for yugabyte, exposing all ports
    • Some port are exposed even if not valid to avoid a UI issue doing queries on unavailable port and waiting for timeout (exposing them reject directly the tcp connection)

It doesn't have, for now (but the PR is quite big):

  • Documentation
  • Some variables are shared with cochroachdb and haven't been renamed to generic ones.
  • AWS implementation is broken dues to changes in the common part, but next PR will fix that :)
  • Default values

It has been tested with two independent GCP k8s clusters that successfully formed a cluster:

image
image

@the-glu the-glu requested a review from barroco May 27, 2025 09:44
@the-glu the-glu force-pushed the yugabyte_in_gcp branch 4 times, most recently from 8613d7e to 08c3a7e Compare June 3, 2025 08:59
Copy link
Contributor

@barroco barroco 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 of comments. May I kindly ask you to provide a list of ports required to be exposed in order to successfully pool two DSS instances?

yugabyte = {
enabled = true

resource = var.yugabyte_light_resources ? {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this and address it in another PR if needed. Since there is no autoscalling, this would only change the requested resources without changing the type of nodes provisionned.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though, I would expect to see a configuration flag similar to https://github.com/interuss/dss/pull/1190/files#diff-f08dfea212cbd81c9b94266f15255cfd4c4b381b146e7b42c2049c77fbb5560eR24 to indicate the number of nodes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's needed when testing on a small cluster (e.g. by following the default values / README) because the services are requesting a lot of resources, I would suggest to keep it to ease quick / test deployments without having the need of spawning a big cluster.

Not sure how it's related to autoscalling however ?

(Will do add a configuration flag for the number of nodes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Number of nodes support added)

{{- end }}

# Tserver nodes Gateways
{{- range $i, $lb := .Values.loadBalancers.yugabyteTserverNodes }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the definition of yugabyteTserverNodes. Is it required to expose publicly those ports ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some are needed for inter-tserver communication, and some are needed for the interface / monitoring stuff.

Should we don't want to be able to use the UI, we could limit those, but then the UI will be in a half-working state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

( yugabyteTserverNodes added )

@the-glu the-glu force-pushed the yugabyte_in_gcp branch from 0a4401f to dbf9377 Compare June 5, 2025 14:24
@the-glu
Copy link
Contributor Author

the-glu commented Jun 5, 2025

@barroco ready for a second pass

May I kindly ask you to provide a list of ports required to be exposed in order to successfully pool two DSS instances?

Where do you want that ? I suggest we do it in the cleanup/doc phase ( #1192 ) :)

Co-authored-by: Michael Barroco <michael@orbitalize.com>
@the-glu the-glu force-pushed the yugabyte_in_gcp branch from dbf9377 to 35dc1df Compare June 5, 2025 14:29
@the-glu the-glu requested a review from barroco June 5, 2025 14:29
Copy link
Contributor

@barroco barroco left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me. Let's merge it and address #1191 as quickly as possible to avoid inconsistency due to changes of terraform-commons-dss.
We can still do a pass of optimization before next release.

@barroco barroco merged commit a248baa into interuss:master Jun 17, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants