-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Make zone configuration optional when creating a regional cluster #19
Changes from 3 commits
28c28dd
5483314
1bbd5f7
d0ee133
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# Simple Cluster | ||
# Simple Regional Cluster | ||
|
||
This example illustrates how to create a simple cluster. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,18 +27,34 @@ function export_vars() { | |
export TEST_ID="modules_gke_integration_gcloud_${RANDOM}" | ||
export KUBECONFIG="${TEMPDIR}/${CLUSTER_TYPE}/${TEST_ID}.kubeconfig" | ||
if [[ $CLUSTER_TYPE = "regional" ]]; then | ||
if [ -f "./regional_config.sh" ]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The complex variable export logic defined in this function adds a barrier to troubleshooting tests. If I needed to test out some modifications to the module, run a In addition, the sourcing of additional scripts means that if I set some variables in This only affects the tests and not the correctness of the module, so I think that we can merge this. @ryanckoch @Jberlinsky what do you think of filing a ticket/doing a followup pull request that converts the tests to something like what we do for the cloud-datastore module? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general, I like what we're doing in
@morgante, would love to hear your thoughts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like my reply got lost here. My preference is that we merge towards the style of testing which @adrienthebo developed (and away from the bash scripts). We can add tickets to the backlog and whoever picks up each module next will also work on the test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @morgante Once this is merged in, I'll create tickets. Thanks for weighing in! |
||
source ./regional_config.sh | ||
fi | ||
export CLUSTER_REGIONAL="true" | ||
export CLUSTER_LOCATION="$REGIONAL_LOCATION" | ||
export CLUSTER_NAME="$REGIONAL_CLUSTER_NAME" | ||
export IP_RANGE_PODS="$REGIONAL_IP_RANGE_PODS" | ||
export IP_RANGE_SERVICES="$REGIONAL_IP_RANGE_SERVICES" | ||
else | ||
if [ -f "./zonal_config.sh" ]; then | ||
source ./zonal_config.sh | ||
fi | ||
if [ -z "${ZONE}" ]; then | ||
echo "Can not create a zonal cluster without specifying \$ZONE. Aborting..." | ||
exit 1 | ||
fi | ||
export CLUSTER_REGIONAL="false" | ||
export CLUSTER_LOCATION="$ZONAL_LOCATION" | ||
export CLUSTER_NAME="$ZONAL_CLUSTER_NAME" | ||
export IP_RANGE_PODS="$ZONAL_IP_RANGE_PODS" | ||
export IP_RANGE_SERVICES="$ZONAL_IP_RANGE_SERVICES" | ||
fi | ||
|
||
if [ "${ZONE}" = "" ] && [ "${ADDITIONAL_ZONES}" = "" ]; then | ||
export ZONES="" | ||
else | ||
export ZONES="\"$ZONE\",$ADDITIONAL_ZONES" | ||
fi | ||
} | ||
|
||
# Activate test working directory | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
# Copyright 2018 Google LLC | ||
# | ||
# Licensed 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. | ||
|
||
require 'googleauth' | ||
require 'google/apis/compute_v1' | ||
|
||
def google_compute_service | ||
Google::Apis::ComputeV1::ComputeService.new.tap do |service| | ||
service.authorization = Google::Auth.get_application_default(['https://www.googleapis.com/auth/cloud-platform']) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
#!/bin/bash | ||
# Copyright 2018 Google LLC | ||
# | ||
# Licensed 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. | ||
|
||
|
||
################################################################# | ||
# PLEASE FILL THE VARIABLES WITH VALID VALUES FOR TESTING # | ||
# DO NOT REMOVE ANY OF THE VARIABLES # | ||
################################################################# | ||
|
||
export ZONE="us-east4-a" | ||
export ADDITIONAL_ZONES='"us-east4-b","us-east4-c"' | ||
export ZONAL_LOCATION="$ZONE" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're explicitly calling out that it's a regional example, lets delete
examples/simple
since that one one was inherently a regional cluster. I like it beingsimple_regional
better... makes it more clear.