Skip to content
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.

Sign up for GitHub

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

[GCP] Add gVNIC support #4095

Merged
merged 3 commits into from
Oct 17, 2024
+25 −1
Merged

[GCP] Add gVNIC support #4095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a79abca
add gvnic support through config.yaml
romilbhardwaj Oct 16, 2024
6685656
lint
romilbhardwaj Oct 16, 2024
c35f023
docs
romilbhardwaj Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/source/reference/config.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,15 @@ Available fields and semantics:
# Default: 'LOCAL_CREDENTIALS'.
remote_identity: LOCAL_CREDENTIALS
# Enable gVNIC (optional).
#
# Set to true to use gVNIC on GCP instances. gVNIC offers higher performance
# for multi-node clusters, but costs more.
# Reference: https://cloud.google.com/compute/docs/networking/using-gvnic
#
# Default: false.
enable_gvnic: false
# Advanced Azure configurations (optional).
# Apply to all new instances but not existing ones.
azure:
Expand Down
5 changes: 5 additions & 0 deletions sky/clouds/gcp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,11 @@ def _failover_disk_tier() -> Optional[resources_utils.DiskTier]:
resources_vars[
'force_enable_external_ips'] = skypilot_config.get_nested(
('gcp', 'force_enable_external_ips'), False)

# Add gVNIC from config
resources_vars['enable_gvnic'] = skypilot_config.get_nested(
('gcp', 'enable_gvnic'), False)

return resources_vars

def _get_feasible_launchable_resources(
Expand Down
6 changes: 5 additions & 1 deletion sky/provision/gcp/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,8 +670,12 @@ def _configure_subnet(region: str, cluster_name: str,
'accessConfigs': [{
'name': 'External NAT',
'type': 'ONE_TO_ONE_NAT',
}],
}]
}]
# Add gVNIC if specified in config
enable_gvnic = config.provider_config.get('enable_gvnic', False)
if enable_gvnic:
default_interfaces[0]['nicType'] = 'gVNIC'
enable_external_ips = _enable_external_ips(config)
if not enable_external_ips:
# Removing this key means the VM will not be assigned an external IP.
Expand Down
3 changes: 3 additions & 0 deletions sky/templates/gcp-ray.yml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ provider:
# leakage.
disable_launch_config_check: true
use_managed_instance_group: {{ gcp_use_managed_instance_group }}
{%- if enable_gvnic %}
enable_gvnic: {{ enable_gvnic }}
{%- endif %}
Comment on lines +67 to +69
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: should this be in provider_config or in the node_config below. I would vote for adding to the node_config below instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to add to node_config too, but realized node_config mainly contains fields that are directly passed to gcp APIs (bulk_insert). Adding to node_config would require special casing with a .pop('enable_gvnic') in downstream methods. Decided it's best to avoid non-GCP API fields in node_config.


auth:
ssh_user: gcpuser
Expand Down
3 changes: 3 additions & 0 deletions sky/utils/schemas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,9 @@ def get_config_schema():
'force_enable_external_ips': {
'type': 'boolean'
},
'enable_gvnic': {
'type': 'boolean'
},
**_LABELS_SCHEMA,
**_NETWORK_CONFIG_SCHEMA,
},
Expand Down
Loading