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

Feature: Add ServiceAnnotationLoadBalancerIP for deprecated LoadBalancerIP #1920

Closed
wants to merge 1 commit into from
Closed

Feature: Add ServiceAnnotationLoadBalancerIP for deprecated LoadBalancerIP #1920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions pkg/consts/consts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ const (
// to specify what subnet it is exposed on
ServiceAnnotationLoadBalancerInternalSubnet = "service.beta.kubernetes.io/azure-load-balancer-internal-subnet"

// ServiceAnnotationLoadBalancerIP is the annotation used on the service
// to specify the IP of Azure load balancer
ServiceAnnotationLoadBalancerIP = "service.beta.kubernetes.io/azure-load-balancer-ip"

// ServiceAnnotationLoadBalancerMode is the annotation used on the service to specify
// which load balancer should be associated with the service. This is valid when using the basic
// load balancer or turn on the multiple standard load balancers mode, or it would be ignored.
Expand Down
18 changes: 9 additions & 9 deletions pkg/provider/azure_loadbalancer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ func (az *Cloud) determinePublicIPName(clusterName string, service *v1.Service,
}

pipResourceGroup := az.getPublicIPAddressResourceGroup(service)
loadBalancerIP := service.Spec.LoadBalancerIP
loadBalancerIP := service.Annotations[consts.ServiceAnnotationLoadBalancerIP]

