Skip to content

Commit

Permalink
Merge pull request #1244 from prameshj/release-1.10-cp
Browse files Browse the repository at this point in the history 
cherrypick PR 1236(resource annotations for ILB) into release-1.10
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Sep 1, 2020
2 parents d53c139 + 969d693 commit 2331ffa
Show file tree
Hide file tree
Showing 8 changed files with 403 additions and 112 deletions.
43 changes: 32 additions & 11 deletions pkg/annotations/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"k8s.io/api/core/v1"
"k8s.io/legacy-cloud-providers/gce"
"strings"
)

const (
Expand Down Expand Up @@ -66,6 +67,25 @@ const (
ProtocolHTTPS AppProtocol = "HTTPS"
// ProtocolHTTP2 protocol for a service
ProtocolHTTP2 AppProtocol = "HTTP2"

// ServiceStatusPrefix is the prefix used in annotations used to record
// debug information in the Service annotations. This is applicable to L4 ILB services.
ServiceStatusPrefix = "service.kubernetes.io"
// TCPForwardingRuleKey is the annotation key used by l4 controller to record
// GCP TCP forwarding rule name.
TCPForwardingRuleKey = ServiceStatusPrefix + "/tcp-forwarding-rule"
// UDPForwardingRuleKey is the annotation key used by l4 controller to record
// GCP UDP forwarding rule name.
UDPForwardingRuleKey = ServiceStatusPrefix + "/udp-forwarding-rule"
// BackendServiceKey is the annotation key used by l4 controller to record
// GCP Backend service name.
BackendServiceKey = ServiceStatusPrefix + "/backend-service"
// FirewallRuleKey is the annotation key used by l4 controller to record
// GCP Firewall rule name.
FirewallRuleKey = ServiceStatusPrefix + "/firewall-rule"
// HealthcheckKey is the annotation key used by l4 controller to record
// GCP Healthcheck name.
HealthcheckKey = ServiceStatusPrefix + "/healthcheck"
)

// NegAnnotation is the format of the annotation associated with the
Expand Down Expand Up @@ -182,24 +202,25 @@ func WantsL4ILB(service *v1.Service) (bool, string) {
return false, fmt.Sprintf("Type : %s, LBType : %s", service.Spec.Type, ltype)
}

// OnlyNEGStatusChanged returns true if the only annotation change between the 2 services is the NEG status annotation.
// This will be true if neg annotation was added or removed in the new service.
// OnlyStatusAnnotationsChanged returns true if the only annotation change between the 2 services is the NEG or ILB
// resources annotations.
// Note : This assumes that the annotations in old and new service are different. If they are identical, this will
// return true.
func OnlyNEGStatusChanged(oldService, newService *v1.Service) bool {
return onlyNEGStatusChanged(oldService, newService) && onlyNEGStatusChanged(newService, oldService)
func OnlyStatusAnnotationsChanged(oldService, newService *v1.Service) bool {
return onlyStatusAnnotationsChanged(oldService, newService) && onlyStatusAnnotationsChanged(newService, oldService)
}

// onlyNEGStatusChanged returns true if the NEG Status annotation is the only extra annotation present in the new
// service but not in the old service.
// onlyStatusAnnotationsChanged returns true if the NEG Status or ILB resources annotations are the only extra
// annotations present in the new service but not in the old service.
// Note : This assumes that the annotations in old and new service are different. If they are identical, this will
// return true.
func onlyNEGStatusChanged(oldService, newService *v1.Service) bool {
for key, _ := range newService.ObjectMeta.Annotations {
if _, ok := oldService.ObjectMeta.Annotations[key]; !ok {
if key != NEGStatusKey {
return false
func onlyStatusAnnotationsChanged(oldService, newService *v1.Service) bool {
for key, val := range newService.ObjectMeta.Annotations {
if oldVal, ok := oldService.ObjectMeta.Annotations[key]; !ok || oldVal != val {
if key == NEGStatusKey || strings.HasPrefix(key, ServiceStatusPrefix) {
continue
}
return false
}
}
return true
Expand Down
170 changes: 170 additions & 0 deletions pkg/annotations/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,3 +459,173 @@ func TestParseNegStatus(t *testing.T) {
})
}
}

func TestOnlyStatusAnnotationsChanged(t *testing.T) {
for _, tc := range []struct {
desc string
service1 *v1.Service
service2 *v1.Service
expectedResult bool
}{
{
desc: "Test add neg annotation",
service1: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service1",
},
},
service2: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service2",
Annotations: map[string]string{
NEGStatusKey: `{"network_endpoint_groups":{"80":"neg-name"},"zones":["us-central1-a"]}`,
},
},
},
expectedResult: true,
},
{
desc: "Test valid diff",
service1: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service1",
Annotations: map[string]string{
NEGStatusKey: `{"network_endpoint_groups":{"80":"neg-name"},"zones":["us-central1-a"]}`,
},
},
},
service2: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service2",
Annotations: map[string]string{
"RandomAnnotation": "abcde",
},
},
},
expectedResult: false,
},
{
desc: "Test no change",
service1: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service1",
Annotations: map[string]string{
"RandomAnnotation": "abcde",
},
},
},
service2: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service2",
Annotations: map[string]string{
"RandomAnnotation": "abcde",
},
},
},
expectedResult: true,
},
{
desc: "Test remove NEG annotation",
service1: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service1",
Annotations: map[string]string{
NEGStatusKey: `{"network_endpoint_groups":{"80":"neg-name"},"zones":["us-central1-a"]}`,
"RandomAnnotation": "abcde",
},
},
},
service2: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service2",
Annotations: map[string]string{
"RandomAnnotation": "abcde",
},
},
},
expectedResult: true,
},
{
desc: "Test only ILB ForwardingRule annotation diff",
service1: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service1",
Annotations: map[string]string{
FirewallRuleKey: "rule1",
TCPForwardingRuleKey: "tcprule",
NEGStatusKey: `{"network_endpoint_groups":{"80":"neg-name"},"zones":["us-central1-a"]}`,
"RandomAnnotation": "abcde",
},
},
},
service2: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service2",
Annotations: map[string]string{
FirewallRuleKey: "rule1",
UDPForwardingRuleKey: "udprule",
NEGStatusKey: `{"network_endpoint_groups":{"80":"neg-name"},"zones":["us-central1-a"]}`,
"RandomAnnotation": "abcde",
},
},
},
expectedResult: true,
},
{
desc: "Test all status annotations removed",
service1: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service1",
Annotations: map[string]string{
FirewallRuleKey: "rule1",
TCPForwardingRuleKey: "tcprule",
NEGStatusKey: `{"network_endpoint_groups":{"80":"neg-name"},"zones":["us-central1-a"]}`,
"RandomAnnotation": "abcde",
},
},
},
service2: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service2",
Annotations: map[string]string{
"RandomAnnotation": "abcde",
},
},
},
expectedResult: true,
},
{
desc: "Test change value of non-status annotation",
service1: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service1",
Annotations: map[string]string{
FirewallRuleKey: "rule1",
TCPForwardingRuleKey: "tcprule",
NEGStatusKey: `{"network_endpoint_groups":{"80":"neg-name"},"zones":["us-central1-a"]}`,
"RandomAnnotation": "abcde",
},
},
},
service2: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service2",
Annotations: map[string]string{
FirewallRuleKey: "rule1",
TCPForwardingRuleKey: "tcprule",
NEGStatusKey: `{"network_endpoint_groups":{"80":"neg-name"},"zones":["us-central1-a"]}`,
"RandomAnnotation": "xyz",
},
},
},
expectedResult: false,
},
} {
t.Run(tc.desc, func(t *testing.T) {
result := OnlyStatusAnnotationsChanged(tc.service1, tc.service2)
if result != tc.expectedResult {
t.Errorf("%s: Expected result for input %v, %v to be %v, got %v instead", tc.desc, tc.service1.Annotations, tc.service2.Annotations, tc.expectedResult, result)
}
})
}
}
Loading

0 comments on commit 2331ffa

Please sign in to comment.