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

Drop the alpha gating for Network Service Tier configuration support #88532

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 4 additions & 26 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_addresses.go
Expand Up @@ -23,7 +23,6 @@ import (

"k8s.io/klog"

computealpha "google.golang.org/api/compute/v0.alpha"
computebeta "google.golang.org/api/compute/v0.beta"
compute "google.golang.org/api/compute/v1"

Expand Down Expand Up @@ -80,15 +79,6 @@ func (g *Cloud) ReserveRegionAddress(addr *compute.Address, region string) error
return mc.Observe(g.c.Addresses().Insert(ctx, meta.RegionalKey(addr.Name, region), addr))
}

// ReserveAlphaRegionAddress creates an Alpha, regional address.
func (g *Cloud) ReserveAlphaRegionAddress(addr *computealpha.Address, region string) error {
ctx, cancel := cloud.ContextWithCallTimeout()
defer cancel()

mc := newAddressMetricContext("reserve", region)
return mc.Observe(g.c.AlphaAddresses().Insert(ctx, meta.RegionalKey(addr.Name, region), addr))
}

// ReserveBetaRegionAddress creates a beta region address
func (g *Cloud) ReserveBetaRegionAddress(addr *computebeta.Address, region string) error {
ctx, cancel := cloud.ContextWithCallTimeout()
Expand Down Expand Up @@ -117,16 +107,6 @@ func (g *Cloud) GetRegionAddress(name, region string) (*compute.Address, error)
return v, mc.Observe(err)
}

// GetAlphaRegionAddress returns the Alpha, regional address by name.
Copy link
Member

Choose a reason for hiding this comment

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

We aren't allowing alpha addresses to be gotten but they can still be created? (ReserveAlphaRegionAddress() above)

Copy link
Member Author

Choose a reason for hiding this comment

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

@cheftako thanks for spotting this ! I pushed a new commit that fixes it.

func (g *Cloud) GetAlphaRegionAddress(name, region string) (*computealpha.Address, error) {
ctx, cancel := cloud.ContextWithCallTimeout()
defer cancel()

mc := newAddressMetricContext("get", region)
v, err := g.c.AlphaAddresses().Get(ctx, meta.RegionalKey(name, region))
return v, mc.Observe(err)
}

