Skip to content
This repository has been archived by the owner on Dec 31, 2023. It is now read-only.

docs(samples): Adding template samples #136

Merged
merged 20 commits into from Nov 17, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4e46538
chore(docs): Instance template creation samples
m-strzelczyk Oct 18, 2021
0486123
Merge branch 'main' into samples-templates
m-strzelczyk Oct 19, 2021
35be7ec
chore(docs): Adding docstrings.
m-strzelczyk Oct 19, 2021
d2b43a1
Merge branch 'main' into samples-templates
m-strzelczyk Oct 19, 2021
d027304
Merge branch 'main' into samples-templates
m-strzelczyk Oct 19, 2021
ee4d8ae
chore(docs): Applying suggested changes.
m-strzelczyk Oct 21, 2021
41512b9
chore(docs): Trying to fix test errors.
m-strzelczyk Oct 21, 2021
758069e
Merge branch 'main' into samples-templates
m-strzelczyk Nov 3, 2021
83c1a1b
Merge branch 'main' into samples-templates
parthea Nov 4, 2021
3896df2
Merge branch 'main' into samples-templates
m-strzelczyk Nov 8, 2021
a41ee95
Apply suggestions from code review
m-strzelczyk Nov 8, 2021
deffb61
chore(docs): Applying changes from review.
m-strzelczyk Nov 8, 2021
7301bbd
Merge branch 'samples-templates' of github.com:googleapis/python-comp…
m-strzelczyk Nov 8, 2021
fe3d984
chore(docs): Fixing region tag.
m-strzelczyk Nov 8, 2021
5d67138
Merge branch 'main' into samples-templates
m-strzelczyk Nov 9, 2021
80bb1d8
chore(samples): Updating tests according to review.
m-strzelczyk Nov 15, 2021
889b4aa
Merge branch 'main' into samples-templates
m-strzelczyk Nov 15, 2021
c6993cf
chore(samples): Fixing the fixture import for tests to work.
m-strzelczyk Nov 15, 2021
702ea0e
Merge branch 'main' into samples-templates
parthea Nov 16, 2021
1e809cc
Merge branch 'main' into samples-templates
m-strzelczyk Nov 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
195 changes: 195 additions & 0 deletions samples/snippets/sample_templates.py
@@ -0,0 +1,195 @@
# Copyright 2021 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.
from typing import Iterable

from google.cloud import compute_v1
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved


def get_instance_template(project_id: str, template_name: str) -> compute_v1.InstanceTemplate:
Copy link
Contributor

Choose a reason for hiding this comment

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

generally the preferred style for newly written samples is one sample per file, as it allows for imports to be included in the region tags: https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/AUTHORING_GUIDE.md#region-tags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the guide you linked doesn't really state that there should be 1:1 ratio between files and samples and I've been doing multiple samples per file for some time now in this repo. See for example vm creation samples. This way imports are included in the samples just fine.

I've read the internal "Code Snippets 201" guide and figured that this is the best way to group the samples. I plan to reorganize the samples soon, to introduce more structure with folders for example. If there are reasons to introduce 1 sample = 1 file division, I'd like to know it.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason is essentially enforcement/ease of use. With your samples, their is only one import so it's very complicated. However if a new import would be added for whatever reason, it should only apply to the sample using it, which becomes more complicated. Same with when an import needs to be removed - if it's no longer using it in the sample (but is in a different sample in the same file), flake8 can't catch that it's only being used in some region tags but not all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I see your point. I will reorganize those samples soon, as we were planning to do a bigger refactoring of how we keep the samples for the Compute documentation in multiple languages.

Could you just suggest what would be the best course of action, when there are rather big pieces of code repeating in multiple samples like in the vm creation samples? There, the create_with_disks method is used by 6 different samples. Can a region snap multiple files? If not, I will have to replicate this code multiple times...

Copy link
Contributor

Choose a reason for hiding this comment

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

It depends on how you arrange the samples on the page. If there really is a certain amount of boiler plate, you could create a different region tag for that section and have a different region tag that references. However, it's probably feasible to just integrate it as part of the main samples.

"""
Retrieve information about given instance template.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

Args:
project_id: project ID or project number of the Cloud project you use.
template_name: name of the template you want to get.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

Returns:
InstanceTemplate object containing information about given template.
"""
template_client = compute_v1.InstanceTemplatesClient()
return template_client.get(project=project_id, instance_template=template_name)


def list_instance_templates(project_id: str) -> Iterable[compute_v1.InstanceTemplate]:
"""
Get a list of InstanceTemplate objects available in given project.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

Args:
project_id: project ID or project number of the Cloud project you use.

Returns:
Iterable list of InstanceTemplate objects.
"""
template_client = compute_v1.InstanceTemplatesClient()
return template_client.list(project=project_id)


