Skip to content

Commit

Permalink
Merge pull request #350 from agau4779/merge-neg-annotations
Browse files Browse the repository at this point in the history 
merge Ingress NEG annotation and Expose NEG annotation
  • Loading branch information
@freehan
freehan committed Jun 22, 2018
2 parents 71df100 + ef4b268 commit c1a9af3
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 100 deletions.
76 changes: 45 additions & 31 deletions pkg/annotations/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,15 @@ const (
// '{"my-https-port":"HTTPS","my-http-port":"HTTP"}'
ServiceApplicationProtocolKey = "service.alpha.kubernetes.io/app-protocols"

// NetworkEndpointGroupAlphaAnnotation is the annotation key to enable GCE NEG feature for ingress backend services.
// To enable this feature, the value of the annotation must be "true".
// This annotation should be specified on services that are backing ingresses.
// WARNING: The feature will NOT be effective in the following circumstances:
// 1. NEG feature is not enabled in feature gate.
// 2. Service is not referenced in any ingress.
// 3. Adding this annotation on ingress.
NetworkEndpointGroupAlphaAnnotation = "alpha.cloud.google.com/load-balancer-neg"

// ExposeNEGAnnotationKey is the annotation key to specify standalone NEGs associated
// with the service. This should be a valid JSON string, as defined in
// ExposeNegAnnotation.
// example: {"80":{},"443":{}}
ExposeNEGAnnotationKey = "cloud.google.com/neg"
// NEGAnnotationKey is the annotation key to enable GCE NEG.
// The value of the annotation must be a valid JSON string in the format
// specified by type NegAnnotation. To enable, must have either Ingress: true
// or a non-empty ExposedPorts map referencing valid ServicePorts.
// examples:
// - `{"exposed_ports":{"80":{},"443":{}}}`
// - `{"ingress":true}`
// - `{"ingress": true,"exposed_ports":{"3000":{},"4000":{}}}`
NEGAnnotationKey = "cloud.google.com/neg"

// NEGStatusKey is the annotation key whose value is the status of the NEGs
// on the Service, and is applied by the NEG Controller.
Expand All @@ -68,10 +63,23 @@ const (
ProtocolHTTP2 AppProtocol = "HTTP2"
)

// ExposeNegAnnotation is the format of the annotation associated with the
// ExposeNEGAnnotationKey key, and maps ServicePort to attributes of the NEG that should be
// associated with the ServicePort. ServicePorts in this map will be NEG-enabled.
type ExposeNegAnnotation map[int32]NegAttributes
// NegAnnotation is the format of the annotation associated with the
// NEGAnnotationKey key.
type NegAnnotation struct {
// "Ingress" indicates whether to enable NEG feature for Ingress referencing
// the service. Each NEG correspond to a service port.
// NEGs will be created and managed under the following conditions:
// 1. Service is referenced by ingress
// 2. "ingress" is set to "true". Default to "false"
// When the above conditions are satisfied, Ingress will create a load balancer
// and target corresponding NEGs as backends. Service Nodeport is not required.
Ingress bool `json:"ingress,omitempty"`
// ExposedPorts specifies the service ports to be exposed as stand-alone NEG.
// The exposed NEGs will be created and managed by NEG controller.
// ExposedPorts maps ServicePort to attributes of the NEG that should be
// associated with the ServicePort.
ExposedPorts map[int32]NegAttributes `json:"exposed_ports,omitempty"`
}

// NegAttributes houses the attributes of the NEGs that are associated with the
// service. Future extensions to the Expose NEGs annotation should be added here.
Expand Down Expand Up @@ -124,8 +132,11 @@ func (svc *Service) ApplicationProtocols() (map[string]AppProtocol, error) {
// NEGEnabledForIngress returns true if the annotation is to be applied on
// Ingress-referenced ports
func (svc *Service) NEGEnabledForIngress() bool {
v, ok := svc.v[NetworkEndpointGroupAlphaAnnotation]
return ok && v == "true"
annotation, err := svc.NegAnnotation()
if err != nil {
return false
}
return annotation.Ingress
}

var (
Expand All @@ -140,34 +151,37 @@ func (svc *Service) NEGExposed() bool {
return false
}

v, ok := svc.v[ExposeNEGAnnotationKey]
return ok && len(v) > 0
annotation, err := svc.NegAnnotation()
if err != nil {
return false
}
return len(annotation.ExposedPorts) > 0
}

var (
ErrExposeNegAnnotationMissing = errors.New("No NEG ServicePorts specified")
ErrExposeNegAnnotationInvalid = errors.New("Expose NEG annotation is invalid")
)

// ExposeNegAnnotation returns the value of the Expose NEG annotation key
func (svc *Service) ExposeNegAnnotation() (ExposeNegAnnotation, error) {
annotation, ok := svc.v[ExposeNEGAnnotationKey]
// NegAnnotation returns the value of the NEG annotation key
func (svc *Service) NegAnnotation() (NegAnnotation, error) {
var res NegAnnotation
annotation, ok := svc.v[NEGAnnotationKey]
if !ok {
return nil, ErrExposeNegAnnotationMissing
return res, ErrExposeNegAnnotationMissing
}

// TODO: add link to Expose NEG documentation when complete
var exposedNegPortMap ExposeNegAnnotation
if err := json.Unmarshal([]byte(annotation), &exposedNegPortMap); err != nil {
return nil, ErrExposeNegAnnotationInvalid
if err := json.Unmarshal([]byte(annotation), &res); err != nil {
return res, ErrExposeNegAnnotationInvalid
}

return exposedNegPortMap, nil
return res, nil
}

// NEGEnabled is true if the service uses NEGs.
func (svc *Service) NEGEnabled() bool {
return svc.NEGExposed() || svc.NEGEnabledForIngress()
return svc.NEGEnabledForIngress() || svc.NEGExposed()
}

type BackendConfigs struct {
Expand Down
31 changes: 17 additions & 14 deletions pkg/annotations/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestNEGService(t *testing.T) {
svc: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
NetworkEndpointGroupAlphaAnnotation: "true",
NEGAnnotationKey: `{"ingress":true}`,
},
},
},
Expand All @@ -48,7 +48,7 @@ func TestNEGService(t *testing.T) {
svc: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
ExposeNEGAnnotationKey: `"{"80":{}}"`,
NEGAnnotationKey: `{"exposed_ports":{"80":{}}}`,
},
},
},
Expand All @@ -60,8 +60,7 @@ func TestNEGService(t *testing.T) {
svc: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
NetworkEndpointGroupAlphaAnnotation: "true",
ExposeNEGAnnotationKey: `"{"80":{}}"`,
NEGAnnotationKey: `{"ingress":true,"exposed_ports":{"80":{}}}`,
},
},
},
Expand Down Expand Up @@ -262,11 +261,11 @@ func TestBackendConfigs(t *testing.T) {
}
}

func TestExposeNegAnnotation(t *testing.T) {
func TestNegAnnotation(t *testing.T) {
testcases := []struct {
desc string
annotation string
expected ExposeNegAnnotation
expected NegAnnotation
expectedErr error
}{
{
Expand All @@ -280,15 +279,19 @@ func TestExposeNegAnnotation(t *testing.T) {
expectedErr: ErrExposeNegAnnotationInvalid,
},
{
desc: "NEG annotation references existing service ports",
expected: ExposeNegAnnotation{80: NegAttributes{}, 443: NegAttributes{}},
annotation: `{"80":{},"443":{}}`,
desc: "NEG annotation references existing service ports",
expected: NegAnnotation{
ExposedPorts: map[int32]NegAttributes{80: NegAttributes{}, 443: NegAttributes{}},
},
annotation: `{"exposed_ports":{"80":{},"443":{}}}`,
},

{
desc: "NEGServicePort takes the union of known ports and ports referenced in the annotation",
annotation: `{"80":{}}`,
expected: ExposeNegAnnotation{80: NegAttributes{}},
annotation: `{"exposed_ports":{"80":{}}}`,
expected: NegAnnotation{
ExposedPorts: map[int32]NegAttributes{80: NegAttributes{}},
},
},
}

Expand All @@ -301,18 +304,18 @@ func TestExposeNegAnnotation(t *testing.T) {

t.Run(tc.desc, func(t *testing.T) {
if len(tc.annotation) > 0 {
service.Annotations[ExposeNEGAnnotationKey] = tc.annotation
service.Annotations[NEGAnnotationKey] = tc.annotation
}

svc := FromService(service)
exposeNegStruct, err := svc.ExposeNegAnnotation()
exposeNegStruct, err := svc.NegAnnotation()

if tc.expectedErr == nil && err != nil {
t.Errorf("ExpectedNEGServicePorts to not return an error, got: %v", err)
}

if !reflect.DeepEqual(exposeNegStruct, tc.expected) {
t.Errorf("Expected NEGServicePorts to equal: %v; got: %v", tc.expected, exposeNegStruct)
t.Errorf("Expected NEGServicePorts to equal: %v; got: %v", tc.expected, exposeNegStruct.ExposedPorts)
}

if tc.expectedErr != nil && err != tc.expectedErr {
Expand Down
24 changes: 17 additions & 7 deletions pkg/neg/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ import (
"github.com/golang/glog"
apiv1 "k8s.io/api/core/v1"
extensions "k8s.io/api/extensions/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/scheme"
unversionedcore "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -63,6 +65,7 @@ type Controller struct {
endpointSynced cache.InformerSynced
ingressLister cache.Indexer
serviceLister cache.Indexer
client kubernetes.Interface

// serviceQueue takes service key as work item. Service key with format "namespace/name".
serviceQueue workqueue.RateLimitingInterface
Expand Down Expand Up @@ -97,6 +100,7 @@ func NewController(
ctx.EndpointInformer.GetIndexer())

negController := &Controller{
client: ctx.KubeClient,
manager: manager,
resyncPeriod: resyncPeriod,
recorder: recorder,
Expand Down Expand Up @@ -242,7 +246,7 @@ func (c *Controller) processService(key string) error {
if !enabled {
c.manager.StopSyncer(namespace, name)
// delete the annotation
return c.syncNegStatusAnnotation(namespace, name, service, make(PortNameMap))
return c.syncNegStatusAnnotation(namespace, name, make(PortNameMap))
}

glog.V(2).Infof("Syncing service %q", key)
Expand All @@ -261,7 +265,7 @@ func (c *Controller) processService(key string) error {
knownPorts[sp.Port] = sp.TargetPort.String()
}

annotation, err := annotations.FromService(service).ExposeNegAnnotation()
annotation, err := annotations.FromService(service).NegAnnotation()
if err != nil {
return err
}
Expand All @@ -274,26 +278,32 @@ func (c *Controller) processService(key string) error {
svcPortMap = svcPortMap.Union(negSvcPorts)
}

err = c.syncNegStatusAnnotation(namespace, name, service, svcPortMap)
err = c.syncNegStatusAnnotation(namespace, name, svcPortMap)
if err != nil {
return err
}
return c.manager.EnsureSyncers(namespace, name, svcPortMap)
}

func (c *Controller) syncNegStatusAnnotation(namespace, name string, service *apiv1.Service, portMap PortNameMap) error {
func (c *Controller) syncNegStatusAnnotation(namespace, name string, portMap PortNameMap) error {
zones, err := c.zoneGetter.ListZones()
if err != nil {
return err
}
svcClient := c.client.CoreV1().Services(namespace)
service, err := svcClient.Get(name, metav1.GetOptions{})
if err != nil {
return err
}

// Remove NEG Status Annotation when no NEG is needed
if len(portMap) == 0 {
if _, ok := service.Annotations[annotations.NEGStatusKey]; ok {
// TODO: use PATCH to remove annotation
delete(service.Annotations, annotations.NEGStatusKey)
glog.V(2).Infof("Removing expose NEG annotation from service: %s/%s", namespace, name)
return c.serviceLister.Update(service)
_, err = svcClient.Update(service)
return err
}
// service doesn't have the expose NEG annotation and doesn't need update
return nil
Expand All @@ -316,8 +326,8 @@ func (c *Controller) syncNegStatusAnnotation(namespace, name string, service *ap

service.Annotations[annotations.NEGStatusKey] = annotation
glog.V(2).Infof("Updating NEG visibility annotation %q on service %s/%s.", annotation, namespace, name)
// TODO: use PATCH to Update Annotation
return c.serviceLister.Update(service)
_, err = svcClient.Update(service)
return err
}

func (c *Controller) handleErr(err error, key interface{}) {
Expand Down
Loading

0 comments on commit c1a9af3

Please sign in to comment.