You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "damccorm (via GitHub)" <gi...@apache.org> on 2023/04/12 15:10:22 UTC
[GitHub] [beam] damccorm commented on a diff in pull request #25994: Update of GKE deployment
damccorm commented on code in PR #25994:
URL: https://github.com/apache/beam/pull/25994#discussion_r1164248286
##########
playground/backend/new_scio_project.sh:
##########
@@ -15,6 +15,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-{ printf scio\\nscio\\n; yes; } | sbt new spotify/scio-template.g8
+if [ -d /opt/sbt-template/scio ]; then
Review Comment:
Why do we need this set of changes?
##########
playground/terraform/infrastructure/private_dns/googleapis_com.tf:
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+resource "google_dns_managed_zone" "private-zone-private-googleapis" {
+ project = var.project_id
+
+ name = "${var.network_name}googleapis-com"
+ dns_name = "googleapis.com."
+ description = "Private GoogleApi Zone"
+
+ visibility = "private"
+
+ private_visibility_config {
+ networks {
+ network_url = var.network_id
+ }
+ }
+}
+
+resource "google_dns_record_set" "a-private-googleapis" {
+ name = "private.googleapis.com."
+ type = "A"
+ ttl = 300
+
+ managed_zone = google_dns_managed_zone.private-zone-private-googleapis.name
+
+ rrdatas = ["199.36.153.8", "199.36.153.9", "199.36.153.10", "199.36.153.11"]
Review Comment:
Same question, where do these come from?
##########
playground/infrastructure/helm-playground/templates/autoscaling-go.yaml:
##########
@@ -12,15 +12,27 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-apiVersion: autoscaling/v1
+apiVersion: autoscaling/v2
kind: HorizontalPodAutoscaler
metadata:
name: playground-go
spec:
- maxReplicas: 10
- minReplicas: 1
+ maxReplicas: {{ .Values.autoscaling.runners.maxReplicas }}
+ minReplicas: {{ .Values.autoscaling.runners.minReplicas }}
scaleTargetRef:
apiVersion: apps/v1
kind: Deployment
name: playground-go
- targetCPUUtilizationPercentage: 90
+ metrics:
+ - type: Resource
+ resource:
+ name: cpu
+ target:
+ type: Utilization
+ averageUtilization: 95
Review Comment:
Same comment applies to other files
##########
playground/infrastructure/helm-playground/templates/autoscaling-go.yaml:
##########
@@ -12,15 +12,27 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-apiVersion: autoscaling/v1
+apiVersion: autoscaling/v2
kind: HorizontalPodAutoscaler
metadata:
name: playground-go
spec:
- maxReplicas: 10
- minReplicas: 1
+ maxReplicas: {{ .Values.autoscaling.runners.maxReplicas }}
+ minReplicas: {{ .Values.autoscaling.runners.minReplicas }}
scaleTargetRef:
apiVersion: apps/v1
kind: Deployment
name: playground-go
- targetCPUUtilizationPercentage: 90
+ metrics:
+ - type: Resource
+ resource:
+ name: cpu
+ target:
+ type: Utilization
+ averageUtilization: 95
Review Comment:
Should these averageUtilization targets be congfigurable values (like max/min replicas?)
##########
playground/terraform/infrastructure/firewall/firewall.tf:
##########
@@ -0,0 +1,82 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+resource "google_compute_firewall" "playground-firewall-deny-egress" {
+ name = "${var.network_name}-deny-egress"
+ network = var.network_name
+ direction = "EGRESS"
+ priority = 1001
+ deny {
+ protocol = "all"
+ }
+ destination_ranges = ["0.0.0.0/0"]
+ target_tags = ["beam-playground"]
+}
+
+resource "google_compute_firewall" "playground-firewall-allow-controlplane" {
+ name = "${var.network_name}-allow-controlplane"
+ network = var.network_name
+ direction = "EGRESS"
+ priority = 1000
+ allow {
+ protocol = "all"
+ }
+ destination_ranges = [var.gke_controlplane_cidr]
+ target_tags = ["beam-playground"]
+}
+
+resource "google_compute_firewall" "playground-firewall-allow-dns" {
+ name = "${var.network_name}-allow-dns"
+ network = var.network_name
+ direction = "EGRESS"
+ priority = 1000
+ allow {
+ protocol = "tcp"
+ ports = ["53"]
+
+ }
+ allow {
+ protocol = "udp"
+ ports = ["53"]
+ }
+ destination_ranges = ["0.0.0.0/0"]
+ target_tags = ["beam-playground"]
+}
+
+resource "google_compute_firewall" "playground-firewall-allow-privateapi" {
+ name = "${var.network_name}-allow-privateapi"
+ network = var.network_name
+ direction = "EGRESS"
+ priority = 1000
+ allow {
+ protocol = "all"
+ }
+
+ destination_ranges = ["199.36.153.8/30"]
Review Comment:
Where is this destination range coming from?
##########
playground/terraform/infrastructure/memorystore/variables.tf:
##########
@@ -48,7 +48,7 @@ variable "tier" {
variable "replicas_mode" {
description = "If enabled read endpoint will be provided and the instance can scale up and down the number of replicas"
- default = "READ_REPLICAS_ENABLED"
+ default = "READ_REPLICAS_DISABLED"
Review Comment:
Why did we change our default here?
##########
playground/terraform/infrastructure/appengine/variables.tf:
##########
@@ -36,3 +36,13 @@ variable "location_id_us" {
description = "Location of App"
default = "us-central"
}
+
+variable "location_id_eu" {
+ description = "Location of App"
+ default = "europe-west"
+}
+
+variable "feature_flag" {
Review Comment:
Could you add a description (and maybe a more descriptive name)? What is this a feature flag for?
##########
playground/terraform/README.md:
##########
@@ -53,29 +54,37 @@ Ensure that the account has at least following privileges:
* [Docker](https://docs.docker.com/engine/install/)
* [Terraform](https://www.terraform.io/downloads)
* [gcloud CLI](https://cloud.google.com/sdk/docs/install-sdk)
+* [Kubectl authentication](https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke)
6. Apache Beam Git repository cloned locally
# Prepare deployment configuration:
-Playground uses `terraform.tfvars` located in `playground/terraform/environment/environment_name` to define variables specific to an environment (e.g., prod, test, staging).<br>
-1. Create a folder (further referred as `environment_name`) to define a new environment and place configuration files into it:
+Playground uses `terraform.tfvars` located in `playground/terraform/environment/<environment_name>` to define variables specific to an environment (e.g., prod, test, staging).<br>
+1. Create a folder (further referred as <environment_name>) to define a new environment and place configuration files into it:
* `terraform.tfvars` environment variables:
```
-project_id = "project_id" #GCP Project ID
-network_name = "network_name" #GCP VPC Network Name for Playground deployment
-gke_name = "playground-backend" #Playground GKE Cluster name
-region = "us-east1" #Set the deployment region
-location = "us-east1-b" #Select the deployment location from available in the specified region
-state_bucket = "bucket_name" #GCS bucket name for Beam Playground temp files
-redis_name = "playground_redis" #Choose the name for redis instance
-min_count = 2 #Min node count for GKE cluster
-max_count = 6 #Max node count for GKE cluster
+project_id = "project_id" # GCP Project ID
+network_name = "playground-network" # GCP VPC Network Name for Playground deployment
Review Comment:
Nit: Could we keep spacing consistent here so that `#` characters are aligned?
##########
playground/terraform/infrastructure/gke/variables.tf:
##########
@@ -54,4 +54,9 @@ variable "min_count" {
variable "max_count" {
description = "Max cluster node count"
default = 6
-}
\ No newline at end of file
+}
+variable "control_plane_cidr" {
+ description = "CIDR block for GKE controlplane"
+ default = "10.129.0.0/28"
Review Comment:
Same question, where is this range coming from?
##########
playground/terraform/applications/backend/backend-go/main.tf:
##########
@@ -1,73 +0,0 @@
-#
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements. See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership. The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License. You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied. See the License for the
-# specific language governing permissions and limitations
-# under the License.
-#
-
-
-resource "google_app_engine_flexible_app_version" "backend_app_go" {
Review Comment:
I'm a little bit confused - why do we no longer need these `.tf` files? Could you give me some more context on this please?
##########
playground/terraform/infrastructure/private_dns/zones.tf:
##########
@@ -0,0 +1,63 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+resource "google_dns_managed_zone" "private-zone-apis" {
+
+ for_each = var.private_zones
+
+ project = var.project_id
+
+ name = "${var.network_name}-${replace(each.key,".","-")}"
+ dns_name = format("%s%s",each.key,".")
+ description = "Private ${each.key} Zone"
+
+ visibility = "private"
+
+ private_visibility_config {
+ networks {
+ network_url = var.network_id
+ }
+ }
+}
+
+resource "google_dns_record_set" "a-private-zone" {
+
+ for_each = google_dns_managed_zone.private-zone-apis
+
+ name = each.value.dns_name
+ type = "A"
+ ttl = 300
+
+ managed_zone = each.value.name
+
+ rrdatas = ["199.36.153.8", "199.36.153.9", "199.36.153.10", "199.36.153.11"]
Review Comment:
Could we abstract these out into a variable since we're using it multiple places?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org