search

GoogleCloudPlatform / bank-of-anthos

Retail banking sample application showcasing Kubernetes and Google Cloud
https://cymbal-bank.fsi.cymbal.dev
Apache License 2.0
1.01k stars 593 forks source link

Add NETWORK environment variable to be able to target the non-default VPC for ledgermonolith #1178

Closed fmichaelobrien closed 1 year ago

fmichaelobrien commented 1 year ago

Describe request or inquiry

Add NETWORK environment variable to be able to target the non-default VPC. Some project configuration ban or do not create the default VPC across all the GCP zones - for FedRamp or ITSG-33 compliant landing zones https://github.com/GoogleCloudPlatform/bank-of-anthos/blob/main/src/ledgermonolith/scripts/deploy-monolith.sh#L65

What purpose/environment will this feature serve?

Allow for running in restricted projects without a default VPC - using a custom mode VPC

I will put up a patch

fmichaelobrien commented 1 year ago 
gcloud compute instances create ledgermonolith-service \
    --project $PROJECT_ID \
    --zone $ZONE \
    --network $NETWORK \
    --subnet $SUBNET \
NimJay commented 1 year ago

Hi @fmichaelobrien,

First off, thank you so much for communicating this effort via a GitHub Issue before creating the pull-request! 👏 :) My team and I find that super useful.

1. Setting Variable Value

I see that the commit from your fork introduces two environment variables:

...
    --network $NETWORK \
    --subnet $SUBNET \
...

Question: Should users set the values of these variables before running the deploy-monolith.sh script?

2. Default Values

Suggestion: We should either:

  1. add validation similar to this that checks to see that $NETWORK and $SUBNET are set,
  2. or hard-code default values for $NETWORK and $SUBNET.

3. Updating Existing Docs

The deploy-monolith.sh file is used by:

  1. Cloud Skills Boost - Migrating a Monolithic Website to Microservices on Google Kubernetes Engine
  2. Code Labs - Migrating a Monolithic Website to Microservices on Google Kubernetes Engine
  3. github.com/googlecodelabs/monolith-to-microservices

Question: Do you plan on updating these pages/scripts? If we hard-code default values for $NETWORK and $SUBNET, we may not need to update these pages/scripts.

P.S. - I am not familiar with this deploy-monolith.sh script, so I'm sorry about my beginner questions.

fmichaelobrien commented 1 year ago

Thanks for the comment - yes I will put up a patch later today

The use case is for full automation of the lab environment (one that is over 3+ hours long - not actually in the list above - but similar) The case is around a GCP landing zone installed organization where there is a org level policy that prohibits default subnets and subnets not in specified regions. Hence the VPC must be created on the fly before running the script

Hardcoding would not work unless the custom vpc was called the same name across different organizations. What I currently do in the script is overlay a modified version of the sh over the repo after cloning it before adding it as a CSR

NimJay commented 1 year ago

Thank you for the quick response, @fmichaelobrien!

Hard-coding Defaults

Sorry, I should clarify what I meant by:

"hard-code default values for $NETWORK and $SUBNET"

I think the user running deploy-monolith.sh should be able to use the $NETWORK and $SUBNET environment variables (i.e., set the variables to a desired value). But if the user does not set the values, we could use pre-defined defaults. For instance:

gcloud ...
    --network {$NETWORK=default}
...

where default is the value of --network if the user hasn't set the value of $NETWORK themselves.

If we do this, we could avoid having to update existing documentation/codelabs/etc. that use deploy-monolith.sh.

Full Automation of Lab

You mentioned:

"The use case is for full automation of the lab environment... not actually in the list above"

