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.

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

GKE Core platform updates and enhancements #82

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

arueth
Copy link
Collaborator

@arueth arueth commented Dec 24, 2024

No description provided.

Copy link
Member

@ferrarimarco ferrarimarco left a comment

Choose a reason for hiding this comment

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

Thanks for this PR. A few questions about variables.

enable_private_nodes = true
}

name = "default-pool"
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to have a variable here? It will be useful for the federated learning use case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This has to be default-pool, it gets deleted because remove_default_node_pool = true

node_config {
machine_type = "e2-standard-4"
machine_type = var.cluster_system_node_pool_machine_type
service_account = google_service_account.cluster.email
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to have a variable here and use google_service_account.cluster.email is the user didn't specify anything? This will be useful for the federated learning use case

cluster = google_container_cluster.cluster.name
initial_node_count = 1
location = var.cluster_region
name = "system"
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to have a variable here in order to customize the name? It will be useful for the federated learning use case. Thanks

node_config {
machine_type = "e2-standard-4"
machine_type = var.cluster_system_node_pool_machine_type
Copy link
Member

Choose a reason for hiding this comment

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

Just checking: we're reusing the same variable as we're using in the system node pool below.

enable_private_nodes = true
}

name = "default-pool"
node_config {
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to add variables to specify custom labels and taints for this node pool? This will be useful for the federated learning use case.

location = var.cluster_region
name = local.cluster_name
project = var.cluster_project_id
node_config {
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to add variables to specify custom labels and taints for this node pool? This will be useful for the federated learning use case.

"resource-type" : "system"
}
machine_type = var.cluster_system_node_pool_machine_type
service_account = google_service_account.cluster.email
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to have a variable here and use google_service_account.cluster.email is the user didn't specify anything? This will be useful for the federated learning use case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants