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

Use compute v1 api to specify network tier #88487

Merged

Conversation

zioproto
Copy link
Member

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

Drop the use of the alpha compute api for operations that are supported by compute v1.
With this change the users of GKE will be able to test the annotation cloud.google.com/network-tier: Standard when creating a GKE alpha cluster, but without the need of whitelisting the project for the compute alpha api.

Bonus: the switch/case logic was wrong because the user can set the default tier for a project:
https://cloud.google.com/network-tiers/docs/using-network-service-tiers#setting_the_tier_for_all_resources_in_a_project
The assumption that the default tier is always PREMIUM is wrong
This patch uses the explicit network tier when possible,
or it falls back to the project default.

Which issue(s) this PR fixes:

Fixes #69314

Does this PR introduce a user-facing change?:

In GKE alpha clusters it will be possible to use the service annotation `cloud.google.com/network-tier: Standard`

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 24, 2020
@k8s-ci-robot k8s-ci-robot added area/cloudprovider sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 24, 2020
@dims
Copy link
Member

dims commented Feb 24, 2020

/assign @cheftako

@zioproto zioproto force-pushed the issues/69314-tier-config-support branch from b90ea05 to 9275d19 Compare February 24, 2020 20:59
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 24, 2020
Drop the use of the alpha api for operations that are supported by compute v1

The switch/case logic was wrong because the user
can set the default tier for a project:
https://cloud.google.com/network-tiers/docs/using-network-service-tiers#setting_the_tier_for_all_resources_in_a_project

The assumption that the default tier is always PREMIUM is wrong

This patch uses the explicit network tier when possible,
or it falls back to the project default.

Signed-off-by: Saverio Proto <saverioproto@google.com>
@zioproto zioproto force-pushed the issues/69314-tier-config-support branch from 9275d19 to bdc54eb Compare February 25, 2020 07:17
@zioproto
Copy link
Member Author

@cheftako @bowei : I am preparing a 2nd PR where I drop completely the alpha gate for Network Tier, because Network Tier is now GA in GCP. The 2nd PR depends on this one.
Do you prefer 2 separate PRs or a single one ?
Thanks

Copy link
Member

@cheftako cheftako left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 2, 2020
@mikedanese
Copy link
Member

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikedanese, zioproto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 3, 2020
@k8s-ci-robot k8s-ci-robot merged commit 481b04c into kubernetes:master Mar 3, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Mar 3, 2020
@zioproto
Copy link
Member Author

zioproto commented Mar 6, 2020

/cherrypick-candidate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cloudprovider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GCE Loadbalancer - Tier configuration support
6 participants