That is awesome to hear (i.e., that this new lab you're working on will automate deployment of the monolith)! Question: Are you able to share a private draft with us (so we can better understand the context)?

fmichaelobrien commented 1 year ago

Sounds good, I didn't think of setting the default right in the ENV substitution - we should go with that There is a 4th alternative internal lab that uses the repo as well - that would benefit from no mods using the default approach The reason for the non-default variable is only for those that run it inside an org that has the location restriction org policy on - without a folder override and the no default vpc org policy enforced - usually in PBMM landing zones

from my private repo

source ./vars.sh inside the main script picks up on 

export REGION=us-central1
export NETWORK=$REGION
export SUBNET=$NETWORK-sn
#export NETWORK=default
#export SUBNET=default
export ZONE=$REGION-a
export VPC=$NETWORK-vpc

in the deployment.sh script - before doing a migctl 1.14 migration of the vm - we create the VM

  cp assets/deploy-monolith.sh ../../../$TEMP_REPO_DIR/bank-of-anthos/src/ledgermonolith/scripts/
  ../../../$TEMP_REPO_DIR/bank-of-anthos/src/ledgermonolith/scripts/deploy-monolith.sh

Issue is that after I pre-automated the lab - and actually ran the lab for the first time - the monolith vm is already up in the setup - the script ended uk deleting the pre-configured monolith GKE VM and recreating it - all good as the lab progressed normally

so my patch of

gcloud compute instances create ledgermonolith-service \
    --project $PROJECT_ID \
    --zone $ZONE \
    --network $NETWORK \
    --subnet $SUBNET \

should be as you mention (all these years I didn't run into the default parameter expansion format - thanks for that - I will use it

gcloud compute instances create ledgermonolith-service \
    --project $PROJECT_ID \
    --zone $ZONE \
    --network ${NETWORK=default} \
    --subnet ${SUBNET=default} \

I can't share this particular lab draft - as it is an ongoing examination lab that uses the BOA code - I'll work out an extract.

unit testing - not integration testing

michael@cloudshell:~/docproc-old/pubsec-declarative-toolkit/solutions/document-processing/gcloud (docproc-old)$ echo $NETWORK

michael@cloudshell:~/docproc-old/pubsec-declarative-toolkit/solutions/document-processing/gcloud (docproc-old)$ echo $SUBNET

michael@cloudshell:~/docproc-old/pubsec-declarative-toolkit/solutions/document-processing/gcloud (docproc-old)$ export REGION=us-central1
michael@cloudshell:~/docproc-old/pubsec-declarative-toolkit/solutions/document-processing/gcloud (docproc-old)$ export ZONE=$REGION-a
michael@cloudshell:~/docproc-old/pubsec-declarative-toolkit/solutions/document-processing/gcloud (docproc-old)$ export PROJECT_ID=docproc-old
michael@cloudshell:~/docproc-old/pubsec-declarative-toolkit/solutions/document-processing/gcloud (docproc-old)$ gcloud config set project docai-gen-4083
Updated property [core/project].
michael@cloudshell:~/docproc-old/pubsec-declarative-toolkit/solutions/document-processing/gcloud (docai-gen-4083)$ gcloud compute instances create ledgermonolith-service --project $PROJECT_ID --zone $ZONE --network ${NETWORK=default} --subnet ${SUBNET=default} --image-family=debian-10 --image-project=debian-cloud --machine-type=n1-standard-2
API [compute.googleapis.com] not enabled on project [281728183611]. Would you like to enable and retry (this will take a few minutes)? (y/N)?  y

Enabling service [compute.googleapis.com] on project [281728183611]...
Operation "operations/acf.p2-281728183611-7641d0a6-0db6-4ddc-9c38-565caa16832a" finished successfully.
Created [https://www.googleapis.com/compute/v1/projects/docproc-old/zones/us-central1-a/instances/ledgermonolith-service].
NAME: ledgermonolith-service
ZONE: us-central1-a
MACHINE_TYPE: n1-standard-2
PREEMPTIBLE:
INTERNAL_IP: 10.128.0.2
EXTERNAL_IP: 34....
STATUS: RUNNING

thanks Nim

patch up