Remove hard coded configuration values use labels for filtering

chris-sanders commented 3 years ago

While working with the scripts I've had a few complications that I've made some changes for which I'll commit back but wanted to have a discussion on the intention before I go to far down this path.

Between the dev and qa environments I'm finding the scripts are. hardcoding things that provide unintuitive results. For example:

The create command hard codes the qa environment but the list and delete commands don't.
Zone isn't consistently set some commands use an env others don't supply a zone at all
The list command won't show the standard codeserver instances b/c we have a different naming convention

I think two changes could be made which would make the commands more generically usable, less surprising, and hopefully easier to maintain moving forward.

Do not specify projects or zones. Settings should be managed by configurations and command should always run in that configurations context
Switch to an owner label instead of name prefixes

You can see what this looks like with my WIP currently Here

Here's an example of creating, listing, and deleting

╭chris:~ %
╰➤ glist
Configuration: qa
WARNING: The following filter keys were not present in any resource : labels.owner
Listed 0 items.
╭chris:~ %
╰➤ gcreate ubuntu-minimal-2004-focal-v20210720 {test-app,test-app2,test-three-app}
Configuration: qa
+gcreate:15> echo test-app test-app2 test-three-app
+gcreate:15> gcloud compute instances create test-app test-app2 test-three-app --labels 'owner=chriss' '--machine-type=n1-standard-4' '--subnet=default' '--network-tier=PREMIUM' '--maintenance-policy=MIGRATE' '--service-account=846065462912-compute@developer.gserviceaccount.com' '--scopes=https://www.googleapis.com/auth/devstorage.read_only,https://www.googleapis.com/auth/logging.write,https://www.googleapis.com/auth/monitoring.write,https://www.googleapis.com/auth/servicecontrol,https://www.googleapis.com/auth/service.management.readonly,https://www.googleapis.com/auth/trace.append' '--image=ubuntu-minimal-2004-focal-v20210720' '--image-project=ubuntu-os-cloud' '--boot-disk-size=200GB' '--boot-disk-type=pd-standard' --no-shielded-secure-boot --shielded-vtpm --shielded-integrity-monitoring '--reservation-affinity=any'
Created [https://www.googleapis.com/compute/v1/projects/replicated-qa/zones/us-central1-a/instances/test-app].
Created [https://www.googleapis.com/compute/v1/projects/replicated-qa/zones/us-central1-a/instances/test-app2].
Created [https://www.googleapis.com/compute/v1/projects/replicated-qa/zones/us-central1-a/instances/test-three-app].
WARNING: Some requests generated warnings:
 - Disk size: '200 GB' is larger than image size: '10 GB'. You might need to resize the root repartition manually if the operating system does not support automatic resizing. See https://cloud.google.com/compute/docs/disks/add-persistent-disk#resize_pd for details.

NAME            ZONE           MACHINE_TYPE   PREEMPTIBLE  INTERNAL_IP  EXTERNAL_IP     STATUS
test-app        us-central1-a  n1-standard-4               10.128.0.73  34.121.13.57    RUNNING
test-app2       us-central1-a  n1-standard-4               10.128.0.80  34.123.235.234  RUNNING
test-three-app  us-central1-a  n1-standard-4               10.128.0.78  34.132.52.185   RUNNING
╭chris:~ %
╰➤ glist
Configuration: qa
NAME            ZONE           MACHINE_TYPE   PREEMPTIBLE  INTERNAL_IP  EXTERNAL_IP     STATUS
test-app        us-central1-a  n1-standard-4               10.128.0.73  34.121.13.57    RUNNING
test-app2       us-central1-a  n1-standard-4               10.128.0.80  34.123.235.234  RUNNING
test-three-app  us-central1-a  n1-standard-4               10.128.0.78  34.132.52.185   RUNNING
╭chris:~ %
╰➤ gdelete test-app
Configuration: qa
The following instances will be deleted. Any attached disks configured
 to be auto-deleted will be deleted unless they are attached to any
other instances or the `--keep-disks` flag is given and specifies them
 for keeping. Deleting a disk is irreversible and any data on the disk
 will be lost.
 - [test-app2] in [us-central1-a]
 - [test-app] in [us-central1-a]

Do you want to continue (Y/n)?  Y

Deleted [https://www.googleapis.com/compute/v1/projects/replicated-qa/zones/us-central1-a/instances/test-app].
Deleted [https://www.googleapis.com/compute/v1/projects/replicated-qa/zones/us-central1-a/instances/test-app2].
╭chris:~ %
╰➤ glist
Configuration: qa
NAME            ZONE           MACHINE_TYPE   PREEMPTIBLE  INTERNAL_IP  EXTERNAL_IP    STATUS
test-three-app  us-central1-a  n1-standard-4               10.128.0.78  34.132.52.185  RUNNING

While you could leave the name prefix and still use labels, the scripts are a lot easier to maintain when you don't have to mess with column and string manipulation to find the items that belong to you.

emosbaugh commented 3 years ago

This repo was born out of some commands copied from my bash profile. It could use some improving. This sounds like a positive change and contributions are always welcome! Glad you are finding this useful.

Only one question. Is it difficult to tell in the browser console who is the owner of the instance when not in the name itself? I only ask cause sometimes we have to explicitly ask that folks delete long running instances that are no longer in use.

chris-sanders commented 3 years ago

@emosbaugh that's a great question, and I did check. You can add the Labels column to the console. I've done that and I'm assuming that's a per-person setting but if an admin could do that by default it would save everyone from having to ask the same questions.

Glad you like it, I'll continue down this path then and try out commands other than list/create/delete and get a PR together.

emosbaugh / gcommands

Remove hard coded configuration values use labels for filtering #4