// Assume that the service without loadBalancerIP set is a primary service.
// If a secondary service doesn't set the loadBalancerIP, it is not allowed to share the IP.
Expand Down Expand Up @@ -863,14 +863,14 @@ func flipServiceInternalAnnotation(service *v1.Service) *v1.Service {
func updateServiceLoadBalancerIP(service *v1.Service, serviceIP string) *v1.Service {
copyService := service.DeepCopy()
if len(serviceIP) > 0 && copyService != nil {
copyService.Spec.LoadBalancerIP = serviceIP
copyService.Annotations[consts.ServiceAnnotationLoadBalancerIP] = serviceIP
}
return copyService
}

func (az *Cloud) findServiceIPAddress(ctx context.Context, clusterName string, service *v1.Service, isInternalLb bool) (string, error) {
if len(service.Spec.LoadBalancerIP) > 0 {
return service.Spec.LoadBalancerIP, nil
if len(service.Annotations[consts.ServiceAnnotationLoadBalancerIP]) > 0 {
return service.Annotations[consts.ServiceAnnotationLoadBalancerIP], nil
}

if len(service.Status.LoadBalancer.Ingress) > 0 && len(service.Status.LoadBalancer.Ingress[0].IP) > 0 {
Expand Down Expand Up @@ -1257,7 +1257,7 @@ func (az *Cloud) isFrontendIPChanged(clusterName string, config network.Frontend
if !strings.EqualFold(to.String(config.Name), lbFrontendIPConfigName) {
return false, nil
}
loadBalancerIP := service.Spec.LoadBalancerIP
loadBalancerIP := service.Annotations[consts.ServiceAnnotationLoadBalancerIP]
isInternal := requiresInternalLoadBalancer(service)
if isInternal {
// Judge subnet
Expand Down Expand Up @@ -1779,7 +1779,7 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv
configProperties.PrivateIPAddressVersion = network.IPVersionIPv6
}

loadBalancerIP := service.Spec.LoadBalancerIP
loadBalancerIP := service.Annotations[consts.ServiceAnnotationLoadBalancerIP]
if loadBalancerIP != "" {
configProperties.PrivateIPAllocationMethod = network.IPAllocationMethodStatic
configProperties.PrivateIPAddress = &loadBalancerIP
Expand Down Expand Up @@ -3202,7 +3202,7 @@ func getServiceTags(service *v1.Service) []string {
// The pip is user-created if and only if there is no service tags.
// The service owns the pip if:
// 1. The serviceName is included in the service tags of a system-created pip.
// 2. The service.Spec.LoadBalancerIP matches the IP address of a user-created pip.
// 2. The service.Annotations[consts.ServiceAnnotationLoadBalancerIP] matches the IP address of a user-created pip.
func serviceOwnsPublicIP(service *v1.Service, pip *network.PublicIPAddress, clusterName string) (bool, bool) {
if service == nil || pip == nil {
klog.Warningf("serviceOwnsPublicIP: nil service or public IP")
Expand All @@ -3222,7 +3222,7 @@ func serviceOwnsPublicIP(service *v1.Service, pip *network.PublicIPAddress, clus

// if there is no service tag on the pip, it is user-created pip
if serviceTag == "" {
return strings.EqualFold(to.String(pip.IPAddress), service.Spec.LoadBalancerIP), true
return strings.EqualFold(to.String(pip.IPAddress), service.Annotations[consts.ServiceAnnotationLoadBalancerIP]), true
}

// if there is service tag on the pip, it is system-created pip
Expand All @@ -3240,7 +3240,7 @@ func serviceOwnsPublicIP(service *v1.Service, pip *network.PublicIPAddress, clus
} else {
// if the service is not included in te tags of the system-created pip, check the ip address
// this could happen for secondary services
return strings.EqualFold(to.String(pip.IPAddress), service.Spec.LoadBalancerIP), false
return strings.EqualFold(to.String(pip.IPAddress), service.Annotations[consts.ServiceAnnotationLoadBalancerIP]), false
}
}

Expand Down
49 changes: 24 additions & 25 deletions pkg/provider/azure_loadbalancer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ func TestServiceOwnsPublicIP(t *testing.T) {
t.Run(c.desc, func(t *testing.T) {
service := getTestService(c.serviceName, v1.ProtocolTCP, nil, false, 80)
if c.serviceLBIP != "" {
service.Spec.LoadBalancerIP = c.serviceLBIP
service.Annotations[consts.ServiceAnnotationLoadBalancerIP] = c.serviceLBIP
}
owns, isUserAssignedPIP := serviceOwnsPublicIP(&service, c.pip, c.clusterName)
assert.Equal(t, c.expectedOwns, owns, "TestCase[%d]: %s", i, c.desc)
Expand Down Expand Up @@ -1542,8 +1542,7 @@ func TestIsFrontendIPChanged(t *testing.T) {
config network.FrontendIPConfiguration
service v1.Service
lbFrontendIPConfigName string
annotations string
loadBalancerIP string
annotations map[string]string
existingSubnet network.Subnet
existingPIPs []network.PublicIPAddress
expectedFlag bool
Expand All @@ -1568,7 +1567,7 @@ func TestIsFrontendIPChanged(t *testing.T) {
expectedError: false,
},
{
desc: "isFrontendIPChanged shall return false if the service is internal, no loadBalancerIP is given, " +
desc: "isFrontendIPChanged shall return false if the service is internal, no annotation service.beta.kubernetes.io/azure-load-balancer-ip is given, " +
"subnetName == nil and config.PrivateIPAllocationMethod == network.Static",
config: network.FrontendIPConfiguration{
Name: to.StringPtr("atest1-name"),
Expand All @@ -1581,7 +1580,7 @@ func TestIsFrontendIPChanged(t *testing.T) {
expectedError: false,
},
{
desc: "isFrontendIPChanged shall return false if the service is internal, no loadBalancerIP is given, " +
desc: "isFrontendIPChanged shall return false if the service is internal, no annotation service.beta.kubernetes.io/azure-load-balancer-ip is given, " +
"subnetName == nil and config.PrivateIPAllocationMethod != network.Static",
config: network.FrontendIPConfiguration{
Name: to.StringPtr("btest1-name"),
Expand All @@ -1605,14 +1604,14 @@ func TestIsFrontendIPChanged(t *testing.T) {
},
lbFrontendIPConfigName: "btest1-name",
service: getInternalTestService("test1", 80),
annotations: "testSubnet",
annotations: map[string]string{consts.ServiceAnnotationLoadBalancerInternalSubnet:"testSubnet"},
existingSubnet: network.Subnet{ID: to.StringPtr("testSubnet1")},
expectedFlag: true,
expectedError: false,
},
{
desc: "isFrontendIPChanged shall return true if the service is internal, subnet == nil, " +
"loadBalancerIP != '' and config.PrivateIPAllocationMethod != 'static'",
"annotations[service.beta.kubernetes.io/azure-load-balancer-ip] != '' and config.PrivateIPAllocationMethod != 'static'",
config: network.FrontendIPConfiguration{
Name: to.StringPtr("btest1-name"),
FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{
Expand All @@ -1621,13 +1620,13 @@ func TestIsFrontendIPChanged(t *testing.T) {
},
lbFrontendIPConfigName: "btest1-name",
service: getInternalTestService("test1", 80),
loadBalancerIP: "1.1.1.1",
annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "1.1.1.1"},
expectedFlag: true,
expectedError: false,
},
{
desc: "isFrontendIPChanged shall return true if the service is internal, subnet == nil and " +
"loadBalancerIP != config.PrivateIPAddress",
"annotations[service.beta.kubernetes.io/azure-load-balancer-ip] != config.PrivateIPAddress",
config: network.FrontendIPConfiguration{
Name: to.StringPtr("btest1-name"),
FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{
Expand All @@ -1637,7 +1636,7 @@ func TestIsFrontendIPChanged(t *testing.T) {
},
lbFrontendIPConfigName: "btest1-name",
service: getInternalTestService("test1", 80),
loadBalancerIP: "1.1.1.1",
annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "1.1.1.1"},
expectedFlag: true,
expectedError: false,
},
Expand All @@ -1653,7 +1652,7 @@ func TestIsFrontendIPChanged(t *testing.T) {
},
lbFrontendIPConfigName: "btest1-name",
service: getTestService("test1", v1.ProtocolTCP, nil, false, 80),
loadBalancerIP: "1.1.1.1",
annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "1.1.1.1"},
existingPIPs: []network.PublicIPAddress{
{
Name: to.StringPtr("pipName"),
Expand All @@ -1677,7 +1676,7 @@ func TestIsFrontendIPChanged(t *testing.T) {
},
lbFrontendIPConfigName: "btest1-name",
service: getTestService("test1", v1.ProtocolTCP, nil, false, 80),
loadBalancerIP: "1.1.1.1",
annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "1.1.1.1"},
existingPIPs: []network.PublicIPAddress{
{
Name: to.StringPtr("pipName"),
Expand All @@ -1704,7 +1703,7 @@ func TestIsFrontendIPChanged(t *testing.T) {
},
lbFrontendIPConfigName: "btest1-name",
service: getTestService("test1", v1.ProtocolTCP, nil, false, 80),
loadBalancerIP: "1.1.1.1",
annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "1.1.1.1"},
existingPIPs: []network.PublicIPAddress{
{
Name: to.StringPtr("pipName"),
Expand Down Expand Up @@ -1739,8 +1738,8 @@ func TestIsFrontendIPChanged(t *testing.T) {
t.Fatalf("TestCase[%d] meets unexpected error: %v", i, err)
}
}
test.service.Spec.LoadBalancerIP = test.loadBalancerIP
test.service.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet] = test.annotations
test.service.Annotations[consts.ServiceAnnotationLoadBalancerIP] = test.annotations[consts.ServiceAnnotationLoadBalancerIP]
test.service.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet] = test.annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet]
flag, rerr := az.isFrontendIPChanged("testCluster", test.config,
&test.service, test.lbFrontendIPConfigName, &test.existingPIPs)
if rerr != nil {
Expand All @@ -1757,28 +1756,28 @@ func TestDeterminePublicIPName(t *testing.T) {

testCases := []struct {
desc string
loadBalancerIP string
annotations map[string]string
existingPIPs []network.PublicIPAddress
expectedIP string
expectedError bool
}{
{
desc: "determinePublicIpName shall get public IP from az.getPublicIPName if no specific " +
"loadBalancerIP is given",
"annotation service.beta.kubernetes.io/azure-load-balancer-ip is given",
expectedIP: "testCluster-atest1",
expectedError: false,
},
{
desc: "determinePublicIpName shall report error if loadBalancerIP is not in the resource group",
loadBalancerIP: "1.2.3.4",
desc: "determinePublicIpName shall report error if annotation service.beta.kubernetes.io/azure-load-balancer-ip is not in the resource group",
annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "1.2.3.4"},
expectedIP: "",
expectedError: true,
},
{
desc: "determinePublicIpName shall return loadBalancerIP in service.Spec if it's in the " +
desc: "determinePublicIpName shall return annotation service.beta.kubernetes.io/azure-load-balancer-ip in service.Annotations if it's in the " +
"resource group",
loadBalancerIP: "1.2.3.4",
existingPIPs: []network.PublicIPAddress{
annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "1.2.3.4"},
existingPIPs: []network.PublicIPAddress{
{
Name: to.StringPtr("pipName"),
PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
Expand All @@ -1794,7 +1793,7 @@ func TestDeterminePublicIPName(t *testing.T) {
for i, test := range testCases {
az := GetTestCloud(ctrl)
service := getTestService("test1", v1.ProtocolTCP, nil, false, 80)
service.Spec.LoadBalancerIP = test.loadBalancerIP
service.Annotations[consts.ServiceAnnotationLoadBalancerIP] = test.annotations[consts.ServiceAnnotationLoadBalancerIP]

mockPIPsClient := az.PublicIPAddressesClient.(*mockpublicipclient.MockInterface)
mockPIPsClient.EXPECT().List(gomock.Any(), "rg").Return(test.existingPIPs, nil).MaxTimes(1)
Expand Down Expand Up @@ -2716,7 +2715,7 @@ func TestReconcileLoadBalancer(t *testing.T) {
clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 3, 3)
setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1)

test.service.Spec.LoadBalancerIP = "1.2.3.4"
test.service.Annotations[consts.ServiceAnnotationLoadBalancerIP] = "1.2.3.4"

err := az.PublicIPAddressesClient.CreateOrUpdate(context.TODO(), "rg", "pipName", network.PublicIPAddress{
Name: to.StringPtr("pipName"),
Expand Down Expand Up @@ -4173,7 +4172,7 @@ func TestUnbindServiceFromPIP(t *testing.T) {
}
serviceName := "ns2/svc2"
service := getTestService(serviceName, v1.ProtocolTCP, nil, false, 80)
service.Spec.LoadBalancerIP = "1.2.3.4"
service.Annotations[consts.ServiceAnnotationLoadBalancerIP] = "1.2.3.4"
expectedTags := []map[string]*string{
nil,
{consts.ServiceTagKey: to.StringPtr("")},
Expand Down
2 changes: 1 addition & 1 deletion pkg/provider/azure_standard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func (az *Cloud) serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, serv
return true, isPrimaryService, nil
}

loadBalancerIP := service.Spec.LoadBalancerIP
loadBalancerIP := service.Annotations[consts.ServiceAnnotationLoadBalancerIP]
if loadBalancerIP == "" {
// it is a must that the secondary services set the loadBalancer IP
return false, isPrimaryService, nil
Expand Down
24 changes: 8 additions & 16 deletions pkg/provider/azure_standard_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1675,9 +1675,7 @@ func TestServiceOwnsFrontendIP(t *testing.T) {
service: &v1.Service{
ObjectMeta: meta.ObjectMeta{
UID: types.UID("secondary"),
},
Spec: v1.ServiceSpec{
LoadBalancerIP: "1.2.3.4",
Annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "4.3.2.1"},
},
},
},
Expand All @@ -1703,10 +1701,9 @@ func TestServiceOwnsFrontendIP(t *testing.T) {
service: &v1.Service{
ObjectMeta: meta.ObjectMeta{
UID: types.UID("secondary"),
Annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "4.3.2.1"},
},
Spec: v1.ServiceSpec{
LoadBalancerIP: "4.3.2.1",
},

},
},
{
Expand All @@ -1730,9 +1727,7 @@ func TestServiceOwnsFrontendIP(t *testing.T) {
service: &v1.Service{
ObjectMeta: meta.ObjectMeta{
UID: types.UID("secondary"),
},
Spec: v1.ServiceSpec{
LoadBalancerIP: "4.3.2.1",
Annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "4.3.2.1"},
},
},
},
Expand All @@ -1757,10 +1752,9 @@ func TestServiceOwnsFrontendIP(t *testing.T) {
service: &v1.Service{
ObjectMeta: meta.ObjectMeta{
UID: types.UID("secondary"),
Annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIP: "4.3.2.1"},
},
Spec: v1.ServiceSpec{
LoadBalancerIP: "4.3.2.1",
},

},
isOwned: true,
},
Expand All @@ -1775,10 +1769,8 @@ func TestServiceOwnsFrontendIP(t *testing.T) {
service: &v1.Service{
ObjectMeta: meta.ObjectMeta{
UID: types.UID("secondary"),
Annotations: map[string]string{consts.ServiceAnnotationLoadBalancerInternal: "true"},
},
Spec: v1.ServiceSpec{
LoadBalancerIP: "4.3.2.1",
Annotations: map[string]string{consts.ServiceAnnotationLoadBalancerInternal: "true", consts.ServiceAnnotationLoadBalancerIP: "4.3.2.1"},

},
},
isOwned: true,
Expand Down
Loading