// GetBetaRegionAddress returns the beta region address by name
func (g *Cloud) GetBetaRegionAddress(name, region string) (*computebeta.Address, error) {
ctx, cancel := cloud.ContextWithCallTimeout()
Expand Down Expand Up @@ -185,14 +165,12 @@ func (g *Cloud) GetBetaRegionAddressByIP(region, ipAddress string) (*computebeta
return nil, makeGoogleAPINotFoundError(fmt.Sprintf("Address with IP %q was not found in region %q", ipAddress, region))
}

// TODO(#51665): retire this function once Network Tiers becomes Beta in GCP.
func (g *Cloud) getNetworkTierFromAddress(name, region string) (string, error) {
if !g.AlphaFeatureGate.Enabled(AlphaFeatureNetworkTiers) {
return cloud.NetworkTierDefault.ToGCEValue(), nil
}
addr, err := g.GetAlphaRegionAddress(name, region)

addr, err := g.GetRegionAddress(name, region)
if err != nil {
return handleAlphaNetworkTierGetError(err)
// Can't get the network tier, just return an error.
return "", err
}
return addr.NetworkTier, nil
}
Expand Down
5 changes: 0 additions & 5 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_alpha.go
Expand Up @@ -19,11 +19,6 @@ limitations under the License.
package gce

const (
// AlphaFeatureNetworkTiers allows Services backed by a GCP load balancer to choose
// what network tier to use. Currently supports "Standard" and "Premium" (default).
//
// alpha: v1.8 (for Services)
AlphaFeatureNetworkTiers = "NetworkTiers"
// AlphaFeatureILBSubsets allows InternalLoadBalancer services to include a subset
// of cluster nodes as backends instead of all nodes.
AlphaFeatureILBSubsets = "ILBSubsets"
Expand Down
Expand Up @@ -182,14 +182,11 @@ func (g *Cloud) DeleteRegionForwardingRule(name, region string) error {
return mc.Observe(g.c.ForwardingRules().Delete(ctx, meta.RegionalKey(name, region)))
}

// TODO(#51665): retire this function once Network Tiers becomes Beta in GCP.
func (g *Cloud) getNetworkTierFromForwardingRule(name, region string) (string, error) {
if !g.AlphaFeatureGate.Enabled(AlphaFeatureNetworkTiers) {
return cloud.NetworkTierDefault.ToGCEValue(), nil
}
fwdRule, err := g.GetAlphaRegionForwardingRule(name, region)
fwdRule, err := g.GetRegionForwardingRule(name, region)
if err != nil {
return handleAlphaNetworkTierGetError(err)
// Can't get the network tier, just return an error.
return "", err
}
return fwdRule.NetworkTier, nil
}
12 changes: 1 addition & 11 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_interfaces.go
Expand Up @@ -19,7 +19,6 @@ limitations under the License.
package gce

import (
computealpha "google.golang.org/api/compute/v0.alpha"
computebeta "google.golang.org/api/compute/v0.beta"
compute "google.golang.org/api/compute/v1"
)
Expand All @@ -34,16 +33,11 @@ type CloudAddressService interface {
DeleteRegionAddress(name, region string) error
// TODO: Mock Global endpoints

// Alpha API.
GetAlphaRegionAddress(name, region string) (*computealpha.Address, error)
ReserveAlphaRegionAddress(addr *computealpha.Address, region string) error

// Beta API
ReserveBetaRegionAddress(address *computebeta.Address, region string) error
GetBetaRegionAddress(name string, region string) (*computebeta.Address, error)
GetBetaRegionAddressByIP(region, ipAddress string) (*computebeta.Address, error)

// TODO(#51665): Remove this once the Network Tiers becomes Alpha in GCP.
getNetworkTierFromAddress(name, region string) (string, error)
}

Expand All @@ -54,10 +48,6 @@ type CloudForwardingRuleService interface {
CreateRegionForwardingRule(rule *compute.ForwardingRule, region string) error
DeleteRegionForwardingRule(name, region string) error

// Alpha API.
GetAlphaRegionForwardingRule(name, region string) (*computealpha.ForwardingRule, error)
CreateAlphaRegionForwardingRule(rule *computealpha.ForwardingRule, region string) error

// Needed for the Alpha "Network Tiers" feature.
// Needed for the "Network Tiers" feature.
getNetworkTierFromForwardingRule(name, region string) (string, error)
}
Expand Up @@ -81,9 +81,7 @@ func (g *Cloud) ensureExternalLoadBalancer(clusterName string, clusterID string,
return nil, err
}
klog.V(4).Infof("ensureExternalLoadBalancer(%s): Desired network tier %q.", lbRefStr, netTier)
if g.AlphaFeatureGate.Enabled(AlphaFeatureNetworkTiers) {
g.deleteWrongNetworkTieredResources(loadBalancerName, lbRefStr, netTier)
}
g.deleteWrongNetworkTieredResources(loadBalancerName, lbRefStr, netTier)

// Check if the forwarding rule exists, and if so, what its IP is.
fwdRuleExists, fwdRuleNeedsUpdate, fwdRuleIP, err := g.forwardingRuleNeedsUpdate(loadBalancerName, g.region, requestedIP, ports)
Expand Down Expand Up @@ -1072,9 +1070,6 @@ func ensureStaticIP(s CloudAddressService, name, serviceName, region, existingIP
}

func (g *Cloud) getServiceNetworkTier(svc *v1.Service) (cloud.NetworkTier, error) {
if !g.AlphaFeatureGate.Enabled(AlphaFeatureNetworkTiers) {
return cloud.NetworkTierDefault, nil
}
tier, err := GetServiceNetworkTier(svc)
if err != nil {
// Returns an error if the annotation is invalid.
Expand Down
Expand Up @@ -233,9 +233,6 @@ func TestDeleteAddressWithWrongTier(t *testing.T) {
s, err := fakeGCECloud(DefaultTestClusterValues())
require.NoError(t, err)

// Enable the cloud.NetworkTiers feature
s.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true

for desc, tc := range map[string]struct {
addrName string
netTier cloud.NetworkTier
Expand Down Expand Up @@ -401,8 +398,6 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) {
gce, err := fakeGCECloud(vals)
require.NoError(t, err)

// Enable the cloud.NetworkTiers feature
gce.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true
svc := fakeLoadbalancerService("")
svc.Annotations = map[string]string{NetworkTierAnnotationKey: "Premium"}

Expand Down Expand Up @@ -460,8 +455,6 @@ func TestEnsureExternalLoadBalancerFailsIfInvalidNetworkTier(t *testing.T) {
nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName)
require.NoError(t, err)

// Enable the cloud.NetworkTiers feature
gce.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true
svc := fakeLoadbalancerService("")
svc.Annotations = map[string]string{NetworkTierAnnotationKey: wrongTier}

Expand Down Expand Up @@ -834,7 +827,6 @@ func TestDeleteWrongNetworkTieredResourcesSucceedsWhenNotFound(t *testing.T) {
gce, err := fakeGCECloud(DefaultTestClusterValues())
require.NoError(t, err)

gce.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true
assert.Nil(t, gce.deleteWrongNetworkTieredResources("Wrong_LB_Name", "", cloud.NetworkTier("")))
}

Expand Down
12 changes: 0 additions & 12 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_util.go
Expand Up @@ -281,18 +281,6 @@ func makeGoogleAPINotFoundError(message string) error {
return &googleapi.Error{Code: http.StatusNotFound, Message: message}
}

// TODO(#51665): Remove this once Network Tiers becomes Beta in GCP.
func handleAlphaNetworkTierGetError(err error) (string, error) {
if isForbidden(err) {
// Network tier is still an Alpha feature in GCP, and not every project
// is whitelisted to access the API. If we cannot access the API, just
// assume the tier is premium.
return cloud.NetworkTierDefault.ToGCEValue(), nil
}
// Can't get the network tier, just return an error.
return "", err
}

// containsCIDR returns true if outer contains inner.
func containsCIDR(outer, inner *net.IPNet) bool {
return outer.Contains(firstIPInRange(inner)) && outer.Contains(lastIPInRange(inner))
Expand Down
1 change: 0 additions & 1 deletion test/e2e/network/BUILD
Expand Up @@ -86,7 +86,6 @@ go_library(
"//vendor/github.com/miekg/dns:go_default_library",
"//vendor/github.com/onsi/ginkgo:go_default_library",
"//vendor/github.com/onsi/gomega:go_default_library",
"//vendor/google.golang.org/api/compute/v0.alpha:go_default_library",
"//vendor/google.golang.org/api/compute/v1:go_default_library",
"//vendor/k8s.io/utils/net:go_default_library",
],
Expand Down
21 changes: 10 additions & 11 deletions test/e2e/network/network_tiers.go
Expand Up @@ -21,7 +21,7 @@ import (
"net/http"
"time"

computealpha "google.golang.org/api/compute/v0.alpha"
compute "google.golang.org/api/compute/v1"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
v1 "k8s.io/api/core/v1"
Expand All @@ -37,7 +37,7 @@ import (
"github.com/onsi/ginkgo"
)

var _ = SIGDescribe("Services [Feature:GCEAlphaFeature][Slow]", func() {
var _ = SIGDescribe("Services [Slow]", func() {
f := framework.NewDefaultFramework("services")

var cs clientset.Interface
Expand Down Expand Up @@ -109,7 +109,7 @@ var _ = SIGDescribe("Services [Feature:GCEAlphaFeature][Slow]", func() {
requestedAddrName := fmt.Sprintf("e2e-ext-lb-net-tier-%s", framework.RunID)
gceCloud, err := gce.GetGCECloud()
framework.ExpectNoError(err)
requestedIP, err := reserveAlphaRegionalAddress(gceCloud, requestedAddrName, cloud.NetworkTierStandard)
requestedIP, err := reserveRegionalAddress(gceCloud, requestedAddrName, cloud.NetworkTierStandard)
framework.ExpectNoError(err, "failed to reserve a STANDARD tiered address")
defer func() {
if requestedAddrName != "" {
Expand Down Expand Up @@ -156,7 +156,7 @@ func waitAndVerifyLBWithTier(jig *e2eservice.TestJig, existingIP string, waitTim
}
// If the IP has been used by previous test, sometimes we get the lingering
// 404 errors even after the LB is long gone. Tolerate and retry until the
// the new LB is fully established since this feature is still Alpha in GCP.
// the new LB is fully established.
e2eservice.TestReachableHTTPWithRetriableErrorCodes(ingressIP, svcPort, []int{http.StatusNotFound}, checkTimeout)

// Verify the network tier matches the desired.
Expand All @@ -170,7 +170,7 @@ func waitAndVerifyLBWithTier(jig *e2eservice.TestJig, existingIP string, waitTim
}

func getLBNetworkTierByIP(ip string) (cloud.NetworkTier, error) {
var rule *computealpha.ForwardingRule
var rule *compute.ForwardingRule
// Retry a few times to tolerate flakes.
err := wait.PollImmediate(5*time.Second, 15*time.Second, func() (bool, error) {
obj, err := getGCEForwardingRuleByIP(ip)
Expand All @@ -186,12 +186,12 @@ func getLBNetworkTierByIP(ip string) (cloud.NetworkTier, error) {
return cloud.NetworkTierGCEValueToType(rule.NetworkTier), nil
}

func getGCEForwardingRuleByIP(ip string) (*computealpha.ForwardingRule, error) {
func getGCEForwardingRuleByIP(ip string) (*compute.ForwardingRule, error) {
cloud, err := gce.GetGCECloud()
if err != nil {
return nil, err
}
ruleList, err := cloud.ListAlphaRegionForwardingRules(cloud.Region())
ruleList, err := cloud.ListRegionForwardingRules(cloud.Region())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -220,14 +220,13 @@ func clearNetworkTier(svc *v1.Service) {
}

// TODO: add retries if this turns out to be flaky.
// TODO(#51665): remove this helper function once Network Tiers becomes beta.
func reserveAlphaRegionalAddress(cloud *gcecloud.Cloud, name string, netTier cloud.NetworkTier) (string, error) {
alphaAddr := &computealpha.Address{
func reserveRegionalAddress(cloud *gcecloud.Cloud, name string, netTier cloud.NetworkTier) (string, error) {
Addr := &compute.Address{
Name: name,
NetworkTier: netTier.ToGCEValue(),
}

if err := cloud.ReserveAlphaRegionAddress(alphaAddr, cloud.Region()); err != nil {
if err := cloud.ReserveRegionAddress(Addr, cloud.Region()); err != nil {
return "", err
}

Expand Down