def create_template(project_id: str, template_name: str) -> compute_v1.InstanceTemplate:
"""
Create a new template in given project with provided name.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

Args:
project_id: project ID or project number of the Cloud project you use.
template_name: name of the new template that is created.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

Returns:
InstanceTemplate object describing the new instance template.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this say "...object that represents the..." instead? It's a little unclear why it would only describe the template rather than be the template itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, represents is better.

"""
# Describe the size and source image of the boot disk to attach to the instance.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved
disk = compute_v1.AttachedDisk()
initialize_params = compute_v1.AttachedDiskInitializeParams()
initialize_params.source_image = (
"projects/debian-cloud/global/images/family/debian-11"
)
initialize_params.disk_size_gb = 250
disk.initialize_params = initialize_params
disk.auto_delete = True
disk.boot = True

# Use the default network, without specifying a subnetwork.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved
network_interface = compute_v1.NetworkInterface()
network_interface.name = "global/networks/default"

# Let the instance have external IP
Copy link
Contributor

Choose a reason for hiding this comment

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

For external IP:
"The template lets the instance use an external IP address through Cloud NAT."

I attempted to clarify, but correct me if I'm misunderstanding the behavior here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not that simple and I'm not sure how to describe it well... ONE_TO_ONE_NAT actually means the Cloud NAT is not involved (that's a weird, I know). But regular NAT routing takes place to deliver the traffic.

See here:

The Compute Engine VM's network interface's primary internal IP address, provided that the network interface doesn't have an external IP address assigned to it. If the network interface has an external IP address assigned to it, Google Cloud automatically performs one-to-one NAT for packets whose sources match the interface's primary internal IP address because the network interface meets the Google Cloud internet access requirements. The existence of an external IP address on an interface always takes precedence and always performs one-to-one NAT, without using Cloud NAT.

Cloud NAT is used when you have a bunch of VMs with only internal IPs and you want to give them Internet access. They can "share" one external IP then, to talk with the Internet and our Cloud NAT is making sure that the traffic is routed properly to those instances.

I'm honestly not sure about the origin on the ONE_TO_ONE_NAT value, why is it called like that... Perhaps it's because there's basically no address translation? The IP stays as is, so it's one-to-one like that?

network_interface.access_configs = [compute_v1.AccessConfig()]
network_interface.access_configs[0].name = "External NAT"
network_interface.access_configs[0].type_ = compute_v1.AccessConfig.Type.ONE_TO_ONE_NAT
network_interface.access_configs[0].network_tier = (
compute_v1.AccessConfig.NetworkTier.PREMIUM
)
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

template = compute_v1.InstanceTemplate()
template.name = template_name
template.properties.disks = [disk]
template.properties.machine_type = "e2-standard-4"
template.properties.network_interfaces = [network_interface]

template_client = compute_v1.InstanceTemplatesClient()
operation_client = compute_v1.GlobalOperationsClient()
op = template_client.insert(project=project_id, instance_template_resource=template)
operation_client.wait(project=project_id, operation=op.name)

return template_client.get(project=project_id, instance_template=template_name)


def create_template_from_instance(project_id: str, instance: str, template_name: str) -> compute_v1.InstanceTemplate:
"""
Create a new instance template based on an existing instance.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

Args:
project_id: project ID or project number of the Cloud project you use.
instance: the instance the new template will be based on. This value uses
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved
the following format: "projects/{project}/zones/{zone}/instances/{instance_name}"
template_name: name of the new template that is created.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

Returns:
InstanceTemplate object describing the new instance template.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same note as above. Does the InstanceTemplate object describe or represent the new template?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kind of both... But I like the represent notion, as it reminds that the actual template is somewhere out there in the cloud.

"""
disk = compute_v1.DiskInstantiationConfig()
# This name must match the name of a disk attached to the instance you are
Copy link
Contributor

Choose a reason for hiding this comment

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

It's unclear why this must match when other aspects can be changed. For example, you replace the image with a Rocky Linux image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but this tells the system which disk we're modifying.

So let's say this one instance has two disks: disk-1 and disk-2. One is a boot disk, other is just some data disk. The template we want to create is supposed to replace image for only one of those disks (i.e. disk-1). That's why it must match. Can you help with phrasing it clearly?

# basing your template on.
disk.device_name = "disk-1"
# Replace the original disk image used in your instance with a Rocky Linux image.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved
disk.instantiate_from = (
compute_v1.DiskInstantiationConfig.InstantiateFrom.CUSTOM_IMAGE
)
disk.custom_image = "projects/rocky-linux-cloud/global/images/family/rocky-linux-8"
# You can override the auto_delete setting.
Copy link
Contributor

Choose a reason for hiding this comment

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

As written, this raises some questions like "What else can I override?". If this change is specific to this example, we can probably change to just say "Override the auto_delete setting."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can't really override anything else. See here to see what the DiskInstantiationConfig can fit.

I'll change to comment as suggested.

disk.auto_delete = True

template = compute_v1.InstanceTemplate()
template.name = template_name
template.source_instance = instance
template.source_instance_params = compute_v1.SourceInstanceParams()
template.source_instance_params.disk_configs = [disk]

template_client = compute_v1.InstanceTemplatesClient()
operation_client = compute_v1.GlobalOperationsClient()
op = template_client.insert(project=project_id, instance_template_resource=template)
operation_client.wait(project=project_id, operation=op.name)

return template_client.get(project=project_id, instance_template=template_name)


def create_template_with_subnet(
project_id: str, network: str, subnetwork: str, template_name: str
) -> compute_v1.InstanceTemplate:
"""
Create an instance template that will use a provided subnet.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

Args:
project_id: project ID or project number of the Cloud project you use.
network: the network to be used in the new template. This value uses
the following format: "projects/{project}/global/networks/{network}"
subnetwork: the subnetwork to be used in the new template. This value
uses the following format: "projects/{project}/regions/{region}/subnetworks/{subnetwork}"
template_name: name of the new template that is created.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

Returns:
InstanceTemplate object describing the new instance template.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved
"""
# Describe the size and source image of the boot disk to attach to the instance.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved
disk = compute_v1.AttachedDisk()
initialize_params = compute_v1.AttachedDiskInitializeParams()
initialize_params.source_image = (
"projects/debian-cloud/global/images/family/debian-11"
)
initialize_params.disk_size_gb = 250
disk.initialize_params = initialize_params
disk.auto_delete = True
disk.boot = True

template = compute_v1.InstanceTemplate()
template.name = template_name
template.properties = compute_v1.InstanceProperties()
template.properties.disks = [disk]
template.properties.machine_type = "e2-standard-4"

# Make this template use provided subnetwork.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved
network_interface = compute_v1.NetworkInterface()
network_interface.network = network
network_interface.subnetwork = subnetwork
template.properties.network_interfaces = [network_interface]

template_client = compute_v1.InstanceTemplatesClient()
operation_client = compute_v1.GlobalOperationsClient()
op = template_client.insert(project=project_id, instance_template_resource=template)
operation_client.wait(project=project_id, operation=op.name)

return template_client.get(project=project_id, instance_template=template_name)


def delete_instance_template(project_id: str, template_name: str):
"""
Delete an instance template.

Args:
project_id: project ID or project number of the Cloud project you use.
template_name: name of the new template to delete.
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved
"""
template_client = compute_v1.InstanceTemplatesClient()
operation_client = compute_v1.GlobalOperationsClient()
op = template_client.delete(project=project_id, instance_template=template_name)
operation_client.wait(project=project_id, operation=op.name)
return
3 changes: 2 additions & 1 deletion samples/snippets/test_sample_start_stop.py
Expand Up @@ -22,7 +22,7 @@

import pytest

from samples.snippets.sample_start_stop import start_instance, start_instance_with_encryption_key, stop_instance
from sample_start_stop import start_instance, start_instance_with_encryption_key, stop_instance

PROJECT = google.auth.default()[1]

Expand All @@ -43,6 +43,7 @@ def _make_disk(raw_key: bytes = None):
disk.auto_delete = True
disk.boot = True
disk.type_ = compute_v1.AttachedDisk.Type.PERSISTENT
disk.device_name = 'disk-1'

if raw_key:
disk.disk_encryption_key = compute_v1.CustomerEncryptionKey()
Expand Down
98 changes: 98 additions & 0 deletions samples/snippets/test_sample_templates.py
@@ -0,0 +1,98 @@
# Copyright 2021 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.

import uuid

import google.auth

from sample_templates import (
create_template,
create_template_from_instance,
create_template_with_subnet,
delete_instance_template,
list_instance_templates,
)

from test_sample_start_stop import compute_instance
# Needed to make the flake8 happy.
assert compute_instance
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved

PROJECT = google.auth.default()[1]

INSTANCE_ZONE = "europe-central2-b"


def test_create_instance():
template_name = "i" + uuid.uuid4().hex[:10]
template = create_template(PROJECT, template_name)

try:
assert template.name == template_name
assert template.properties.disks[0].initialize_params.disk_size_gb == 250
assert (
"debian-11" in template.properties.disks[0].initialize_params.source_image
)
assert (
template.properties.network_interfaces[0].name == "global/networks/default"
)
assert template.properties.machine_type == "e2-standard-4"
finally:
delete_instance_template(PROJECT, template_name)
m-strzelczyk marked this conversation as resolved.
Show resolved Hide resolved
assert all(
template.name != template_name for template in list_instance_templates(PROJECT)
)


def test_create_from_instance(compute_instance):
template_name = "i" + uuid.uuid4().hex[:10]
template = create_template_from_instance(
PROJECT, compute_instance.self_link, template_name
)

try:
assert template.name == template_name
assert template.properties.machine_type in compute_instance.machine_type
assert (
template.properties.disks[0].disk_size_gb
== compute_instance.disks[0].disk_size_gb
)
assert (
template.properties.disks[0].initialize_params.source_image
== "projects/rocky-linux-cloud/global/images/family/rocky-linux-8"
)
finally:
delete_instance_template(PROJECT, template_name)


def test_create_template_with_subnet():
template_name = "i" + uuid.uuid4().hex[:10]
template = create_template_with_subnet(
PROJECT,
"global/networks/default",
"regions/asia-east1/subnetworks/default",
template_name,
)

try:
assert template.name == template_name
assert (
"global/networks/default"
in template.properties.network_interfaces[0].network
)
assert (
"regions/asia-east1/subnetworks/default"
in template.properties.network_interfaces[0].subnetwork
)
finally:
delete_instance_template(PROJECT, template_name)